Skip to content

Commit 9dbe457

Browse files
authored
Disallow --nominal with GC (#4758)
Nominal types don't make much sense without GC, and in particular trying to emit them with typed function references but not GC enabled can result in invalid binaries because nominal types do not respect the type ordering constraints required by the typed function references proposal. Making this change was mostly straightforward, but required fixing the fuzzer to use --nominal only when GC is enabled and required exiting early from nominal-only optimizations when GC was not enabled. Fixes #4756.
1 parent 7b7e2c5 commit 9dbe457

10 files changed

+41
-30
lines changed

scripts/fuzz_opt.py

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939

4040
# feature options that are always passed to the tools.
4141
CONSTANT_FEATURE_OPTS = ['--all-features']
42-
CONSTANT_FEATURE_OPTS.append(TYPE_SYSTEM_FLAG)
4342

4443
INPUT_SIZE_MIN = 1024
4544
INPUT_SIZE_MEAN = 40 * 1024
@@ -120,6 +119,9 @@ def randomize_feature_opts():
120119
if possible in IMPLIED_FEATURE_OPTS:
121120
FEATURE_OPTS.extend(IMPLIED_FEATURE_OPTS[possible])
122121
print('randomized feature opts:', '\n ' + '\n '.join(FEATURE_OPTS))
122+
# Type system flags only make sense when GC is enabled
123+
if '--disable-gc' not in FEATURE_OPTS:
124+
FEATURE_OPTS.append(TYPE_SYSTEM_FLAG)
123125

124126

125127
ALL_FEATURE_OPTS = ['--all-features', '-all', '--mvp-features', '-mvp']
@@ -819,8 +821,8 @@ def handle_pair(self, input, before_wasm, after_wasm, opts):
819821
b1 = open('b1.wasm', 'rb').read()
820822
b2 = open('b2.wasm', 'rb').read()
821823
if (b1 != b2):
822-
run([in_bin('wasm-dis'), 'b1.wasm', '-o', 'b1.wat', TYPE_SYSTEM_FLAG])
823-
run([in_bin('wasm-dis'), 'b2.wasm', '-o', 'b2.wat', TYPE_SYSTEM_FLAG])
824+
run([in_bin('wasm-dis'), 'b1.wasm', '-o', 'b1.wat', FEATURE_OPTS])
825+
run([in_bin('wasm-dis'), 'b2.wasm', '-o', 'b2.wat', FEATURE_OPTS])
824826
t1 = open('b1.wat', 'r').read()
825827
t2 = open('b2.wat', 'r').read()
826828
compare(t1, t2, 'Output must be deterministic.', verbose=False)
@@ -1379,10 +1381,8 @@ def randomize_opt_flags():
13791381
with open('reduce.sh', 'w') as reduce_sh:
13801382
reduce_sh.write('''\
13811383
# check the input is even a valid wasm file
1382-
echo "At least one of the next two values should be 0:"
1383-
%(wasm_opt)s %(typesystem)s --detect-features %(temp_wasm)s
1384-
echo " " $?
1385-
%(wasm_opt)s %(typesystem)s --all-features %(temp_wasm)s
1384+
echo "The following value should be 0:"
1385+
%(wasm_opt)s %(features)s %(temp_wasm)s
13861386
echo " " $?
13871387
13881388
# run the command
@@ -1419,7 +1419,7 @@ def randomize_opt_flags():
14191419
'auto_init': auto_init,
14201420
'original_wasm': original_wasm,
14211421
'temp_wasm': os.path.abspath('t.wasm'),
1422-
'typesystem': TYPE_SYSTEM_FLAG,
1422+
'features': ' '.join(FEATURE_OPTS),
14231423
'reduce_sh': os.path.abspath('reduce.sh')})
14241424

14251425
print('''\
@@ -1441,17 +1441,16 @@ def randomize_opt_flags():
14411441
vvvv
14421442
14431443
1444-
%(wasm_reduce)s %(type_system_flag)s %(original_wasm)s '--command=bash %(reduce_sh)s' -t %(temp_wasm)s -w %(working_wasm)s
1444+
%(wasm_reduce)s %(features)s %(original_wasm)s '--command=bash %(reduce_sh)s' -t %(temp_wasm)s -w %(working_wasm)s
14451445
14461446
14471447
^^^^
14481448
||||
14491449
14501450
Make sure to verify by eye that the output says something like this:
14511451
1452-
At least one of the next two values should be 0:
1452+
The following value should be 0:
14531453
0
1454-
1
14551454
The following value should be 1:
14561455
1
14571456
@@ -1471,7 +1470,7 @@ def randomize_opt_flags():
14711470
'working_wasm': os.path.abspath('w.wasm'),
14721471
'wasm_reduce': in_bin('wasm-reduce'),
14731472
'reduce_sh': os.path.abspath('reduce.sh'),
1474-
'type_system_flag': TYPE_SYSTEM_FLAG})
1473+
'features': ' '.join(FEATURE_OPTS)})
14751474
break
14761475
if given_seed is not None:
14771476
break

