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

RESOLVED FIXED in Firefox 53

Status

()

Core
JavaScript Engine
P1
critical
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: decoder, Assigned: h4writer)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
mozilla55
x86_64
Linux
assertion, crash, jsbugmon, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox51 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53+ fixed, firefox54+ fixed, firefox55 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment)

(Reporter)

Description

10 months ago
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) {}


Backtrace:

 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.

Updated

10 months ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

10 months ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/94d8af817497
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.

Updated

10 months ago
Flags: needinfo?(hv1989)
Blocks: 1328148
status-firefox51: --- → unaffected
status-firefox52: --- → unaffected
status-firefox53: --- → affected
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → unaffected
tracking-firefox53: --- → ?
tracking-firefox54: --- → ?

Updated

10 months ago
Keywords: sec-high
Tracking for 53 and 54 since this is sec-high. 53 is about to go into beta next week.
tracking-firefox53: ? → +
tracking-firefox54: ? → +
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)

Updated

10 months ago
Assignee: nobody → hv1989
(Assignee)

Comment 4

9 months ago
Please keep needinfo's!
Flags: needinfo?(hv1989)
Priority: -- → P1
(Assignee)

Comment 5

9 months ago
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
(Assignee)

Comment 6

9 months ago
Created attachment 8846625 [details] [diff] [review]
Also disallow magic types.
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+

Comment 8

9 months ago
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26ded80b0df5
IonMonkey - Don't allow magic types for concatenation, r=jandem
(Assignee)

Comment 9

9 months ago
(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.
(Assignee)

Comment 10

9 months ago
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]: 
No

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

[Is the change risky?]:
No

[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?

Comment 11

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/26ded80b0df5
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox55: --- → fixed
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+

Comment 13

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/602928ae7209
status-firefox54: affected → fixed

Comment 14

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/6a4296f12e40
status-firefox53: affected → fixed
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.