Closed Bug 1590920 Opened 3 years ago Closed 3 years ago

Weird location crash or Assertion failure: &instance->code() == &segment.code() || trap == Trap::IndirectCallBadSig, at WasmSignalHandlers.cpp or Assertion failure: jitCaller->footer()->type() == jit::ExitFrameType::DirectWasmJitCall, at WasmFrameIter.cpp

Categories

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

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- unaffected
firefox72 --- fixed

People

(Reporter: gkw, Assigned: wingo)

References

(Regression)

Details

(5 keywords, Whiteboard: [fuzzblocker][jsbugmon:update])

Crash Data

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 782f341be605 (build with --enable-debug --disable-optimize, run with --fuzzing-safe --no-threads --no-baseline --no-ion --wasm-compiler=baseline):

let { exports } = new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
    (module
        (type (func))
        (export "g" (func 1))
        (func (;;) (type 0)
            (local i32)
            (drop
                (block
                    (result i32)
                    local.get0
                    (if
                        (i32.const1)
                        (return)
                    )
                )
            )
        )
        (func (type 0) call 0))
`)));
exports.g();

Backtrace:

#0  0x00007f1f1c88bac0 in ?? ()
#1  0x00002705be06f0b9 in ?? ()
#2  0x00007ffd13b84190 in ?? ()
#3  0x0000000000000000 in ?? ()
/snip

For detailed crash information, see attachment.

Setting [fuzzblocker] as this is crashing all over the place, memory addresses are on the stack and there are also various assertions I see that seem to be related:

Assertion failure: &instance->code() == &segment.code() || trap == Trap::IndirectCallBadSig, at js/src/wasm/WasmSignalHandlers.cpp:711
Assertion failure: jitCaller->footer()->type() == jit::ExitFrameType::DirectWasmJitCall, at js/src/wasm/WasmFrameIter.cpp:779

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/a9d2b57a99be
user: Andy Wingo
date: Tue Oct 22 15:30:10 2019 +0000
summary: Bug 1578418 - Use WasmABIResults iterator to place block and function results r=luke,lth

Andy/Lars, is bug 1578418 a likely regressor?

Flags: needinfo?(wingo)
Summary: Crash at weird memory address or Assertion failure: &instance->code() == &segment.code() || trap == Trap::IndirectCallBadSig, at js/src/wasm/WasmSignalHandlers.cpp:711 involving wasm → Weird location crash or Assertion failure: &instance->code() == &segment.code() || trap == Trap::IndirectCallBadSig, at WasmSignalHandlers.cpp or Assertion failure: jitCaller->footer()->type() == jit::ExitFrameType::DirectWasmJitCall, at WasmFrameIter.cpp
Flags: needinfo?(lhansen)

Bug 1579418 is almost certainly the regressor. Am on it.

Flags: needinfo?(wingo)

(In reply to Andy Wingo [:wingo] from comment #4)

Bug 1579418 is almost certainly the regressor. Am on it.

Thanks! Please also let us know if the Pernosco session in comment 3 is helpful for you.

Flags: needinfo?(lhansen) → needinfo?(wingo)

Here's a reduced test case:

let { exports } = new WebAssembly.Instance(new WebAssembly.Module(
    wasmTextToBinary(`
      (module
        (func (export "g")
          (drop
            (block (result i32)
              (i32.clz (i32.const 1))
              (if
                (i32.const 1)
                (return))))))`)));
exports.g();

And the corresponding generated code, annotated manually:

	.balign 16, 0xf4   ;; hlt
.Llabel0:
	cmpl       $0x1, %r10d
	je         .Lfrom10
	ud2
.Lfrom10:
	push       %r14         ; tls
	push       %rbp         ; saved bp
	movq       %rsp, %rbp   ; 
	movq       %rsp, %rax
	addq       $0x0000, %rax ; reserve stack
	cmpq       %rax, 0x28(%r14) ; check stack
	jb         .Lfrom41
	ud2
.Lfrom41:

	;; beginFunction: enter body with masm.framePushed = 0
	movl       $0x1, %eax   ; i32.const 1
	bsrl       %eax, %eax   ; clz
	jne        .Lfrom57     ; if clz operand wasn't zero, go ahead
	movl       $0x3f, %eax  ; in the zero case, consider as having 63 leading zeroes (?)
.Lfrom57:
	xorl       $31, %eax    ; truncate to maximum 31 zeroes
	movl       $0x1, %ecx   ; push if test operand: 1
	push       %rax         ; save clz result
	testl      %ecx, %ecx   ; test if operand
	je         .Lfrom79
	jmp        .Lfrom84
.Llabel84:
.Lfrom79:
	addq       $8, %rsp
	jmp        .Lfrom93
	;; endFunction: start of function epilogue
	int3
.Lfrom93:
	pop        %rbp
	pop        %r14
	ret
	;; endFunction: end of function epilogue
	;; endFunction: start of OOL code
	;; endFunction: end of OOL code for index 0
	ud2

It would seem that the "return" isn't getting compiled correctly in this case. I will poke.

@gkw -- I am not able to see the pernosco session; though I auth'd my github, it says "You successfully logged in, but either you are not authorized to view this trace OR the debugging database for this trace has expired (typically 7 days after the trace was collected) and needs to be rebuilt. "

Flags: needinfo?(wingo)

Incidentally @lth, the "if" syntax in this example is nonstandard. See https://webassembly.github.io/spec/core/text/instructions.html#folded-instructions for the standard form. For me to know precisely what it means, I have to read the wasmTextToBinary source :/ Related to bug 1527871.

See Also: → 1527871

(In reply to Andy Wingo [:wingo] from comment #7)

Incidentally @lth, the "if" syntax in this example is nonstandard. See https://webassembly.github.io/spec/core/text/instructions.html#folded-instructions for the standard form. For me to know precisely what it means, I have to read the wasmTextToBinary source :/ Related to bug 1527871.

Well, yeah ;-)

Fix attached in phab; the regressing patch was indeed bug 1579418, landed yesterday.

Keywords: checkin-needed
Component: JavaScript Engine → Javascript: WebAssembly
Assignee: nobody → wingo
Status: NEW → ASSIGNED
Priority: -- → P1

(In reply to Andy Wingo [:wingo] from comment #6)

@gkw -- I am not able to see the pernosco session; though I auth'd my github,

Oh, I realised it needs a mozilla.com email connected to GitHub for this to work for now. Sorry for that. Thanks for looking at this so quickly.

We should consider m-c uplift and respinning Nightlies, if this also fixes bug 1590961 and bug 1590973. The js::frontend::EmitterScope::searchInEnclosingScope and js::wasm::Instance::callExport crash signatures are trending.

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Group: javascript-core-security → core-security-release

Thanks a million, @gkw and other fuzzy people -- it was 1000x easier to fix this bug given the generated test case, instead of a whole site like in bug 1590961. Cheers!

Group: core-security-release
You need to log in before you can comment on or make changes to this bug.