Skip to content

Commit 0d9fe61

Browse files
aheejinaheejin
committed
[wasm-split] Allow empty string for options
We weren't able to give empty strings as options like `--import-namespace=` or `--import-namespace=""` so far because 1. When `command-line.cpp` parses `--option=`, it parses the next option as its value. For example, if we have `wasm-split ... --import-namespace -o output.wasm`, it parses `-o` as the value for `--import-namespace`. 2. Even if we fix 1, many places in `wasm-split.cpp` checks for `if (options.optionName.size())` so the option with size 0 cannot be processed. This fixes them and allow `--import-namespace` and other similar options take an empty string for a value. Since WebAssembly#7966 changed the default primary export namespace to `primary`, acx_gallery failed to run (which I realized it only recently) and giving it `--import-namespace=""` didn't work because of the above reasons.
1 parent 7a9ed0c commit 0d9fe61

6 files changed

Lines changed: 174 additions & 7 deletions

File tree

‎src/support/command-line.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,12 @@ void Options::parse(int argc, const char* argv[]) {
231231

232232
// Non-positional.
233233
std::string argument;
234+
bool explicitArg = false;
234235
auto equal = currentOption.find_first_of('=');
235236
if (equal != std::string::npos) {
236237
argument = currentOption.substr(equal + 1);
237238
currentOption = currentOption.substr(0, equal);
239+
explicitArg = true;
238240
}
239241
Option* option = nullptr;
240242
for (auto& o : options) {
@@ -262,7 +264,7 @@ void Options::parse(int argc, const char* argv[]) {
262264
}
263265
[[fallthrough]];
264266
case Arguments::N:
265-
if (!argument.size()) {
267+
if (!argument.size() && !explicitArg) {
266268
if (i + 1 == e) {
267269
std::cerr << "Couldn't find expected argument for '"
268270
<< currentOption << "'\n";

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ WasmSplitOptions::WasmSplitOptions()
235235
Options::Arguments::One,
236236
[&](Options* o, const std::string& argument) {
237237
importNamespace = argument;
238+
hasImportNamespace = true;
238239
})
239240
.add("--placeholder-namespace-prefix",
240241
"",
@@ -246,6 +247,7 @@ WasmSplitOptions::WasmSplitOptions()
246247
Options::Arguments::One,
247248
[&](Options* o, const std::string& argument) {
248249
placeholderNamespacePrefix = argument;
250+
hasPlaceholderNamespacePrefix = true;
249251
})
250252
.add("--placeholder-namespace",
251253
"",
@@ -255,6 +257,7 @@ WasmSplitOptions::WasmSplitOptions()
255257
Options::Arguments::One,
256258
[&](Options* o, const std::string& argument) {
257259
placeholderNamespacePrefix = argument;
260+
hasPlaceholderNamespacePrefix = true;
258261
})
259262
.add("--jspi",
260263
"",
@@ -272,7 +275,10 @@ WasmSplitOptions::WasmSplitOptions()
272275
WasmSplitOption,
273276
{Mode::Split, Mode::MultiSplit},
274277
Options::Arguments::One,
275-
[&](Options* o, const std::string& argument) { exportPrefix = argument; })
278+
[&](Options* o, const std::string& argument) {
279+
exportPrefix = argument;
280+
hasExportPrefix = true;
281+
})
276282
.add("--profile-export",
277283
"",
278284
"The export name of the function the embedder calls to write the "
@@ -319,6 +325,7 @@ WasmSplitOptions::WasmSplitOptions()
319325
Options::Arguments::One,
320326
[&](Options* o, const std::string& argument) {
321327
secondaryMemoryName = argument;
328+
hasSecondaryMemoryName = true;
322329
})
323330
.add(
324331
"--emit-module-names",

‎src/tools/wasm-split/split-options.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,13 @@ struct WasmSplitOptions : ToolOptions {
6767
std::string secondaryOutput;
6868

6969
std::string importNamespace;
70+
bool hasImportNamespace = false;
7071
std::string placeholderNamespacePrefix;
72+
bool hasPlaceholderNamespacePrefix = false;
7173
std::string secondaryMemoryName;
74+
bool hasSecondaryMemoryName = false;
7275
std::string exportPrefix;
76+
bool hasExportPrefix = false;
7377

7478
std::string manifestFile;
7579
std::string outPrefix;

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,10 @@ void instrumentModule(const WasmSplitOptions& options) {
115115

116116
uint64_t moduleHash = hashFile(options.inputFiles[0]);
117117
InstrumenterConfig config;
118-
if (options.importNamespace.size()) {
118+
if (options.hasImportNamespace) {
119119
config.importNamespace = options.importNamespace;
120120
}
121-
if (options.secondaryMemoryName.size()) {
121+
if (options.hasSecondaryMemoryName) {
122122
config.secondaryMemoryName = options.secondaryMemoryName;
123123
}
124124
config.storageKind = options.storageKind;
@@ -226,13 +226,13 @@ void setCommonSplitConfigs(ModuleSplitting::Config& config,
226226
const WasmSplitOptions& options) {
227227
config.usePlaceholders = options.usePlaceholders;
228228
config.minimizeNewExportNames = !options.passOptions.debugInfo;
229-
if (options.importNamespace.size()) {
229+
if (options.hasImportNamespace) {
230230
config.importNamespace = options.importNamespace;
231231
}
232-
if (options.exportPrefix.size()) {
232+
if (options.hasExportPrefix) {
233233
config.newExportPrefix = options.exportPrefix;
234234
}
235-
if (options.placeholderNamespacePrefix.size()) {
235+
if (options.hasPlaceholderNamespacePrefix) {
236236
config.placeholderNamespacePrefix = options.placeholderNamespacePrefix;
237237
}
238238
}

‎test/lit/wasm-split/instrument-in-secondary-memory-custom-names.wast‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
;; RUN: wasm-split %s --instrument --in-secondary-memory --import-namespace=custom_env --secondary-memory-name=custom_name -all -S -o - | filecheck %s
22

3+
;; RUN: wasm-split %s --instrument --in-secondary-memory --import-namespace= --secondary-memory-name= -all -S -o - | filecheck %s --check-prefix=EMPTY
4+
35
;; Check that the output round trips and validates as well
46
;; RUN: wasm-split %s --instrument --in-secondary-memory -all -g -o %t.wasm
57
;; RUN: wasm-opt -all %t.wasm -S -o -
@@ -29,3 +31,11 @@
2931
;; CHECK: (i32.atomic.store8 $custom_name
3032
;; CHECK: (i32.atomic.store8 $custom_name offset=1
3133
;; CHECK: (i32.atomic.load8_u $custom_name
34+
35+
;; Do the checks for an empty import namespace and empty secondary memory name
36+
;; as well;
37+
;; EMPTY: (import "" "" (memory $"" 1 1 shared))
38+
39+
;; EMPTY: (i32.atomic.store8 $""
40+
;; EMPTY: (i32.atomic.store8 $"" offset=1
41+
;; EMPTY: (i32.atomic.load8_u $""

‎test/lit/wasm-split/multi-split-options.wast‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@
1212
;; RUN: wasm-split -all -g --multi-split %s --manifest %S/multi-split.wast.manifest --out-prefix=%t --placeholder-namespace=placeholder_env -o %t.wasm
1313
;; RUN: wasm-dis %t.wasm | filecheck %s --check-prefix=PRIMARY-PLACEHOLDER-NAMESPACE
1414

15+
;; Check if empty strings work for options.
16+
;; RUN: wasm-split -all -g --multi-split %s --manifest %S/multi-split.wast.manifest --out-prefix=%t --import-namespace= --placeholder-namespace= --export-prefix= -o %t.wasm
17+
;; RUN: wasm-dis %t.wasm | filecheck %s --check-prefix=PRIMARY-EMPTY
18+
;; RUN: wasm-dis %t1.wasm | filecheck %s --check-prefix=MOD1-EMPTY
19+
;; RUN: wasm-dis %t2.wasm | filecheck %s --check-prefix=MOD2-EMPTY
20+
;; RUN: wasm-dis %t3.wasm | filecheck %s --check-prefix=MOD3-EMPTY
21+
1522
(module
1623
;; PRIMARY-OPTIONS: (type $ret-i64 (func (result i64)))
1724

@@ -23,6 +30,11 @@
2330
;; PRIMARY-PLACEHOLDER-NAMESPACE: (type $ret-f32 (func (result f32)))
2431

2532
;; PRIMARY-PLACEHOLDER-NAMESPACE: (type $ret-i32 (func (result i32)))
33+
;; PRIMARY-EMPTY: (type $ret-i64 (func (result i64)))
34+
35+
;; PRIMARY-EMPTY: (type $ret-f32 (func (result f32)))
36+
37+
;; PRIMARY-EMPTY: (type $ret-i32 (func (result i32)))
2638
(type $ret-i32 (func (result i32)))
2739
(type $ret-i64 (func (result i64)))
2840
(type $ret-f32 (func (result f32)))
@@ -59,6 +71,38 @@
5971
;; MOD1-OPTIONS-NEXT: )
6072
;; MOD1-OPTIONS-NEXT: (i32.const 0)
6173
;; MOD1-OPTIONS-NEXT: )
74+
;; MOD1-EMPTY: (type $0 (func (result i64)))
75+
76+
;; MOD1-EMPTY: (type $1 (func (result f32)))
77+
78+
;; MOD1-EMPTY: (type $2 (func (result i32)))
79+
80+
;; MOD1-EMPTY: (import "" "table" (table $timport$0 3 funcref))
81+
82+
;; MOD1-EMPTY: (import "" "trampoline_B" (func $trampoline_B (exact (result i64))))
83+
84+
;; MOD1-EMPTY: (import "" "trampoline_C" (func $trampoline_C (exact (result f32))))
85+
86+
;; MOD1-EMPTY: (elem $0 (i32.const 2) $A)
87+
88+
;; MOD1-EMPTY: (func $A (result i32)
89+
;; MOD1-EMPTY-NEXT: (drop
90+
;; MOD1-EMPTY-NEXT: (call_ref $2
91+
;; MOD1-EMPTY-NEXT: (ref.func $A)
92+
;; MOD1-EMPTY-NEXT: )
93+
;; MOD1-EMPTY-NEXT: )
94+
;; MOD1-EMPTY-NEXT: (drop
95+
;; MOD1-EMPTY-NEXT: (call_ref $0
96+
;; MOD1-EMPTY-NEXT: (ref.func $trampoline_B)
97+
;; MOD1-EMPTY-NEXT: )
98+
;; MOD1-EMPTY-NEXT: )
99+
;; MOD1-EMPTY-NEXT: (drop
100+
;; MOD1-EMPTY-NEXT: (call_ref $1
101+
;; MOD1-EMPTY-NEXT: (ref.func $trampoline_C)
102+
;; MOD1-EMPTY-NEXT: )
103+
;; MOD1-EMPTY-NEXT: )
104+
;; MOD1-EMPTY-NEXT: (i32.const 0)
105+
;; MOD1-EMPTY-NEXT: )
62106
(func $A (type $ret-i32) (result i32)
63107
(drop
64108
(call_ref $ret-i32
@@ -110,6 +154,38 @@
110154
;; MOD2-OPTIONS-NEXT: )
111155
;; MOD2-OPTIONS-NEXT: (i64.const 0)
112156
;; MOD2-OPTIONS-NEXT: )
157+
;; MOD2-EMPTY: (type $0 (func (result i32)))
158+
159+
;; MOD2-EMPTY: (type $1 (func (result f32)))
160+
161+
;; MOD2-EMPTY: (type $2 (func (result i64)))
162+
163+
;; MOD2-EMPTY: (import "" "table" (table $timport$0 3 funcref))
164+
165+
;; MOD2-EMPTY: (import "" "trampoline_A" (func $trampoline_A (exact (result i32))))
166+
167+
;; MOD2-EMPTY: (import "" "trampoline_C" (func $trampoline_C (exact (result f32))))
168+
169+
;; MOD2-EMPTY: (elem $0 (i32.const 0) $B)
170+
171+
;; MOD2-EMPTY: (func $B (result i64)
172+
;; MOD2-EMPTY-NEXT: (drop
173+
;; MOD2-EMPTY-NEXT: (call_ref $0
174+
;; MOD2-EMPTY-NEXT: (ref.func $trampoline_A)
175+
;; MOD2-EMPTY-NEXT: )
176+
;; MOD2-EMPTY-NEXT: )
177+
;; MOD2-EMPTY-NEXT: (drop
178+
;; MOD2-EMPTY-NEXT: (call_ref $2
179+
;; MOD2-EMPTY-NEXT: (ref.func $B)
180+
;; MOD2-EMPTY-NEXT: )
181+
;; MOD2-EMPTY-NEXT: )
182+
;; MOD2-EMPTY-NEXT: (drop
183+
;; MOD2-EMPTY-NEXT: (call_ref $1
184+
;; MOD2-EMPTY-NEXT: (ref.func $trampoline_C)
185+
;; MOD2-EMPTY-NEXT: )
186+
;; MOD2-EMPTY-NEXT: )
187+
;; MOD2-EMPTY-NEXT: (i64.const 0)
188+
;; MOD2-EMPTY-NEXT: )
113189
(func $B (type $ret-i64) (result i64)
114190
(drop
115191
(call_ref $ret-i32
@@ -161,6 +237,38 @@
161237
;; MOD3-OPTIONS-NEXT: )
162238
;; MOD3-OPTIONS-NEXT: (f32.const 0)
163239
;; MOD3-OPTIONS-NEXT: )
240+
;; MOD3-EMPTY: (type $0 (func (result i32)))
241+
242+
;; MOD3-EMPTY: (type $1 (func (result i64)))
243+
244+
;; MOD3-EMPTY: (type $2 (func (result f32)))
245+
246+
;; MOD3-EMPTY: (import "" "table" (table $timport$0 3 funcref))
247+
248+
;; MOD3-EMPTY: (import "" "trampoline_A" (func $trampoline_A (exact (result i32))))
249+
250+
;; MOD3-EMPTY: (import "" "trampoline_B" (func $trampoline_B (exact (result i64))))
251+
252+
;; MOD3-EMPTY: (elem $0 (i32.const 1) $C)
253+
254+
;; MOD3-EMPTY: (func $C (result f32)
255+
;; MOD3-EMPTY-NEXT: (drop
256+
;; MOD3-EMPTY-NEXT: (call_ref $0
257+
;; MOD3-EMPTY-NEXT: (ref.func $trampoline_A)
258+
;; MOD3-EMPTY-NEXT: )
259+
;; MOD3-EMPTY-NEXT: )
260+
;; MOD3-EMPTY-NEXT: (drop
261+
;; MOD3-EMPTY-NEXT: (call_ref $1
262+
;; MOD3-EMPTY-NEXT: (ref.func $trampoline_B)
263+
;; MOD3-EMPTY-NEXT: )
264+
;; MOD3-EMPTY-NEXT: )
265+
;; MOD3-EMPTY-NEXT: (drop
266+
;; MOD3-EMPTY-NEXT: (call_ref $2
267+
;; MOD3-EMPTY-NEXT: (ref.func $C)
268+
;; MOD3-EMPTY-NEXT: )
269+
;; MOD3-EMPTY-NEXT: )
270+
;; MOD3-EMPTY-NEXT: (f32.const 0)
271+
;; MOD3-EMPTY-NEXT: )
164272
(func $C (type $ret-f32) (result f32)
165273
(drop
166274
(call_ref $ret-i32
@@ -245,3 +353,39 @@
245353
;; PRIMARY-PLACEHOLDER-NAMESPACE-NEXT: (i32.const 2)
246354
;; PRIMARY-PLACEHOLDER-NAMESPACE-NEXT: )
247355
;; PRIMARY-PLACEHOLDER-NAMESPACE-NEXT: )
356+
357+
;; PRIMARY-EMPTY: (import ".2" "0" (func $placeholder_0 (result i64)))
358+
359+
;; PRIMARY-EMPTY: (import ".3" "1" (func $placeholder_1 (result f32)))
360+
361+
;; PRIMARY-EMPTY: (import ".1" "2" (func $placeholder_2 (result i32)))
362+
363+
;; PRIMARY-EMPTY: (table $0 3 funcref)
364+
365+
;; PRIMARY-EMPTY: (elem $0 (i32.const 0) $placeholder_0 $placeholder_1 $placeholder_2)
366+
367+
;; PRIMARY-EMPTY: (export "trampoline_B" (func $trampoline_B))
368+
369+
;; PRIMARY-EMPTY: (export "trampoline_C" (func $trampoline_C))
370+
371+
;; PRIMARY-EMPTY: (export "trampoline_A" (func $trampoline_A))
372+
373+
;; PRIMARY-EMPTY: (export "table" (table $0))
374+
375+
;; PRIMARY-EMPTY: (func $trampoline_B (result i64)
376+
;; PRIMARY-EMPTY-NEXT: (call_indirect (type $ret-i64)
377+
;; PRIMARY-EMPTY-NEXT: (i32.const 0)
378+
;; PRIMARY-EMPTY-NEXT: )
379+
;; PRIMARY-EMPTY-NEXT: )
380+
381+
;; PRIMARY-EMPTY: (func $trampoline_C (result f32)
382+
;; PRIMARY-EMPTY-NEXT: (call_indirect (type $ret-f32)
383+
;; PRIMARY-EMPTY-NEXT: (i32.const 1)
384+
;; PRIMARY-EMPTY-NEXT: )
385+
;; PRIMARY-EMPTY-NEXT: )
386+
387+
;; PRIMARY-EMPTY: (func $trampoline_A (result i32)
388+
;; PRIMARY-EMPTY-NEXT: (call_indirect (type $ret-i32)
389+
;; PRIMARY-EMPTY-NEXT: (i32.const 2)
390+
;; PRIMARY-EMPTY-NEXT: )
391+
;; PRIMARY-EMPTY-NEXT: )

0 commit comments

Comments
 (0)