src/passes/ConstantFieldPropagation.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@ struct PCVScanner
176176

177177
struct ConstantFieldPropagation : public Pass {
178178
void run(PassRunner* runner, Module* module) override {
179+
if (!module->features.hasGC()) {
180+
return;
181+
}
179182
if (getTypeSystem() != TypeSystem::Nominal) {
180183
Fatal() << "ConstantFieldPropagation requires nominal typing";
181184
}

src/passes/GlobalRefining.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ namespace {
3232

3333
struct GlobalRefining : public Pass {
3434
void run(PassRunner* runner, Module* module) override {
35+
if (!module->features.hasGC()) {
36+
return;
37+
}
3538
if (getTypeSystem() != TypeSystem::Nominal) {
3639
Fatal() << "GlobalRefining requires nominal typing";
3740
}

src/passes/GlobalStructInference.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ struct GlobalStructInference : public Pass {
6262
std::unordered_map<HeapType, std::vector<Name>> typeGlobals;
6363

6464
void run(PassRunner* runner, Module* module) override {
65+
if (!module->features.hasGC()) {
66+
return;
67+
}
6568
if (getTypeSystem() != TypeSystem::Nominal) {
6669
Fatal() << "GlobalStructInference requires nominal typing";
6770
}

src/passes/GlobalTypeOptimization.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ struct GlobalTypeOptimization : public Pass {
112112
std::unordered_map<HeapType, std::vector<Index>> indexesAfterRemovals;
113113

114114
void run(PassRunner* runner, Module* module) override {
115+
if (!module->features.hasGC()) {
116+
return;
117+
}
115118
if (getTypeSystem() != TypeSystem::Nominal) {
116119
Fatal() << "GlobalTypeOptimization requires nominal typing";
117120
}

src/passes/SignaturePruning.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ struct SignaturePruning : public Pass {
4848
std::unordered_map<HeapType, Signature> newSignatures;
4949

5050
void run(PassRunner* runner, Module* module) override {
51+
if (!module->features.hasGC()) {
52+
return;
53+
}
5154
if (getTypeSystem() != TypeSystem::Nominal) {
5255
Fatal() << "SignaturePruning requires nominal typing";
5356
}

src/passes/SignatureRefining.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ struct SignatureRefining : public Pass {
4848
std::unordered_map<HeapType, Signature> newSignatures;
4949

5050
void run(PassRunner* runner, Module* module) override {
51+
if (!module->features.hasGC()) {
52+
return;
53+
}
5154
if (getTypeSystem() != TypeSystem::Nominal) {
5255
Fatal() << "SignatureRefining requires nominal typing";
5356
}

src/passes/TypeRefining.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ struct TypeRefining : public Pass {
7878
StructUtils::StructValuesMap<FieldInfo> finalInfos;
7979

8080
void run(PassRunner* runner, Module* module) override {
81+
if (!module->features.hasGC()) {
82+
return;
83+
}
8184
if (getTypeSystem() != TypeSystem::Nominal) {
8285
Fatal() << "TypeRefining requires nominal typing";
8386
}

src/tools/tool-options.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ struct ToolOptions : public Options {
176176
void applyFeatures(Module& module) const {
177177
module.features.enable(enabledFeatures);
178178
module.features.disable(disabledFeatures);
179+
// Non-default type systems only make sense with GC enabled. TODO: Error on
180+
// non-GC equirecursive types as well once we make isorecursive the default
181+
// if we don't remove equirecursive types entirely.
182+
if (!module.features.hasGC() && getTypeSystem() == TypeSystem::Nominal) {
183+
Fatal() << "Nominal typing is only allowed when GC is enabled";
184+
}
179185
}
180186

181187
private:

test/lit/nominal-no-gc.wast

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,5 @@
1-
;; Write the module with --nominal but without GC
2-
;; RUN: wasm-opt %s --nominal --disable-gc -g -o %t.wasm
1+
;; Using --nominal without GC is not allowed.
32

4-
;; We should not get any recursion groups even though we used --nominal. We use
5-
;; --hybrid -all here to make sure that any rec groups from the binary will
6-
;; actually show up in the output and cause the test to fail.
7-
;; RUN: wasm-opt %t.wasm --hybrid -all -S -o - | filecheck %s
3+
;; RUN: not wasm-opt %s --nominal --disable-gc -g -o %t.wasm 2>&1 | filecheck %s
84

9-
;; Also check that we don't get a failure with the default configuration.
10-
;; RUN: wasm-opt %t.wasm
11-
12-
;; CHECK-NOT: rec
13-
14-
(module
15-
(type $foo (func))
16-
(type $bar (func))
17-
18-
(func $f1 (type $foo))
19-
(func $f2 (type $bar))
20-
)
5+
;; CHECK: Nominal typing is only allowed when GC is enabled

0 commit comments

Comments
 (0)