1425387 - Assertion failure: iter.cur() == iter.maybeSkipAutomaticInstructions(), at js/src/jit/arm/MacroAssembler-arm.cpp:359 with wasm and Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 8062887ff0d9 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --arm-asm-nop-fill=1 --ion-offthread-compile=off):

var g = newGlobal();
var dbg = new Debugger(g);
g.eval(`
  o = new WebAssembly.parse(
    new WebAssembly.Module(wasmTextToBinary('(module (func) (export "" 0))'))
  );
`);


Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0853a5ae in js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator> (imm32=..., dest=..., c=js::jit::Assembler::Always, rs=js::jit::Assembler::L_MOVWT, iter=...) at js/src/jit/arm/MacroAssembler-arm.cpp:359
#1  0x08915046 in js::jit::MacroAssembler::patchAdd32ToPtr (imm=..., offset=..., this=0xf6dfe3b0) at js/src/jit/arm/MacroAssembler-arm-inl.h:376
#2  js::wasm::BaseStackFrame::patchAllocStack (this=0xf6dfe27c) at js/src/wasm/WasmBaselineCompile.cpp:1191
#3  js::wasm::BaseCompiler::endFunction (this=0xf6dfdec0) at js/src/wasm/WasmBaselineCompile.cpp:3009
#4  js::wasm::BaseCompiler::emitFunction (this=0xf6dfdec0) at js/src/wasm/WasmBaselineCompile.cpp:9212
#5  js::wasm::BaselineCompileFunctions (env=..., lifo=..., inputs=..., code=0xf6e9d1b8, error=0xf6dfeaf4) at js/src/wasm/WasmBaselineCompile.cpp:9361
#6  0x08997533 in ExecuteCompileTask (task=0xf6e9d058, error=error@entry=0xf6dfeaf4) at js/src/wasm/WasmGenerator.cpp:626
#7  0x08997e40 in js::wasm::ExecuteCompileTaskFromHelperThread (task=0xf6e9d058) at js/src/wasm/WasmGenerator.cpp:644
#8  0x087a5681 in js::HelperThread::handleWasmWorkload (this=0xf6e3d400, locked=..., mode=js::wasm::CompileMode::Tier1) at js/src/vm/HelperThreads.cpp:1831
#9  0x087b3ff3 in js::HelperThread::threadLoop (this=0xf6e3d400) at js/src/vm/HelperThreads.cpp:2299
#10 0x087b4298 in js::HelperThread::ThreadMain (arg=0xf6e3d400) at js/src/vm/HelperThreads.cpp:1816
#11 0x087b482b in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::callMain<0u> (this=0xf6e0e058) at js/src/threading/Thread.h:242
#12 js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start (aPack=0xf6e0e058) at js/src/threading/Thread.h:235
#13 0xf7fb226a in start_thread (arg=0xf6dffb40) at pthread_create.c:333
#14 0xf7cd33fe in clone () from /lib32/libc.so.6
eax	0x0	0
ebx	0xf6dfdd80	-153100928
ecx	0xf7d9f864	-136710044
edx	0x0	0
esi	0xf6ea0078	-152436616
edi	0x8d98ff4	148475892
ebp	0xf6dfdd68	4141866344
esp	0xf6dfdd40	4141866304
eip	0x853a5ae <js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator>(js::jit::Imm32, js::jit::Register, js::jit::Assembler::Condition, js::jit::Assembler::RelocStyle, js::jit::BufferInstructionIterator)+366>
=> 0x853a5ae <js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator>(js::jit::Imm32, js::jit::Register, js::jit::Assembler::Condition, js::jit::Assembler::RelocStyle, js::jit::BufferInstructionIterator)+366>:	movl   $0x0,0x0
   0x853a5b8 <js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator>(js::jit::Imm32, js::jit::Register, js::jit::Assembler::Condition, js::jit::Assembler::RelocStyle, js::jit::BufferInstructionIterator)+376>:	ud2

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

This is an assertion added during a sec-audit protected bug, so it should be protected too.

Comment 2

[Sean Stangl [:sstangl]]

This isn't a security problem. The issue is that I added an assertion that ma_mov_patch() is only called on "intentionally-inserted" instructions. When the flag --arm-asm-nop-fill=1 is passed, NOPs are automatically inserted in places where they could never be in normal execution, and that causes the asserted condition to no longer be true.

The assertion is valuable to have, but I suppose it's not true in all conditions due to this argument.

Comment 3

[Sean Stangl [:sstangl]]

Attachment: 0001-Bug-1425387-Skip-automatically-inserted-NOPs-while-p.patch

This patch will skip automatically-inserted NOPs when patching instructions.

These NOPs can't be present there in normal execution. They are inserted only by --arm-asm-nop-fill=1.

Comment 4

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e6b873b57f5f5e0cf7de9b62c71981eef4f59b5

Comment 5

[Andrew Overholt [:overholt]]

Should we close this now?

Comment 6

[Sean Stangl [:sstangl]]

Yes, it was landed.

Comment 7

[Ryan VanderMeulen (PTO, back July 27)]

For posterity:
https://hg.mozilla.org/mozilla-central/rev/2e6b873b57f5f5e0cf7de9b62c71981eef4f59b5