1891422 - Crash [@ ??] or Assertion failure: framePtr->hasCachedSavedFrame() || hasGoodExcuse, at vm/SavedStacks.cpp:1483 or Assertion failure: enterAndLeaveFrameTrapsCounter_ > 0, at wasm/WasmDebug.cpp:313

Status: VERIFIED FIXED

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

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20240402-de086751acb8 (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

function run(f) {
    f();
}
g47 = newGlobal({ newCompartment: true });
g47.z74 = [];
g47.eval(
  "(" +
  function () {
      Debugger(z74).onExceptionUnwind = function (y65) {};
  } +
  ")()"
);
function wasmEvalBinary(binary, imports, compileOptions) {
  m = new WebAssembly.Module(binary, compileOptions);
  return new WebAssembly.Instance(m, imports);
}
function wasmEvalText(str, imports, compileOptions) {
  return wasmEvalBinary(wasmTextToBinary(str), imports, compileOptions);
}
var ins0 = wasmEvalText(`(module
  (func $fac-acc (export "fac-acc") (param i64 i64) (result i64)
    (if (result i64) (i64.eqz (local.get 0))
      (then (local.get 1))
      (else
        (return_call $fac-acc
          (i64.sub (local.get 0) (i64.const 1))
          (i64.mul (local.get 0) (local.get 1))
        )
      )
    )
  )
)`);
var ins = wasmEvalText(`(module
  (import "" "fac-acc" (func $fac-acc (param i64 i64) (result i64)))
  (type $tz (func (param i64) (result i64)))
  (table $t 1 1 funcref)
  (func $f (export "fac") (param i64) (result i64)
    local.get 0
    i32.const 0
    return_call_indirect $t (type $tz)
  )
  (elem $t (i32.const 0) $fac-acc)
  (func (export "main") (result i64)
    i64.const 5
    call $f
  )
)`, {"": {"fac-acc": ins0.exports["fac-acc"]}});
run(() => ins.exports.main(), WebAssembly.RuntimeError, /indirect call signature mismatch/);

Backtrace:

received signal SIGILL, Illegal instruction.
0x00000cd469ba2021 in ?? ()
#0  0x00000cd469ba2021 in ?? ()
[...]
#12 0x0000000000000000 in ?? ()
rax	0xcd469ba2010	14106446405648
rbx	0x127	295
rcx	0x7fffffffce01	140737488342529
rdx	0x1	1
rsi	0x7ffff3797100	140737278210304
rdi	0x5	5
rbp	0x7fffffffc8e0	140737488341216
rsp	0x7fffffffc8e0	140737488341216
r8	0xffffd55555963e18	-46912491864552
r9	0xffffd55555963e08	-46912491864568
r10	0x127	295
r11	0x5	5
r12	0x7fffffffc950	140737488341328
r13	0x7fffffffcee8	140737488342760
r14	0x7ffff37fe580	140737278633344
r15	0x0	0
rip	0xcd469ba2021	14106446405665
=> 0xcd469ba2021:	ud2    
   0xcd469ba2023:	nopw   0x0(%rax,%rax,1)

Likely sec-moderate at most due to Debugger being involved.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Christian Holler (:decoder)]

Also seeing crashes [@ debugFuncType] that are likely the same also involving debugger.

Comment 4

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20240415160004-fcfbb607fde2.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: 2a7dd75d1bfc1693115b94b3e424e58b331cefab (20240117040825)
End: de086751acb845c252c37aab95a7e6b265df39a6 (20240402094108)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)

Comment 5

[Yury Delendik (:yury)]

Attachment: smaller test case

Basically there is "misunderstanding" about frame being a DebugFrame. Reduced the test case to show interesting moments: the signature check fails, and during unwind it stuck on ReturnStub (which shall not have valid DebugFrame).

Comment 6

[Yury Delendik (:yury)]

Attachment: Bug 1891422 - Check if debugEnabled for top frame when unwinding. r?jseward

Comment 7

[Yury Delendik (:yury)]

The patch fixes incomplete implementation of bug 1865044. The only problem are see that will happen when the asserts are off: the debug traps/breakpoints for functions will be in incorrect state, not allowing to continue debugging.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

:yury, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit BugBot documentation.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7dcf45990771
Check if debugEnabled for top frame when unwinding. r=jseward

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7dcf45990771

Comment 11

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20240514041333-2dfb8db5d717.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 13

[Yury Delendik (:yury)]

Comment on attachment 9398857 [details]
Bug 1891422 - Check if debugEnabled for top frame when unwinding. r?jseward

Beta/Release Uplift Approval Request

• User impact if declined: Some wasm programs maybe crash when using devtools/debugger
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Affects only devtools running wasm programs
• String changes made/needed:
• Is Android affected?: Unknown

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9398857 [details]
Bug 1891422 - Check if debugEnabled for top frame when unwinding. r?jseward

Approved for 127 beta 5, thanks.

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/157bb0b0a5c2