FuzzAgainstJS: Do not refine exact to inexact defined types#8855
FuzzAgainstJS: Do not refine exact to inexact defined types#8855kripken wants to merge 1 commit into
Conversation
tlively
left a comment
There was a problem hiding this comment.
I think this could all be much simpler (and more general) if options were a vector of std::pair<HeapType, Exactness>. Then the loop where we populate options could take exactness into account and we wouldn't have to deal with it afterward.
|
I tried that a little but I don't think it avoids complexity? We still need to handle the case of the old type beginning exact, and only allowing exact for the exact heap type in the new type, make sure not to make a basic type exact - i.e. all the same stuff? |
|
Here's what I have in mind: diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp
index 2e1e9281b..00075e05b 100644
--- a/src/tools/fuzzing/fuzzing.cpp
+++ b/src/tools/fuzzing/fuzzing.cpp
@@ -2487,32 +2487,41 @@ void TranslateToFuzzReader::mutateJSBoundary() {
if (new_ == Type::unreachable) {
new_ = Type(old.getHeapType().getBottom(), NonNullable);
}
+ assert(Type::isSubType(new_, old));
// Find all heap types between the old and new, starting from new.
auto oldHeapType = old.getHeapType();
+ auto oldExactness = old.getExactness();
auto newHeapType = new_.getHeapType();
- assert(HeapType::isSubType(newHeapType, oldHeapType));
- std::vector<HeapType> options;
+ auto newExactness = new_.getExactness();
+ std::vector<std::pair<HeapType, Exactness>> options;
while (1) {
- options.push_back(newHeapType);
+ options.push_back({newHeapType, newExactness});
// We cannot look at a bottom type's supers (there can be many, and the
// getSuperType() API doesn't return them), but can use
// interestingHeapSubTypes: any subtype of old is valid.
if (newHeapType.isBottom()) {
for (auto type : interestingHeapSubTypes[oldHeapType]) {
- options.push_back(type);
+ if (type != oldHeapType || oldExactness == Inexact) {
+ options.push_back({type, Inexact});
+ }
+ options.push_back({type, Exact});
}
break;
}
// Continue until we reach the old type.
- if (newHeapType == oldHeapType) {
+ if (newHeapType == oldHeapType && newExactness == oldExactness) {
break;
}
+ if (newExactness == Exact) {
+ newExactness = Inexact;
+ continue;
+ }
auto next = newHeapType.getSuperType();
assert(next);
newHeapType = *next;
}
- newHeapType = pick(options);
+ auto [heapType, exactness] = pick(options);
// Pick the nullability.
auto oldNullability = old.getNullability();
@@ -2521,23 +2530,7 @@ void TranslateToFuzzReader::mutateJSBoundary() {
newNullability = getNullability();
}
- // Pick the exactness.
- auto oldExactness = old.getExactness();
- auto newExactness = new_.getExactness();
- // We can only be exact if we are using the new heap type: that type is
- // exactly what is sent here, and no intermediate heap type would be valid.
- // For example, given $A :> $B :> $C, then maybeRefine($A, exact $C) can
- // return exact $C, but cannot return exact $B.
- //
- // Also, basic heap types cannot be exact.
- if (newHeapType != new_.getHeapType() || newHeapType.isBasic()) {
- newExactness = Inexact;
- } else if (newExactness != oldExactness) {
- // TODO: once getExactness() is fixed (see there), use that
- newExactness = oneIn(2) ? Exact : Inexact;
- }
-
- return Type(newHeapType, newNullability, newExactness);
+ return Type(heapType, newNullability, exactness);
};
// Given a set of types (all params or all results), and an index among them,
This is a net reduction in code. |
|
That is shorter, but iianm, given old type |
|
Sure, I guess I misplaced the new |
It is ok to refine
exact $Atonullref, but not to inexact$B.