Print and compare prototypes in fuzzer interpreter#8457
Print and compare prototypes in fuzzer interpreter#8457
Conversation
GTO already handled keeping the prototype configuration fields on descriptors of types that flowed out to JS via @binaryen.js.called functions, but it did not handle propagating the information that a type flows out to JS to that type's subtypes. It also did not consider that types may flow out to JS via imports and exports. Fix these issues.
When a struct flows out to JS, if it has a descriptor and that desriptor's first field is an externref, that field's value becomes the JS prototype of the struct. There is a class of bugs where we misoptimize something in this setup so that the JS-observable prototype changes. To catch those bugs in the fuzzer, update the fuzzer interpreter and fuzz_shell.js to print the prototypes of objects. This lets the fuzzer make sure that engines like V8 and interpreter agree on whether there is a prototype. Also update the fuzzer interpreter to compare configured prototypes when comparing two execution traces. This lets --fuzz-exec detect when optimizations have changed a prototype.
| } | ||
| // The environment always sees externalized references and is able to | ||
| // observe the difference between external references and externalized | ||
| // internal references. Make sure this is accounted for below by unrapping |
There was a problem hiding this comment.
| // internal references. Make sure this is accounted for below by unrapping | |
| // internal references. Make sure this is accounted for below by unwrapping |
| std::ostream& operator<<(std::ostream& o, const WasmException& exn) { | ||
| auto exnData = exn.exn.getExnData(); | ||
| return o << exnData->tag->name << " " << exnData->payload; | ||
| } |
There was a problem hiding this comment.
This was only used in two places: wasm-ctor-eval and execution-results. The latter was updated to use its own printValue function to print exception payloads, so I just inlined this original method into the single remaining use site in wasm-ctor-eval.
| ;; CHECK-NEXT: [exception thrown: A [ref (type $array.0 (array (mut i32))) (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0[..])]] | ||
| ;; CHECK-NEXT: [exception thrown: A object(null)] | ||
| (func $array (export "array") (result (ref $A)) | ||
| ;; Throw a very large array. We should not print all 12K items in it, as that |
There was a problem hiding this comment.
Comment needs updating. But why is the new output better?
There was a problem hiding this comment.
The new output matches what will be printed by fuzz_shell.js. I figure that the less they differ, the simpler.
There was a problem hiding this comment.
It is simpler, yeah. We are losing something though, when the fuzzer compares its logging to itself after opts. See in fuzz_opt.py where we simplify the output for comparison reasons, e.g.
# funcref(0) has the index of the function in it, and optimizations can
# change that index, so ignore it
out = re.sub(r'funcref\([\d\w$+-_:]+\)', 'funcref()', out)I think we could do a similar thing for this?
There was a problem hiding this comment.
Yes, we could do something similar, but I think it's better not to. The input "fixing" is somewhat complex in the fuzzer, and bugs there can mean missing real bugs. I think it would be better to try to reduce the amount of fixing that happens in fuzz_opt.py. Also, I contend that if we ignore some difference in the fuzzer (and when comparing interpreter executions), then it's clearer if that difference doesn't show up in the textual log, either.
There was a problem hiding this comment.
We only ignore it when comparing VMs, though. We can use the differences when comparing a VM to itself.
There was a problem hiding this comment.
Ok, the alternative here would be to keep the existing printing and additionally add new printing for the prototypes, then to update fuzz_opt.py as necessary to filter that out as well. Is that what you would prefer?
I really do think it would be attractive to have fuzz_shell.js and execution-results produce identical logging, though :/
There was a problem hiding this comment.
I'm more worried about losing fuzzer coverage. But maybe there is a better way to do it. Does --fuzz-exec compare contents more deeply than what we print? That is, if the equality comparison in execution-results.h looks into objects when it can, that might be good enough.
| !a.type.getHeapType().isMaybeShared(HeapType::i31)) { | ||
| return true; | ||
| // TODO: We could compare more information when we know it will be | ||
| // externally visible, for example when the type of the value is public. |
There was a problem hiding this comment.
I guess this TODO covers my last comment. Would it be hard to do, do you think?
There was a problem hiding this comment.
No, gathering the public types should be easy, and then we could find the most specific public supertype of a GC value and then print the contents visible based on that type. How about I do that as a follow-up? This PR already makes areEqual more discerning than it was before, so there should not even be a temporary regression in coverage.
There was a problem hiding this comment.
Plan sgtm.
Though I do think we'd have a temporary regression - coverage in fuzz_opt would decrease as execution-results improves - but I think it's fine, and the execution-results improvements later would more than make up for it.
|
Sounds good. I'll hold off on landing this until it can get through 100k iterations, but I'll start working on the follow-up. |
2 participants
When a struct flows out to JS, if it has a descriptor and that desriptor's first field is an externref, that field's value becomes the JS prototype of the struct. There is a class of bugs where we misoptimize something in this setup so that the JS-observable prototype changes. To catch those bugs in the fuzzer, update the fuzzer interpreter and fuzz_shell.js to print the prototypes of objects. This lets the fuzzer make sure that engines like V8 and interpreter agree on whether there is a prototype. Also update the fuzzer interpreter to compare configured prototypes when comparing two execution traces. This lets --fuzz-exec detect when optimizations have changed a prototype.