Disallow --nominal with GC #4758
Conversation
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.
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
scripts/fuzz_opt.py
Outdated
| @@ -120,6 +119,9 @@ def randomize_feature_opts(): | |||
| if possible in IMPLIED_FEATURE_OPTS: | |||
| FEATURE_OPTS.extend(IMPLIED_FEATURE_OPTS[possible]) | |||
| print('randomized feature opts:', '\n ' + '\n '.join(FEATURE_OPTS)) | |||
| # Type system flags only make sense when GC is enabled | |||
| if "--disable-gc" not in FEATURE_OPTS: | |||
There was a problem hiding this comment.
| if "--disable-gc" not in FEATURE_OPTS: | |
| if '--disable-gc' not in FEATURE_OPTS: |
| @@ -176,6 +176,9 @@ struct PCVScanner | |||
|
|
|||
| struct ConstantFieldPropagation : public Pass { | |||
| void run(PassRunner* runner, Module* module) override { | |||
| if (!module->features.hasGC()) { | |||
| return; | |||
| } | |||
There was a problem hiding this comment.
I guess this is needed now because the fuzzer may call this pass without --nominal, and we want to avoid the Fatal below? It is odd though to silently return on not having GC but error on nominal. How about making the fuzzer not run these passes? We could move the relevant passes from opt_choices to gc_opt_choices and only pick from the latter when gc (+ nominal) is enabled?
There was a problem hiding this comment.
Yeah that would be another good solution. I slightly prefer keeping these early returns because they seem like what we will want to do in the long term; if running GC optimization passes on non-GC modules gracefully does nothing, then we can run GC optimization passes by default without a problem. I hope that the fatal errors on nonsupported type systems are more temporary and that we eventually update the passes to do what they can with whatever type systems we choose to permanently maintain.
There was a problem hiding this comment.
I guess both are reasonable. I'd prefer to not run GC passes by default, though, as they add clutter - for example in debug mode they'd get printed out in the list. Seems nicer to only print out the relevant passes, which we do now. Maybe we can add a mechanism to avoid that, though, like a hook "canRun" that would avoid even adding the pass - that would avoid needing to skip the pass both in pass.cpp and in the fuzzer.
There was a problem hiding this comment.
Anyhow, this lgtm + filing an issue to figure out a better solution later.
#4758 regressed the determinism fuzzer. First, FEATURE_OPTS is not a list but a string. Second, as a hack another location would add --strip-dwarf to that list, but that only works in wasm-opt. To fix that, remove the hack and instead just ignore DWARF-containing files.
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.