Skip to content

Commit bf8c98e

Browse files
committed
[wasm_split] Fix crash when manifest starts with a blank line
When the first line of the manifest file is blank, currFuncs is null, but will be dereferenced to check if the current module will be empty. This CL fixes that and also cleans up the logic to clarify how the state machine works.
1 parent 0b8d88e commit bf8c98e

2 files changed

Lines changed: 80 additions & 28 deletions

File tree

‎src/tools/wasm-split/wasm-split.cpp‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -424,42 +424,41 @@ void multiSplitModule(const WasmSplitOptions& options) {
424424
bool newSection = true;
425425
std::unordered_set<Name> moduleNameSet;
426426
while (std::getline(manifest, line)) {
427-
if (line.empty()) {
428-
newSection = true;
429-
if (currFuncs->empty() && !options.quiet) {
430-
std::cerr << "warning: Module " << currModule << " will be empty\n";
431-
}
432-
continue;
433-
}
434-
Name name = line;
435427
if (newSection) {
436-
if (name.endsWith(":")) {
437-
name = name.substr(0, name.size() - 1);
438-
if (name.size() == 0) {
439-
Fatal() << "Module name is empty\n";
440-
}
428+
Name moduleName = line;
429+
if (moduleName.endsWith(":")) {
430+
moduleName = moduleName.substr(0, moduleName.size() - 1);
431+
}
432+
if (moduleName.size() == 0) {
433+
Fatal() << "Module name is empty\n";
441434
}
442-
if (moduleNameSet.contains(name)) {
443-
Fatal() << "Module name " << name << " is listed more than once\n";
435+
if (moduleNameSet.contains(moduleName)) {
436+
Fatal() << "Module name " << moduleName
437+
<< " is listed more than once\n";
444438
}
445-
currModule = name;
439+
currModule = moduleName;
446440
moduleNameSet.insert(currModule);
447441
config.secondaryNames.push_back(currModule);
448442
config.secondaryFuncs.emplace_back(std::set<Name>());
449443
currFuncs = &config.secondaryFuncs.back();
450444
newSection = false;
451-
continue;
452-
}
453-
assert(currFuncs);
454-
if (!canSplitFunc(name, wasm, options)) {
455-
continue;
456-
}
457-
currFuncs->insert(name);
458-
auto [it, inserted] = funcModules.insert({name, currModule});
459-
if (!inserted && it->second != currModule) {
460-
Fatal() << "Function " << name << "cannot be assigned to module "
461-
<< currModule << "; it is already assigned to module "
462-
<< it->second << '\n';
445+
} else if (line.empty()) {
446+
newSection = true;
447+
if (currFuncs->empty() && !options.quiet) {
448+
std::cerr << "warning: Module " << currModule << " will be empty\n";
449+
}
450+
} else {
451+
assert(currFuncs);
452+
Name funcName = line;
453+
if (canSplitFunc(funcName, wasm, options)) {
454+
currFuncs->insert(funcName);
455+
auto [it, inserted] = funcModules.try_emplace(funcName, currModule);
456+
if (!inserted && it->second != currModule) {
457+
Fatal() << "Function " << funcName << "cannot be assigned to module "
458+
<< currModule << "; it is already assigned to module "
459+
<< it->second << '\n';
460+
}
461+
}
463462
}
464463
}
465464

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
;; Check that a manifest starting with a blank line produces an error rather
2+
;; than crashing.
3+
;; RUN: echo "" > %t.blank.manifest
4+
;; RUN: echo "1:" >> %t.blank.manifest
5+
;; RUN: echo "foo" >> %t.blank.manifest
6+
;; RUN: not wasm-split --multi-split %s --manifest %t.blank.manifest --out-prefix=%t -o %t.wasm 2>&1 \
7+
;; RUN: | filecheck %s --check-prefix=EMPTY-NAME
8+
9+
;; Also check a module name consisting only of a colon.
10+
;; RUN: echo ":" > %t.colon.manifest
11+
;; RUN: echo "foo" >> %t.colon.manifest
12+
;; RUN: not wasm-split --multi-split %s --manifest %t.colon.manifest --out-prefix=%t -o %t.wasm 2>&1 \
13+
;; RUN: | filecheck %s --check-prefix=EMPTY-NAME
14+
15+
;; Check that an empty module followed by two blank lines warns about the empty
16+
;; module and then errors on the second blank line.
17+
;; RUN: echo "1:" > %t.empty-mod.manifest
18+
;; RUN: echo "" >> %t.empty-mod.manifest
19+
;; RUN: echo "" >> %t.empty-mod.manifest
20+
;; RUN: not wasm-split --multi-split %s --manifest %t.empty-mod.manifest --out-prefix=%t -o %t.wasm 2>&1 \
21+
;; RUN: | filecheck %s --check-prefix=EMPTY-MOD
22+
23+
;; Check that duplicate module names are rejected.
24+
;; RUN: echo "1:" > %t.dup-mod.manifest
25+
;; RUN: echo "foo" >> %t.dup-mod.manifest
26+
;; RUN: echo "" >> %t.dup-mod.manifest
27+
;; RUN: echo "1:" >> %t.dup-mod.manifest
28+
;; RUN: echo "bar" >> %t.dup-mod.manifest
29+
;; RUN: not wasm-split --multi-split %s --manifest %t.dup-mod.manifest --out-prefix=%t -o %t.wasm 2>&1 \
30+
;; RUN: | filecheck %s --check-prefix=DUP-MOD
31+
32+
;; Check that assigning the same function to two modules is rejected.
33+
;; RUN: echo "1:" > %t.dup-func.manifest
34+
;; RUN: echo "foo" >> %t.dup-func.manifest
35+
;; RUN: echo "" >> %t.dup-func.manifest
36+
;; RUN: echo "2:" >> %t.dup-func.manifest
37+
;; RUN: echo "foo" >> %t.dup-func.manifest
38+
;; RUN: not wasm-split --multi-split %s --manifest %t.dup-func.manifest --out-prefix=%t -o %t.wasm 2>&1 \
39+
;; RUN: | filecheck %s --check-prefix=DUP-FUNC
40+
41+
;; EMPTY-NAME: Fatal: Module name is empty
42+
43+
;; EMPTY-MOD: warning: Module 1 will be empty
44+
;; EMPTY-MOD: Fatal: Module name is empty
45+
46+
;; DUP-MOD: Fatal: Module name 1 is listed more than once
47+
48+
;; DUP-FUNC: Fatal: Function foocannot be assigned to module 2; it is already assigned to module 1
49+
50+
(module
51+
(func $foo)
52+
(func $bar)
53+
)

0 commit comments

Comments
 (0)