-
Notifications
You must be signed in to change notification settings - Fork 873
Type ref.null as exact
#7371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Type ref.null as exact
#7371
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1570,20 +1570,22 @@ void TranslateToFuzzReader::recombine(Function* func) { | |
| } | ||
|
|
||
| std::vector<Type> ret; | ||
| auto heapType = type.getHeapType(); | ||
| auto nullability = type.getNullability(); | ||
| ret.push_back(type); | ||
|
|
||
| if (nullability == NonNullable) { | ||
| ret = getRelevantTypes(Type(heapType, Nullable)); | ||
| if (type.isNonNullable()) { | ||
| auto nullable = getRelevantTypes(type.with(Nullable)); | ||
| ret.insert(ret.end(), nullable.begin(), nullable.end()); | ||
| } | ||
| if (type.isExact()) { | ||
| auto inexact = getRelevantTypes(type.with(Inexact)); | ||
| ret.insert(ret.end(), inexact.begin(), inexact.end()); | ||
| // Do not consider exact references to supertypes. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh right 😆 |
||
| return ret; | ||
| } | ||
|
|
||
| while (1) { | ||
| ret.push_back(Type(heapType, nullability)); | ||
| auto super = heapType.getSuperType(); | ||
| if (!super) { | ||
| break; | ||
| } | ||
| heapType = *super; | ||
| for (auto heapType = type.getHeapType().getSuperType(); heapType; | ||
| heapType = heapType->getSuperType()) { | ||
| ret.push_back(type.with(*heapType)); | ||
| } | ||
|
|
||
| return ret; | ||
|
|
@@ -4906,9 +4908,17 @@ static auto makeArrayBoundsCheck(Expression* ref, | |
| Function* func, | ||
| Builder& builder, | ||
| Expression* length = nullptr) { | ||
| auto tempRef = builder.addVar(func, ref->type); | ||
| // The reference might be a RefNull, in which case its type is exact. But we | ||
| // want to avoid creating exact-typed locals until we support them more widely | ||
| // in the fuzzer, so adjust the type. TODO: remove this once exact references | ||
| // are better supported. | ||
| Type refType = ref->type; | ||
| if (refType.isExact()) { | ||
| refType = refType.with(Inexact); | ||
| } | ||
| auto tempRef = builder.addVar(func, refType); | ||
| auto tempIndex = builder.addVar(func, index->type); | ||
| auto* teeRef = builder.makeLocalTee(tempRef, ref, ref->type); | ||
| auto* teeRef = builder.makeLocalTee(tempRef, ref, refType); | ||
| auto* teeIndex = builder.makeLocalTee(tempIndex, index, index->type); | ||
| auto* getSize = builder.makeArrayLen(teeRef); | ||
|
|
||
|
|
@@ -4935,7 +4945,7 @@ static auto makeArrayBoundsCheck(Expression* ref, | |
| // An additional use of the length, if it was provided. | ||
| Expression* getLength = nullptr; | ||
| } result = {builder.makeBinary(LtUInt32, effectiveIndex, getSize), | ||
| builder.makeLocalGet(tempRef, ref->type), | ||
| builder.makeLocalGet(tempRef, refType), | ||
| builder.makeLocalGet(tempIndex, index->type), | ||
| getLength}; | ||
| return result; | ||
|
|
@@ -5324,13 +5334,37 @@ Nullability TranslateToFuzzReader::getNullability() { | |
| return Nullable; | ||
| } | ||
|
|
||
| Exactness TranslateToFuzzReader::getExactness() { | ||
| // Without GC, the only heap types are func and extern, neither of which is | ||
| // exactly inhabitable. To avoid introducing uninhabitable types, only | ||
| // generate exact references when GC is enabled. We don't need custom | ||
| // descriptors to be enabled even though that is the feature that introduces | ||
| // exact references because the binary writer can always generalize the exact | ||
| // reference types away. | ||
| // | ||
| // if (wasm.features.hasGC() && oneIn(8)) { | ||
| // return Exact; | ||
| // } | ||
| // | ||
| // However, we cannot yet handle creating exact references in general, so for | ||
| // now we always generate inexact references when given the choice. TODO. | ||
| return Inexact; | ||
| } | ||
|
|
||
| Nullability TranslateToFuzzReader::getSubType(Nullability nullability) { | ||
| if (nullability == NonNullable) { | ||
| return NonNullable; | ||
| } | ||
| return getNullability(); | ||
| } | ||
|
|
||
| Exactness TranslateToFuzzReader::getSubType(Exactness exactness) { | ||
| if (exactness == Exact) { | ||
| return Exact; | ||
| } | ||
| return getExactness(); | ||
| } | ||
|
|
||
| HeapType TranslateToFuzzReader::getSubType(HeapType type) { | ||
| if (oneIn(3)) { | ||
| return type; | ||
|
|
@@ -5422,9 +5456,18 @@ Type TranslateToFuzzReader::getSubType(Type type) { | |
| if (!funcContext && heapType.isMaybeShared(HeapType::exn)) { | ||
| return type; | ||
| } | ||
| heapType = getSubType(heapType); | ||
| if (type.isExact()) { | ||
| // The only other possible heap type is bottom, but we don't want to | ||
| // generate too many bottom types. | ||
| if (!heapType.isBottom() && oneIn(20)) { | ||
| heapType = heapType.getBottom(); | ||
| } | ||
| } else { | ||
| heapType = getSubType(heapType); | ||
| } | ||
| auto nullability = getSubType(type.getNullability()); | ||
| auto subType = Type(heapType, nullability); | ||
| auto exactness = getSubType(type.getExactness()); | ||
| auto subType = Type(heapType, nullability, exactness); | ||
| // We don't want to emit lots of uninhabitable types like (ref none), so | ||
| // avoid them with high probability. Specifically, if the original type was | ||
| // inhabitable then return that; avoid adding more uninhabitability. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. | ||
|
|
||
| ;; RUN: wasm-ctor-eval %s -all --ctors=test --kept-exports=test --ignore-external-input -S -o - \ | ||
| ;; RUN: | filecheck %s | ||
|
|
||
| ;; Check that materializing a non-data null local, which at time of writing uses | ||
| ;; Builder::makeConstantExpression, does not trigger an assertion failure. | ||
|
|
||
| (module | ||
| (func $test (export "test") (param $0 externref) | ||
| (local $3 anyref) | ||
| (local.set $3 | ||
| (any.convert_extern | ||
| (local.get $0) | ||
| ) | ||
| ) | ||
| ) | ||
| ) | ||
| ;; CHECK: (type $0 (func (param externref))) | ||
|
|
||
| ;; CHECK: (export "test" (func $test_1)) | ||
|
|
||
| ;; CHECK: (func $test_1 (type $0) (param $0 externref) | ||
| ;; CHECK-NEXT: (local $3 anyref) | ||
| ;; CHECK-NEXT: (nop) | ||
| ;; CHECK-NEXT: ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| ;; NOTE: Assertions have been generated by update_lit_checks.py --output=fuzz-exec and should not be edited. | ||
|
|
||
| ;; RUN: wasm-opt %s -all --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s | ||
|
|
||
| (module | ||
| ;; CHECK: [fuzz-exec] calling convert-null-extern | ||
| ;; CHECK-NEXT: [fuzz-exec] note result: convert-null-extern => null | ||
| (func $convert-null-extern (export "convert-null-extern") (result (exact nullref)) | ||
| (local externref) | ||
| ;; The value produced by this cast must be exact to avoid triggering an | ||
| ;; assertion. | ||
| (ref.cast (exact nullref) | ||
| (any.convert_extern | ||
| (local.get 0) | ||
| ) | ||
| ) | ||
| ) | ||
| ) | ||
| ;; CHECK: [fuzz-exec] calling convert-null-extern | ||
| ;; CHECK-NEXT: [fuzz-exec] note result: convert-null-extern => null | ||
| ;; CHECK-NEXT: [fuzz-exec] comparing convert-null-extern |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fuzzer does not seem to trip over this (yet), so I left it unfixed for now. Eventually I expect us to trip over this bug, at which point we will be able to reduce the reproducer to a test case and ensure we have test coverage.