Skip to content

Commit f9a3380

Browse files
aheejinaheejin
authored
[wasm-split] Allow empty string for options (WebAssembly#8243)
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) because it assumed the empty string namespace, and giving it `--import-namespace=""` didn't work because of the above reasons.
1 parent f6871f4 commit f9a3380

File tree

5 files changed

+173
-15
lines changed

5 files changed

+173
-15
lines changed

‎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.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
#ifndef wasm_tools_wasm_split_options_h
1818
#define wasm_tools_wasm_split_options_h
1919

20+
#include <optional>
21+
2022
#include "tools/tool-options.h"
2123

2224
namespace wasm {
@@ -66,10 +68,10 @@ struct WasmSplitOptions : ToolOptions {
6668
std::string primaryOutput;
6769
std::string secondaryOutput;
6870

69-
std::string importNamespace;
70-
std::string placeholderNamespacePrefix;
71-
std::string secondaryMemoryName;
72-
std::string exportPrefix;
71+
std::optional<std::string> importNamespace;
72+
std::optional<std::string> placeholderNamespacePrefix;
73+
std::optional<std::string> secondaryMemoryName;
74+
std::optional<std::string> exportPrefix;
7375

7476
std::string manifestFile;
7577
std::string outPrefix;

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

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

116116
uint64_t moduleHash = hashFile(options.inputFiles[0]);
117117
InstrumenterConfig config;
118-
if (options.importNamespace.size()) {
119-
config.importNamespace = options.importNamespace;
118+
if (options.importNamespace) {
119+
config.importNamespace = *options.importNamespace;
120120
}
121-
if (options.secondaryMemoryName.size()) {
122-
config.secondaryMemoryName = options.secondaryMemoryName;
121+
if (options.secondaryMemoryName) {
122+
config.secondaryMemoryName = *options.secondaryMemoryName;
123123
}
124124
config.storageKind = options.storageKind;
125125
config.profileExport = options.profileExport;
@@ -226,14 +226,14 @@ 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()) {
230-
config.importNamespace = options.importNamespace;
229+
if (options.importNamespace) {
230+
config.importNamespace = *options.importNamespace;
231231
}
232-
if (options.exportPrefix.size()) {
233-
config.newExportPrefix = options.exportPrefix;
232+
if (options.exportPrefix) {
233+
config.newExportPrefix = *options.exportPrefix;
234234
}
235-
if (options.placeholderNamespacePrefix.size()) {
236-
config.placeholderNamespacePrefix = options.placeholderNamespacePrefix;
235+
if (options.placeholderNamespacePrefix) {
236+
config.placeholderNamespacePrefix = *options.placeholderNamespacePrefix;
237237
}
238238
}
239239

‎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)