1891485 - Differential error: RuntimeError: too many array elements

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P2)

Description

[Christian Holler (:decoder)]

The attached testcase shows differential behavior on mozilla-central revision 20240413-4a07e55947ef (build with fuzzing-asan-opt, run with --no-threads --wasm-compiler=baseline).

I made the testcase crash based on the last exception seen which differs when running with --wasm-compiler=baseline vs. --wasm-compiler=optimizing.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Ben Visness [:bvisness]]

The two runtime errors here are of different categories. The unreachable executed is the result of an unreachable instruction; the too many array elements is an implementation limit and not required by the spec.

The reduced test case is simply:

  (type $a (array (mut i8)))
  (func (export "test")
    i32.const -1
    array.new_default $a
    unreachable
  )

Ion is optimizing away the array.new_default entirely, preventing us from triggering the implementation limit. Although this is an observable difference in behavior between the two compilers, it's one we're ok with. Which implementation limits may be triggered in which order is not something worth making deterministic.

Comment 4

[Christian Holler (:decoder)]

We have no way of ignoring this difference, the usual contract is to try and make it deterministic when --differential-testing is set. Otherwise, we would have to stop all differential testing.

Comment 5

[Ben Visness [:bvisness]]

Sorry, I didn't realize how we usually handle this sort of thing in differential testing. Iain pointed me to this similar example in JS; we can add something similar to these messages to give you the clue you need to ignore these differences.

Comment 6

[Christian Holler (:decoder)]

No worries.

The message you pointed to is a last resort thing that works for the JS fuzzers but it wouldn't work in this scenario without some modifications.

I assume it isn't possible to fix up the situation (make the behavior deterministic)? If that's the case, then we need some way to determine from JS if we are in this situation (e.g. a shell function isWasmDifferentialTestingTainted returning a bool) something like this I could use from within the fuzzer (that is running inside the JS shell) to check if we should throw away the result and continue.

Comment 7

[Daniel Veditz [:dveditz]]

Problem for the fuzzing infrastructure, but not a user security issue.

Comment 8

[Bugmon [:jkratzer for issues]]

Unable to reproduce bug 1891485 using build mozilla-central 20240413085157-4a07e55947ef. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 9

[Ben Visness [:bvisness]]

Attachment: Bug 1891485: Make wasm gc allocations deterministic for differential fuzzing. r=yury

Comment 10

[Ben Visness [:bvisness]]

For the record, Yury asked about this behavior on the wasm design repo (https://github.com/WebAssembly/gc/issues/540) and the consensus was that the spec indeed does not require this particular trap to be observable.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f47e86c86c23
Make wasm gc allocations deterministic for differential fuzzing. r=yury

Comment 12

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/f47e86c86c23

Comment 13

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:bvisness, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.