Closed Bug 539379 Opened 10 years ago Closed 10 years ago

TM: Crash [@ ExecuteTrace] or [@ ExecuteTree]

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
flash10.1
Tracking Status
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: gkw, Assigned: njn)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase, Whiteboard: [ccbr][sg:critical?] fixed-in-nanojit, fixed-in-tracemonkey)

Crash Data

Attachments

(1 file)

x = /x/;
(function f() {
    x.r = x;
    return f()
})()

crashes js dbg and opt shell with -j on TM tip at random scary addresses with ExecuteTrace and/or ExecuteTree on the stack.

Setting security-sensitive because of scary addresses.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000106
0x0000000100692f5d in ?? ()
(gdb) bt
#0  0x0000000100692f5d in ?? ()
#1  0x0000000100155099 in ExecuteTrace (cx=0x100411610, f=0x100809a20, state=@0x7fff5fbfe830) at ../jstracer.cpp:6522
#2  0x000000010018792b in ExecuteTree (cx=0x100411610, f=0x100809a20, inlineCallCount=@0x7fff5fbfee0c, innermostNestedGuardp=0x7fff5fbfe990) at ../jstracer.cpp:6618
#3  0x0000000100188d69 in js_MonitorLoopEdge (cx=0x100411610, inlineCallCount=@0x7fff5fbfee0c, reason=Record_EnterFrame) at ../jstracer.cpp:7111
#4  0x000000010009064e in js_Interpret (cx=0x100411610) at jsops.cpp:2231
#5  0x00000001000a4096 in js_Execute (cx=0x100411610, chain=0x1003f2000, script=0x1004148a0, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1618
#6  0x0000000100010db6 in JS_ExecuteScript (cx=0x100411610, obj=0x1003f2000, script=0x1004148a0, rval=0x0) at ../jsapi.cpp:4970
#7  0x00000001000092ec in Process (cx=0x100411610, obj=0x1003f2000, filename=0x7fff5fbff91d "../2interesting/w4653-reduced.js", forceTTY=0) at ../../shell/js.cpp:439
#8  0x0000000100009f34 in ProcessArgs (cx=0x100411610, obj=0x1003f2000, argv=0x7fff5fbff778, argc=2) at ../../shell/js.cpp:848
#9  0x000000010000a24b in main (argc=2, argv=0x7fff5fbff778, envp=0x7fff5fbff790) at ../../shell/js.cpp:4898
(gdb)
Tested with a 64-bit js shell on Mac OS X 10.6.2.

autoBisect shows this is probably related to bug 538060:

The first bad revision is:
changeset:   37024:56cdca9fe3d8
user:        Nicholas Nethercote
date:        Mon Jan 11 15:51:49 2010 +1100
summary:     Bug 538060 - nanojit: improve 64-bit loads and stores in the X64 back-end.  r=gal,rreitmai,edwsmith.
Blocks: 538060
Hardware: x86 → x86_64
Does not seem to cause 32-bit shells to crash.
I can reproduce with that revision.  I'll take a look tomorrow morning (Melbourne time).
Assignee: general → nnethercote
Attached patch patchSplinter Review
The problem was this LIR instruction:

   stqi $global0[48] = $global0

My new code wasn't allowing for the case where an stqi had a 'base' and 'value' that were equal, and $global sort of ended up in two registers, but in a totally bogus way.  This is pretty rare, which is why my testing didn't uncover it.

I could have done a simple 3-line fix but there is a common pattern involved with word-sized stores.  So I did some refactoring and introduced getBaseReg2(), which is like getBaseReg() but works for two registers, similar to findRegFor2().  I used it in the i386, X64, ARM and Sparc backends, but not the PPC backend, because it didn't quite follow the same pattern and also is harder to understand because it is both 32-bit and 64-bit and I can't test it.

In order to do this I also had to tweak findRegFor2() so it can use different allowed masks for the two values.  This has the added benefit that it will slightly improve code generation in a few cases, because if 'base' and/or 'value' had different allowed masks, both were forced into the more restrictive one.  Eg. on i386, for byte-sized stores 'value' must be in ah/bh/ch/dh, and so 'base' was being forced into ah/bh/ch/dh as well;  on X64 'base' cannot be in R12 so 'value' could not be either.

All the basic tests pass on TM, TR, NJ on i386 and X64, and the TM tests pass on ARM (my ARM VM is too slow to test beyond that).
Attachment #421547 - Flags: review?(edwsmith)
Attachment #421547 - Flags: review?(edwsmith) → review+
Target Milestone: --- → flash10.1
http://hg.mozilla.org/projects/nanojit-central/rev/a5115ee971c8

NOTE TO SELF: when merging this to TM, DON'T FORGET THE NEW TEST CASE.
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-nanojit
http://hg.mozilla.org/tracemonkey/rev/80ff3ca19b1c
http://hg.mozilla.org/tracemonkey/rev/4e8621ab0232
Whiteboard: [sg:critical?] fixed-in-nanojit → [sg:critical?] fixed-in-nanojit, fixed-in-tracemonkey
Whiteboard: [sg:critical?] fixed-in-nanojit, fixed-in-tracemonkey → [ccbr][sg:critical?] fixed-in-nanojit, fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/80ff3ca19b1c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Crash Signature: [@ ExecuteTrace] [@ ExecuteTree]
Tracer has been long gone on trunk, marking verified.
Status: RESOLVED → VERIFIED
Crash Signature: [@ ExecuteTrace] [@ ExecuteTree] → [@ ExecuteTrace] [@ ExecuteTree]
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug539379.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.