diff options
author | Thomas Lively <tlively@google.com> | 2023-08-17 13:57:23 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-17 13:57:23 -0700 |
commit | 67dd6f7db409f9ab7171e97db9da2a4e01a5dc4b (patch) | |
tree | c063c06898fa1430c38715b5b05ed0cfec036f37 /src | |
parent | c39ca2e1cde95b6fcef6cdfeb9326dadd75e55df (diff) | |
download | binaryen-67dd6f7db409f9ab7171e97db9da2a4e01a5dc4b.tar.gz binaryen-67dd6f7db409f9ab7171e97db9da2a4e01a5dc4b.tar.bz2 binaryen-67dd6f7db409f9ab7171e97db9da2a4e01a5dc4b.zip |
Ensure br_on_cast* target type is subtype of input type (#5881)
The WasmGC spec will require that the target cast type of br_on_cast and
br_on_cast_fail be a subtype of the input type, but so far Binaryen has not
enforced this constraint, so it could produce invalid modules when optimizations
refined the input to a br_on_cast* such that it was no longer a supertype of the
cast target type.
Fix this problem by setting the cast target type to be the greatest lower bound
of the original cast target type and the current input type in
`BrOn::finalize()`. This maintains the invariant that the cast target type
should be a subtype of the input type and it also does not change cast behavior;
any value that could make the original cast succeed at runtime necessarily
inhabits both the original cast target type and the input type, so it also must
inhabit their greatest lower bound and will make the updated cast succeed as
well.
Diffstat (limited to 'src')
-rw-r--r-- | src/wasm/wasm-binary.cpp | 3 | ||||
-rw-r--r-- | src/wasm/wasm-s-parser.cpp | 6 | ||||
-rw-r--r-- | src/wasm/wasm-stack.cpp | 1 | ||||
-rw-r--r-- | src/wasm/wasm-validator.cpp | 5 | ||||
-rw-r--r-- | src/wasm/wasm.cpp | 7 |
5 files changed, 22 insertions, 0 deletions
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 98e832225..0d405251e 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -7028,6 +7028,9 @@ bool WasmBinaryReader::maybeVisitBrOn(Expression*& out, uint32_t code) { auto castHeapType = getHeapType(); castType = Type(castHeapType, castNullability); auto inputType = Type(inputHeapType, inputNullability); + if (!Type::isSubType(castType, inputType)) { + throwError("br_on_cast* cast type must be subtype of input type"); + } if (!Type::isSubType(ref->type, inputType)) { throwError(std::string("Invalid reference type for ") + ((op == BrOnCast) ? "br_on_cast" : "br_on_cast_fail")); diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 0115cdac3..0519afd83 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -2880,6 +2880,12 @@ Expression* SExpressionWasmBuilder::makeBrOnCast(Element& s, bool onFail) { auto name = getLabel(*s[i++]); auto inputType = elementToType(*s[i++]); auto castType = elementToType(*s[i++]); + if (!Type::isSubType(castType, inputType)) { + throw ParseException( + "br_on_cast* cast type must be a subtype of its input type", + s.line, + s.col); + } auto* ref = parseExpression(*s[i]); if (!Type::isSubType(ref->type, inputType)) { throw ParseException( diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp index 3b7756992..115013307 100644 --- a/src/wasm/wasm-stack.cpp +++ b/src/wasm/wasm-stack.cpp @@ -2048,6 +2048,7 @@ void BinaryInstWriter::visitBrOn(BrOn* curr) { o << U32LEB(BinaryConsts::BrOnCastFail); } assert(curr->ref->type.isRef()); + assert(Type::isSubType(curr->castType, curr->ref->type)); uint8_t flags = (curr->ref->type.isNullable() ? 1 : 0) | (curr->castType.isNullable() ? 2 : 0); o << flags; diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 9c1f9fdbb..b48ec632e 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -2583,6 +2583,11 @@ void FunctionValidator::visitBrOn(BrOn* curr) { curr->ref->type.getHeapType().getBottom(), curr, "br_on_cast* target type and ref type must have a common supertype"); + shouldBeSubType( + curr->castType, + curr->ref->type, + curr, + "br_on_cast* target type must be a subtype of its input type"); } else { shouldBeEqual(curr->castType, Type(Type::none), diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index 552ec7e57..648a25d9e 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -972,6 +972,13 @@ void BrOn::finalize() { type = Type::unreachable; return; } + if (op == BrOnCast || op == BrOnCastFail) { + // The cast type must be a subtype of the input type. If we've refined the + // input type so that this is no longer true, we can fix it by similarly + // refining the cast type in a way that will not change the cast behavior. + castType = Type::getGreatestLowerBound(castType, ref->type); + assert(castType.isRef()); + } switch (op) { case BrOnNull: // If we do not branch, we flow out the existing value as non-null. |