diff options
| author | Thomas Lively <tlively@google.com> | 2024-08-15 15:32:32 -0400 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-08-15 12:32:32 -0700 |
| commit | 033a16ec0d063cbcfb6c67adcf228f70653a1bf5 (patch) | |
| tree | baa6c82aa41b55c05a7c0c4f64110295fdfdc039 /src/wasm/wasm-validator.cpp | |
| parent | d5675780f45f4e2994787fd7dbc5283a6c0162f0 (diff) | |
| download | binaryen-033a16ec0d063cbcfb6c67adcf228f70653a1bf5.tar.gz binaryen-033a16ec0d063cbcfb6c67adcf228f70653a1bf5.tar.bz2 binaryen-033a16ec0d063cbcfb6c67adcf228f70653a1bf5.zip | |
Simplify validation of stale types (#6842)
The previous rules for stale types were complicated and hard to
remember: in general it was ok for result types to be further refinable
as long as they were not refinable all the way to `unreachable`, but
control flow structures had a carve-out and it was ok for them to be
refinable all the way to unreachable.
Simplify the rules so that further refinable result types are always ok,
no matter what they can be refined to and no matter what kind of
instruction is being validated. This will be much easier to remember and
reason about.
This relaxation of the rules strictly increases the set of valid IR, so
no passes or tests need to be updated. It does make it possible for us
to miss type refinement opportunities that previously would have been
validation errors, but only in cases where non-control-flow instructions
could have been refined all the way to unreachable, so the risk seems
small.
Diffstat (limited to 'src/wasm/wasm-validator.cpp')
| -rw-r--r-- | src/wasm/wasm-validator.cpp | 33 |
1 files changed, 9 insertions, 24 deletions
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index b35d1b3be..b032e6bad 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3592,31 +3592,16 @@ static void validateBinaryenIR(Module& wasm, ValidationInfo& info) { auto oldType = curr->type; ReFinalizeNode().visit(curr); auto newType = curr->type; - if (newType != oldType) { - // We accept concrete => undefined on control flow structures: - // e.g. - // - // (drop (block (result i32) (unreachable))) - // - // The block has a type annotated on it, which can make its unreachable - // contents have a concrete type. Refinalize will make it unreachable, - // so both are valid here. - bool validControlFlowStructureChange = - Properties::isControlFlowStructure(curr) && oldType.isConcrete() && - newType == Type::unreachable; - // It's ok in general for types to get refined as long as they don't - // become unreachable. - bool validRefinement = - Type::isSubType(newType, oldType) && newType != Type::unreachable; - if (!validRefinement && !validControlFlowStructureChange) { - std::ostringstream ss; - ss << "stale type found in " << scope << " on " << curr - << "\n(marked as " << oldType << ", should be " << newType - << ")\n"; - info.fail(ss.str(), curr, getFunction()); - } - curr->type = oldType; + // It's ok for types to be further refinable, but they must admit a + // superset of the values allowed by the most precise possible type, i.e. + // they must not be strict subtypes of or unrelated to the refined type. + if (!Type::isSubType(newType, oldType)) { + std::ostringstream ss; + ss << "stale type found in " << scope << " on " << curr + << "\n(marked as " << oldType << ", should be " << newType << ")\n"; + info.fail(ss.str(), curr, getFunction()); } + curr->type = oldType; // check if a node is a duplicate - expressions must not be seen more than // once if (!seen.insert(curr).second) { |