Closed Bug 1450804 Opened 4 years ago Closed 4 years ago

Crash [@ js::jit::Simulator::instructionDecode] with wasm and Debugger

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- verified

People

(Reporter: decoder, Assigned: luke)

Details

(4 keywords, Whiteboard: [jsbugmon:update,bisect][adv-main61-])

Crash Data

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 8063b0c54b88 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --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):

See attachment.


Backtrace:

received signal SIGSEGV, Segmentation fault.
js::jit::Simulator::instructionDecode (this=0xf6e58000, instr=0xeaffffff) at js/src/jit/arm/Simulator-arm.cpp:4848
#0  js::jit::Simulator::instructionDecode (this=0xf6e58000, instr=0xeaffffff) at js/src/jit/arm/Simulator-arm.cpp:4848
#1  0x085aa72a in js::jit::Simulator::execute<false> (this=0xf6e58000) at js/src/jit/arm/Simulator-arm.cpp:4927
#2  js::jit::Simulator::callInternal (this=0xf6e58000, entry=0x45237c50 "\377\377\377\352\004\340-\345\377\377\377\352\360\037-\351\377\377\377\352\020\212-\355\377\377\377", <incomplete sequence \352>) at js/src/jit/arm/Simulator-arm.cpp:5007
#3  0x085aa9a1 in js::jit::Simulator::call (this=<optimized out>, entry=<optimized out>, argument_count=<optimized out>) at js/src/jit/arm/Simulator-arm.cpp:5090
#4  0x0897642a in js::wasm::Instance::callExport (this=0xf57c5ac0, cx=0xf6e1d800, funcIndex=0, args=...) at js/src/wasm/WasmInstance.cpp:741
#5  0x08976d5b in WasmCall (cx=0xf6e1d800, argc=1, vp=0xffffbd30) at js/src/wasm/WasmJS.cpp:1195
#6  0x081acc59 in js::CallJSNative (cx=0xf6e1d800, native=0x8976cb0 <WasmCall(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:290
#7  0x081a1dad in js::InternalCallOrConstruct (cx=0xf6e1d800, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:467
#8  0x081a2170 in InternalCall (cx=cx@entry=0xf6e1d800, args=...) at js/src/vm/Interpreter.cpp:516
#9  0x081a232a in js::Call (cx=0xf6e1d800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:535
#10 0x081a37d8 in js::SpreadCallOperation (cx=0xf6e1d800, script=..., pc=0xf57a2149 ")\233", thisv=..., callee=..., arr=..., newTarget=..., res=...) at js/src/vm/Interpreter.cpp:4900
#11 0x08195b09 in Interpret (cx=0xf6e1d800, state=...) at js/src/vm/Interpreter.cpp:3036
#12 0x081a191a in js::RunScript (cx=0xf6e1d800, state=...) at js/src/vm/Interpreter.cpp:417
#13 0x081a1e75 in js::InternalCallOrConstruct (cx=0xf6e1d800, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:489
#14 0x081a2170 in InternalCall (cx=0xf6e1d800, args=...) at js/src/vm/Interpreter.cpp:516
#15 0x08195da4 in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:522
#16 Interpret (cx=0xf6e1d800, state=...) at js/src/vm/Interpreter.cpp:3084
#17 0x081a191a in js::RunScript (cx=0xf6e1d800, state=...) at js/src/vm/Interpreter.cpp:417
#18 0x081a3f66 in js::ExecuteKernel (cx=0xf6e1d800, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=0x0) at js/src/vm/Interpreter.cpp:700
#19 0x081a4487 in js::Execute (cx=0xf6e1d800, script=..., envChainArg=..., rval=0x0) at js/src/vm/Interpreter.cpp:733
#20 0x085d3658 in ExecuteScript (cx=0xf6e1d800, scope=..., scope@entry=..., script=script@entry=..., rval=0x0) at js/src/jsapi.cpp:4702
#21 0x085d38e3 in JS_ExecuteScript (cx=0xf6e1d800, scriptArg=...) at js/src/jsapi.cpp:4735
#22 0x08071315 in RunFile (compileOnly=false, file=<optimized out>, filename=<optimized out>, cx=0xf6e1d800) at js/src/shell/js.cpp:833
#23 Process (cx=0xf6e1d800, filename=0xffffcfbc "test.js", forceTTY=false, kind=FileScript) at js/src/shell/js.cpp:1277
#24 0x08072c64 in ProcessArgs (op=0xffffcc64, cx=0xf6e1d800) at js/src/shell/js.cpp:8280
#25 Shell (cx=cx@entry=0xf6e1d800, op=op@entry=0xffffcc64, envp=0xffffcde8) at js/src/shell/js.cpp:8672
#26 0x08082d35 in main (argc=4, argv=0xffffcdd4, envp=0xffffcde8) at js/src/shell/js.cpp:9137
eax	0x1	1
ebx	0xf6e58000	-152731648
ecx	0x0	0
edx	0xf562d940	-178071232
esi	0xeaffffff	-352321537
edi	0x8dd5ff4	148725748
ebp	0xffffb688	4294948488
esp	0xffffb660	4294948448
eip	0x85a685f <js::jit::Simulator::instructionDecode(js::jit::SimInstruction*)+47>
=> 0x85a685f <js::jit::Simulator::instructionDecode(js::jit::SimInstruction*)+47>:	mov    (%esi),%edx
   0x85a6861 <js::jit::Simulator::instructionDecode(js::jit::SimInstruction*)+49>:	mov    %edx,%eax


This uses the debugger, but it isn't a null crash and also no debugger on the stack anywhere so I'll mark it s-s until it is confirmed that this is debugger only.
Attached patch fix-armSplinter Review
I don't think this failure can be exhibited in real execution; only with --arm-asm-nop-fill=1.  The issue was nops being inserted in the range where the comment clearly says it assumes they are not.
Assignee: nobody → luke
Attachment #8964439 - Flags: review?(bbouvier)
Comment on attachment 8964439 [details] [diff] [review]
fix-arm

Review of attachment 8964439 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

::: js/src/wasm/WasmBaselineCompile.cpp
@@ +3606,5 @@
>  #elif defined(JS_CODEGEN_ARM)
>          // Flush constant pools: offset must reflect the distance from the MOV
>          // to the start of the table; as the address of the MOV is given by the
>          // label, nothing must come between the bind() and the ma_mov().
> +        AutoForbidPools afp(&masm, /* number of instructions in scope = */ 5);

It seems the ma_sub can expand to one or two physical instructions, according to the subtracted value, but it's hard to tell. Since the leaveNoPool() call checks the actual number of instructions that have been written, I assume this is correct.
Attachment #8964439 - Flags: review?(bbouvier) → review+
Yes, that was my assumption: sub + possible movw/movt.
Sounds like this doesn't need backporting, but feel free to nominate for approval if I got that wrong.
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
Group: javascript-core-security → core-security-release
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][adv-main61-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.