Fuzz binaryen.js.called in closed-world#8343
Conversation
|
Verified by making |
| // We cannot actually use this as jsCalled if it does not have a type | ||
| // compatible with the callRef* imports. They send a funcref, so we must | ||
| // only send non-shared functions. | ||
| if (!func->type.getHeapType().isShared()) { |
There was a problem hiding this comment.
Why not check this before attaching the annotation?
There was a problem hiding this comment.
It is still ok to add the annotation. And perhaps that might find bugs by itself (e.g. if a pass crashes on handling the annotation).
| Expression* TranslateToFuzzReader::makeCallRef(Type type) { | ||
| // look for a call target with the right type |
There was a problem hiding this comment.
Why aren't we just leaving this to makeRefFunc?
There was a problem hiding this comment.
Leaving what? The old logic in makeCallRef on this line, or the new logic for avoiding bad RefFuncs?
There was a problem hiding this comment.
Not related to this PR, but why are we choosing a call target in makeCallRef rather than just choosing a call target type and then calling make with that type? As it is, it seems much harder than it should be for us to generate e.g. reading the call target out of a table or out of a struct field since we're just generating a ref.func directly.
There was a problem hiding this comment.
rather than just choosing a call target type and then calling make with that type?
I think the current logic aims to avoid traps. If we called make with a function type, make might fail to find such a type (if no such function exists).
There was a problem hiding this comment.
Could we look through the functions to find the available types, then call make with one of them? That would ensure there's always a possible function to target (assuming the generator doesn't decide to make a subtype instead, etc).
There was a problem hiding this comment.
That is essentially what the code does right now, except that it makes a specific RefFunc rather than call make. That is less likely to trap (I'm not sure make will always find a suitable function, it might give up and emit unreachable). There is a TODO though a few lines below to generalize that,
// TODO: half the time make a completely random item with that type.
Co-authored-by: Thomas Lively <tlively123@gmail.com>
|
Found a real bug, #8345 |
2 participants
Closed-world fuzzing disabled the
callRef*imports until now. Those importsget a wasm funcref and call it from JS, so they break the closed world
assumption. This PR enables
callRef*imports in that mode, by onlysending them valid functions, ones marked with
@binaryen.js.called.To do this in the fuzzer,
callRef*calls andensuring they are given a valid funcref, not a random value.
callRef*indirectly in closed world. We have nogood way to avoid a random funcref being sent out there, as we
do not see those calls statically, so we can't fix them up.