PTAL

I'll take a look. This may be actually expected behavior of the code serializer. Are we OK with that, or do we want to harden APIs against hard failures?

Prior to my change, this run was failing, but differently. In either case, failure is the expected outcome. 

Here is what causes a difference in where the serializer fails: before my change, asm.js, converted to wasm, was internally captured as the FixedArray representing the compiled module data. The FixedArray is serializable, but then, when trying to serialize wasm functions, serialization would fail.

With my change, we represent asm.js compiled code as a module object (a JSObject), which references the above FixedArray. This is matching what we do for wasm, where the JSObject is the user (js) facing handle, exposing the public Wasm Module APIs, while the FixedArray is what we need to support creating new instances (the two have different lifetimes, too).

This JSObject fails the !obj->IsMap() check in the serializer, so a bit earlier than before.

Reopening:

I don't think a failure should be the expected outcome. There is some leeway regarding priority.

If your CL didn't cause the crash, it would be great if you could find a proper owner.

Added Daniel. Daniel, do you own the code serializer?

"Daniel, do you own the code serializer?" Not really. Yang owns it, but I'll be happy to do code reviews in his absence. That's why I'm in the OWNERS file in the first place.



If I understand this correctly, the snapshot must be independent of the context, and context-dependent objects-to-be-serialized must be either specially treated or must not be present in the first place. The check !obj->IsMap() asserts that, at that point, no context-dependent objects (Maps) are expected. If there's one anyway, that's not really the fault of the serializer.


If the intention is that code as in the example shouldn't be snapshotted anyways, then the serializer should not produce a code cache at all, rather than crashing.

If the intention is that the WASM API object be re-created by the de-serializer, then it would presumably need a treatment similar to the root objects, where a suitable tag gets serialized, and on de-serialization this gets replaced with the equivalent object of the target context.


As hablich@ pointed out, crashing is decidedly not ok.

Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I suspect that we are trying to serialize asm-wasm code in the regular code serialize, which cannot deal with it. Can we turn off serializing that for now until we have a solution?

Test case:

(function f() {
    "use asm";
    function g() { }
    return { g: g };
})();

Run in d8 with --cache=code --validate-asm

I haven't taken a look at this yet, past the analysis earlier in the thread. Also, I'm not convinced of its priority - it appears one would have to manually pass a bunch of flags in an attempt to exercise an unsupported scenario.

I'm lowering the priority and will take a look at it once my plate clears a bit. Please let me know if we feel it should pop back up on the stack.

Well, code serializer is #4 top crasher in canary and the crashes come from games (so likely asm.js). I would say, the priority should be very high.

Generally, we should try to fix clusterfuzz bugs ASAP.

Canary crasher bug https://crbug.com/673321.

Thanks for the data - I'll get a fix ready asap.

Thanks Mircea!

ClusterFuzz has detected this issue as fixed in range 41703:41704.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6409600423428096

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_v8_arm_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !obj->IsMap() in code-serializer.cc
  
Regressed: V8: r39096:39097
Fixed: V8: r41703:41704

Minimized Testcase (0.10 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv97Mtva3cnzqzu4LKWT97XyRu6lmqx3NCwsq277Hm8GynLW_cJ6NGxFuj65B2mqZPfkjyHgIj2qPkmRl8FppFozPkU7uOpVO0KAh1SjFS8H3NwM1-ldEuiGMVSuywqwYBQRYmND5Op_kWgXvvdQS_kQC8_xABw?testcase_id=6409600423428096
(function __f_13() {
    "use asm";
    function __f_16() { }
    return { __f_16: __f_16 };
})();


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 6409600423428096 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.