Skip to content

Commit 2e51491

Browse files
authored
Add Features enum to IR (#1250)
This enum describes which wasm features the IR is expected to include. The validator should reject operations which require excluded features, and passes should avoid producing IR which requires excluded features. This makes it easier to catch possible errors in Binaryen producers (e.g. emscripten). Asm2wasm has a flag to enable or disable atomics. Other tools currently just accept all features (as, dis and opt are just for inspecting or modifying existing modules, so it would be annoying to have to use flags with those tools and I expect the risk of accidentally introducing atomics to be low).
1 parent 9d409e1 commit 2e51491

12 files changed

Lines changed: 43 additions & 12 deletions

File tree

scripts/test/asm2wasm.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ def test_asm2wasm():
3333
for precise in [0, 1, 2]:
3434
for opts in [1, 0]:
3535
cmd = ASM2WASM + [os.path.join(options.binaryen_test, asm)]
36+
if 'threads' in asm:
37+
cmd += ['--enable-threads']
3638
wasm = asm.replace('.asm.js', '.fromasm')
3739
if not precise:
3840
cmd += ['--trap-mode=allow', '--ignore-implicit-traps']

src/asm2wasm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,6 @@ class Asm2WasmBuilder {
379379

380380
Asm2WasmPreProcessor& preprocessor;
381381
bool debug;
382-
383382
TrapMode trapMode;
384383
TrappingFunctionContainer trappingFunctions;
385384
PassOptions passOptions;
@@ -1286,6 +1285,7 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
12861285
};
12871286

12881287
PassRunner passRunner(&wasm);
1288+
passRunner.setFeatures(passOptions.features);
12891289
if (debug) {
12901290
passRunner.setDebug(true);
12911291
passRunner.setValidateGlobally(false);

src/pass.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ struct PassOptions {
6262
int shrinkLevel = 0; // 0, 1, 2 correspond to -O0, -Os, -Oz
6363
bool ignoreImplicitTraps = false; // optimize assuming things like div by 0, bad load/store, will not trap
6464
bool debugInfo = false; // whether to try to preserve debug info through, which are special calls
65+
FeatureSet features = Feature::MVP; // Which wasm features to accept, and be allowed to use
6566
};
6667

6768
//
@@ -87,6 +88,9 @@ struct PassRunner {
8788
void setValidateGlobally(bool validate) {
8889
options.validateGlobally = validate;
8990
}
91+
void setFeatures(FeatureSet features) {
92+
options.features = features;
93+
}
9094

9195
void add(std::string passName) {
9296
auto pass = PassRegistry::get()->createPass(passName);

src/passes/pass.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ void PassRunner::run() {
231231
totalTime += diff;
232232
// validate, ignoring the time
233233
std::cerr << "[PassRunner] (validating)\n";
234-
if (!WasmValidator().validate(*wasm, validationFlags)) {
234+
if (!WasmValidator().validate(*wasm, options.features, validationFlags)) {
235235
if (passDebug >= 2) {
236236
std::cerr << "Last pass (" << pass->name << ") broke validation. Here is the module before: \n" << moduleBefore.str() << "\n";
237237
} else {
@@ -246,7 +246,7 @@ void PassRunner::run() {
246246
std::cerr << "[PassRunner] passes took " << totalTime.count() << " seconds." << std::endl;
247247
// validate
248248
std::cerr << "[PassRunner] (final validation)\n";
249-
if (!WasmValidator().validate(*wasm, validationFlags)) {
249+
if (!WasmValidator().validate(*wasm, options.features, validationFlags)) {
250250
std::cerr << "final module does not validate\n";
251251
abort();
252252
}

src/tools/asm2wasm.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ int main(int argc, const char *argv[]) {
115115
.add("--emit-text", "-S", "Emit text instead of binary for the output file",
116116
Options::Arguments::Zero,
117117
[&](Options *o, const std::string &argument) { emitBinary = false; })
118+
.add("--enable-threads", "-a", "Enable the Atomics wasm feature",
119+
Options::Arguments::Zero,
120+
[&](Options *o, const std::string &argument) { options.passOptions.features |= Feature::Atomics; })
118121
.add_positional("INFILE", Options::Arguments::One,
119122
[](Options *o, const std::string &argument) {
120123
o->extra["infile"] = argument;
@@ -176,6 +179,7 @@ int main(int argc, const char *argv[]) {
176179
wasm.memory.segments.emplace_back(init, data);
177180
if (options.runningDefaultOptimizationPasses()) {
178181
PassRunner runner(&wasm);
182+
runner.setFeatures(options.features);
179183
runner.add("memory-packing");
180184
runner.run();
181185
}
@@ -202,7 +206,7 @@ int main(int argc, const char *argv[]) {
202206
}
203207
}
204208

205-
if (!WasmValidator().validate(wasm)) {
209+
if (!WasmValidator().validate(wasm, options.passOptions.features)) {
206210
Fatal() << "error in validating output";
207211
}
208212

src/tools/optimization-options.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ struct OptimizationOptions : public Options {
2525

2626
std::vector<std::string> passes;
2727
PassOptions passOptions;
28+
FeatureSet features = Feature::Atomics;
2829

2930
OptimizationOptions(const std::string &command, const std::string &description) : Options(command, description) {
3031
(*this).add("", "-O", "execute default optimization passes",
@@ -118,6 +119,7 @@ struct OptimizationOptions : public Options {
118119
void runPasses(Module& wasm) {
119120
PassRunner passRunner(&wasm, passOptions);
120121
if (debug) passRunner.setDebug(true);
122+
passRunner.setFeatures(features);
121123
for (auto& pass : passes) {
122124
if (pass == DEFAULT_OPT_PASSES) {
123125
passRunner.addDefaultOptimizationPasses();
@@ -130,4 +132,3 @@ struct OptimizationOptions : public Options {
130132
};
131133

132134
} // namespace wasm
133-

src/tools/wasm-as.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ int main(int argc, const char *argv[]) {
9292

9393
if (options.extra["validate"] != "none") {
9494
if (options.debug) std::cerr << "Validating..." << std::endl;
95-
if (!wasm::WasmValidator().validate(wasm,
95+
if (!wasm::WasmValidator().validate(wasm, Feature::All,
9696
WasmValidator::Globally | (options.extra["validate"] == "web" ? WasmValidator::Web : 0))) {
9797
Fatal() << "Error: input module is not valid.\n";
9898
}

src/tools/wasm-opt.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ int main(int argc, const char* argv[]) {
110110
options.parse(argc, argv);
111111

112112
Module wasm;
113+
// It should be safe to just always enable atomics in wasm-opt, because we
114+
// don't expect any passes to accidentally generate atomic ops
115+
FeatureSet features = Feature::Atomics;
113116

114117
if (options.debug) std::cerr << "reading...\n";
115118

@@ -125,14 +128,14 @@ int main(int argc, const char* argv[]) {
125128
Fatal() << "error in building module, std::bad_alloc (possibly invalid request for silly amounts of memory)";
126129
}
127130

128-
if (!WasmValidator().validate(wasm)) {
131+
if (!WasmValidator().validate(wasm, features)) {
129132
Fatal() << "error in validating input";
130133
}
131134
} else {
132135
// translate-to-fuzz
133136
TranslateToFuzzReader reader(wasm);
134137
reader.read(options.extra["infile"]);
135-
if (!WasmValidator().validate(wasm)) {
138+
if (!WasmValidator().validate(wasm, features)) {
136139
std::cerr << "translate-to-fuzz must always generate a valid module";
137140
abort();
138141
}
@@ -173,7 +176,7 @@ int main(int argc, const char* argv[]) {
173176
if (options.runningPasses()) {
174177
if (options.debug) std::cerr << "running passes...\n";
175178
options.runPasses(wasm);
176-
assert(WasmValidator().validate(wasm));
179+
assert(WasmValidator().validate(wasm, features));
177180
}
178181

179182
if (fuzzExec) {
@@ -189,7 +192,7 @@ int main(int argc, const char* argv[]) {
189192
auto input = buffer.getAsChars();
190193
WasmBinaryBuilder parser(second, input, false);
191194
parser.read();
192-
assert(WasmValidator().validate(second));
195+
assert(WasmValidator().validate(second, features));
193196
}
194197
results.check(*compare);
195198
}

src/wasm-printing.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ namespace wasm {
2727
struct WasmPrinter {
2828
static std::ostream& printModule(Module* module, std::ostream& o) {
2929
PassRunner passRunner(module);
30+
passRunner.setFeatures(Feature::All);
3031
passRunner.setIsNested(true);
3132
passRunner.add<Printer>(&o);
3233
passRunner.run();

src/wasm-validator.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ struct WasmValidator {
5757
};
5858
typedef uint32_t Flags;
5959

60-
bool validate(Module& module, Flags flags = Globally);
60+
bool validate(Module& module, FeatureSet features = MVP, Flags flags = Globally);
6161
};
6262

6363
} // namespace wasm

0 commit comments

Comments
 (0)