Closed Bug 1342882 Opened 5 years ago Closed 5 years ago

Hit MOZ_CRASH(unexpected type) at js/src/jit/Lowering.cpp:2323


(Core :: JavaScript Engine, defect, P1)




Tracking Status
firefox-esr45 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 + fixed
firefox54 + fixed
firefox55 --- fixed


(Reporter: decoder, Assigned: h4writer)



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


(1 file)

The following testcase crashes on mozilla-central revision a08ec245fa24 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-eager --ion-offthread-compile=off):

for (let [k, map = send.log += "" + map] of map) {}


 received signal SIGSEGV, Segmentation fault.
0x00000000007921c0 in js::jit::LIRGenerator::visitToString (this=0x7fffffffbf30, ins=0x7ffff69cf1b0) at js/src/jit/Lowering.cpp:2323
#0  0x00000000007921c0 in js::jit::LIRGenerator::visitToString (this=0x7fffffffbf30, ins=0x7ffff69cf1b0) at js/src/jit/Lowering.cpp:2323
#1  0x00000000007a2849 in js::jit::LIRGenerator::visitInstruction (this=this@entry=0x7fffffffbf30, ins=ins@entry=0x7ffff69cf1b0) at js/src/jit/Lowering.cpp:4851
#2  0x00000000007a2bae in js::jit::LIRGenerator::visitInstruction (ins=0x7ffff69cf1b0, this=0x7fffffffbf30) at js/src/ds/LifoAlloc.h:338
#3  js::jit::LIRGenerator::visitBlock (this=this@entry=0x7fffffffbf30, block=block@entry=0x7ffff69a9f50) at js/src/jit/Lowering.cpp:4931
#4  0x00000000007a2f1b in js::jit::LIRGenerator::generate (this=this@entry=0x7fffffffbf30) at js/src/jit/Lowering.cpp:4999
#5  0x00000000006c9deb in js::jit::GenerateLIR (mir=mir@entry=0x7ffff69a91b8) at js/src/jit/Ion.cpp:1962
#6  0x00000000006f2c65 in js::jit::CompileBackEnd (mir=mir@entry=0x7ffff69a91b8) at js/src/jit/Ion.cpp:2057
#7  0x000000000042f6c6 in js::jit::IonCompile (cx=cx@entry=0x7ffff6948000, script=<optimized out>, baselineFrame=baselineFrame@entry=0x0, osrPc=osrPc@entry=0x0, recompile=<optimized out>, optimizationLevel=<optimized out>) at js/src/jit/Ion.cpp:2338
#8  0x00000000006f3168 in js::jit::Compile (cx=cx@entry=0x7ffff6948000, script=script@entry=..., osrFrame=osrFrame@entry=0x0, osrPc=osrPc@entry=0x0, forceRecompile=forceRecompile@entry=false) at js/src/jit/Ion.cpp:2525
#9  0x00000000006f3308 in js::jit::CanEnter (cx=cx@entry=0x7ffff6948000, state=...) at js/src/jit/Ion.cpp:2622
#10 0x00000000005354cd in js::RunScript (cx=cx@entry=0x7ffff6948000, state=...) at js/src/vm/Interpreter.cpp:369
#19 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8371
rax	0x0	0
rbx	0x7fffffffbf30	140737488338736
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbdc0	140737488338368
rsp	0x7fffffffbd80	140737488338304
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff69cf1b0	140737330868656
r13	0x7ffff69cf1b0	140737330868656
r14	0x7fffffffbf30	140737488338736
r15	0x7ffff4322580	140737290315136
rip	0x7921c0 <js::jit::LIRGenerator::visitToString(js::jit::MToString*)+800>
=> 0x7921c0 <js::jit::LIRGenerator::visitToString(js::jit::MToString*)+800>:	movl   $0x0,0x0
   0x7921cb <js::jit::LIRGenerator::visitToString(js::jit::MToString*)+811>:	ud2    

Marking s-s until triaged because this is a LIR assertion, given the assertion message this might indicate some form of type mismatch.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
user:        Hannes Verschore
date:        Thu Jan 05 10:22:16 2017 +0100
summary:     Bug 1328148: IonMonkey - Use MConcat for more cases, r=jandem

This iteration took 256.112 seconds to run.
Flags: needinfo?(hv1989)
Keywords: sec-high
Tracking for 53 and 54 since this is sec-high. 53 is about to go into beta next week.
Hannes, can you *please* take a look at this? This issue is on its way into beta and we want to prevent it from going into release.
Flags: needinfo?(hv1989)
Assignee: nobody → hv1989
Please keep needinfo's!
Flags: needinfo?(hv1989)
Priority: -- → P1
Not really security sensitive. We have a MOZ_CRASH, which means we will safely crash upon encountering this. Opening.
Group: javascript-core-security
Flags: needinfo?(hv1989)
Keywords: sec-high
Attachment #8846625 - Flags: review?(jdemooij)
Comment on attachment 8846625 [details] [diff] [review]
Also disallow magic types.

Review of attachment 8846625 [details] [diff] [review]:

Please add a test (ideally one that's a bit nicer than the fuzz test..)
Attachment #8846625 - Flags: review?(jdemooij) → review+
Pushed by
IonMonkey - Don't allow magic types for concatenation, r=jandem
(In reply to Jan de Mooij [:jandem] from comment #7)
> Please add a test (ideally one that's a bit nicer than the fuzz test..)

I went with the fuzz test. I didn't find a way to make it nicer and still exhibiting the issue.
Comment on attachment 8846625 [details] [diff] [review]
Also disallow magic types.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1328148

[User impact if declined]:
Safe crashes on JS code with specific code and runtime information

[Is this code covered by automated tests?]:
Yes, fuzz test landed

[Has the fix been verified in Nightly?]:
Jit-tests show it is fixed. Just pushed to inbound and it will also run there.

[Needs manual test from QE? If yes, steps to reproduce]: 

[List of other uplifts needed for the feature/fix]:

[Is the change risky?]:

[Why is the change risky/not risky?]:
For this specific path we are now running the old again, which is still heavily run/tested.

[String changes made/needed]:
Attachment #8846625 - Flags: approval-mozilla-beta?
Attachment #8846625 - Flags: approval-mozilla-aurora?
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8846625 [details] [diff] [review]
Also disallow magic types.

Fix a regression. Beta53+ & Aurora54+.
Attachment #8846625 - Flags: approval-mozilla-beta?
Attachment #8846625 - Flags: approval-mozilla-beta+
Attachment #8846625 - Flags: approval-mozilla-aurora?
Attachment #8846625 - Flags: approval-mozilla-aurora+
Setting qe-verify- based on Hannes' assessment on manual testing needs (see Comment 10) and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.