From da23be08da53ef461f8432b062f3429448cb7ad5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 16 Aug 2022 15:21:30 -0700 Subject: Validator: More carefully check for stale types (#4907) The validation logic to check for stale types (code where we forgot to run refinalize) had a workaround for a control flow issue. That workaround meant we didn't catch errors where a type was concrete but it should be unreachable. This PR makes that workaround only apply for control flow structures, so we can catch more errors. --- src/wasm/wasm-validator.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'src/wasm/wasm-validator.cpp') diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 8cf09e747..b7354ff3f 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -2814,15 +2814,19 @@ static void validateBinaryenIR(Module& wasm, ValidationInfo& info) { ReFinalizeNode().visit(curr); auto newType = curr->type; if (newType != oldType) { - // We accept concrete => undefined, + // We accept concrete => undefined on control flow structures: // e.g. // // (drop (block (result i32) (unreachable))) // - // The block has an added type, not derived from the ast itself, so it - // is ok for it to be either i32 or 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; if (!Type::isSubType(newType, oldType) && - !(oldType.isConcrete() && newType == Type::unreachable)) { + !validControlFlowStructureChange) { std::ostringstream ss; ss << "stale type found in " << scope << " on " << curr << "\n(marked as " << oldType << ", should be " << newType -- cgit v1.2.3