Skip to content

Commit c87a7da

Browse files
authored
[Strings] Erase the strings section after StringLifting (#7546)
After we lift the strings section into stringref, we don't need the section any more, and leaving it around could cause problems with repeated lifting/ lowering operations (which would be very much possible with #7540). In particular, without erasing it, we'd accumulate such sections over time.
1 parent 5f04581 commit c87a7da

File tree

2 files changed

+58
-31
lines changed

2 files changed

+58
-31
lines changed

src/passes/StringLifting.cpp

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -79,41 +79,45 @@ struct StringLifting : public Pass {
7979
}
8080

8181
// Imported strings may also be found in the string section.
82-
for (auto& section : module->customSections) {
83-
if (section.name == "string.consts") {
84-
// We found the string consts section. Parse it.
85-
auto copy = section.data;
86-
json::Value array;
87-
array.parse(copy.data(), json::Value::WTF16);
88-
if (!array.isArray()) {
82+
auto stringSectionIter = std::find_if(
83+
module->customSections.begin(),
84+
module->customSections.end(),
85+
[&](CustomSection& section) { return section.name == "string.consts"; });
86+
if (stringSectionIter != module->customSections.end()) {
87+
// We found the string consts section. Parse it.
88+
auto& section = *stringSectionIter;
89+
auto copy = section.data;
90+
json::Value array;
91+
array.parse(copy.data(), json::Value::WTF16);
92+
if (!array.isArray()) {
93+
Fatal() << "StringLifting: string.const section should be a JSON array";
94+
}
95+
96+
// We have the array of constants from the section. Find globals that
97+
// refer to it.
98+
for (auto& global : module->globals) {
99+
if (!global->imported() || global->module != "string.const") {
100+
continue;
101+
}
102+
// The index in the array is the basename.
103+
Index index = std::stoi(std::string(global->base.str));
104+
if (index >= array.size()) {
105+
Fatal() << "StringLifting: bad index in string.const section";
106+
}
107+
auto item = array[index];
108+
if (!item->isString()) {
89109
Fatal()
90-
<< "StringLifting: string.const section should be a JSON array";
110+
<< "StringLifting: string.const section entry is not a string";
91111
}
92-
93-
// We have the array of constants from the section. Find globals that
94-
// refer to it.
95-
for (auto& global : module->globals) {
96-
if (!global->imported() || global->module != "string.const") {
97-
continue;
98-
}
99-
// The index in the array is the basename.
100-
Index index = std::stoi(std::string(global->base.str));
101-
if (index >= array.size()) {
102-
Fatal() << "StringLifting: bad index in string.const section";
103-
}
104-
auto item = array[index];
105-
if (!item->isString()) {
106-
Fatal()
107-
<< "StringLifting: string.const section entry is not a string";
108-
}
109-
if (importedStrings.count(global->name)) {
110-
Fatal()
111-
<< "StringLifting: string.const section tramples other const";
112-
}
113-
importedStrings[global->name] = item->getIString();
112+
if (importedStrings.count(global->name)) {
113+
Fatal() << "StringLifting: string.const section tramples other const";
114114
}
115-
break;
115+
importedStrings[global->name] = item->getIString();
116116
}
117+
118+
// Remove the custom section: After lifting it has no purpose (and could
119+
// cause problems with repeated lifting/lowering).
120+
module->customSections.erase(stringSectionIter);
117121
}
118122

119123
auto array16 = Type(Array(Field(Field::i16, Mutable)), Nullable);
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
;; Test that the section vanishes after lifting.
2+
3+
(module
4+
(func $strings
5+
;; These strings cannot appear as magic imports, and will definitely go in
6+
;; the strings section.
7+
(drop
8+
(string.const "unpaired high surrogate \ED\A0\80 ")
9+
)
10+
(drop
11+
(string.const "unpaired low surrogate \ED\BD\88 ")
12+
)
13+
)
14+
)
15+
16+
;; Lower into the section. We should see the section.
17+
;; RUN: wasm-opt %s -all --string-lowering -S -o - | filecheck %s --check-prefix=LOWER
18+
;; LOWER: custom section
19+
20+
;; Also lift. Now no section should appear.
21+
;; RUN: wasm-opt %s -all --string-lowering --string-lifting -S -o - | filecheck %s --check-prefix=AND_LIFT
22+
;; AND_LIFT-NOT: custom section
23+

0 commit comments

Comments
 (0)