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

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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)

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Does not seem to cause 32-bit shells to crash.

Comment 3

[Nicholas Nethercote [inactive]]

I can reproduce with that revision.  I'll take a look tomorrow morning (Melbourne time).

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: patch

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).

Comment 5

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/projects/nanojit-central/rev/a5115ee971c8

NOTE TO SELF: when merging this to TM, DON'T FORGET THE NEW TEST CASE.

Comment 6

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/80ff3ca19b1c
http://hg.mozilla.org/tracemonkey/rev/4e8621ab0232

Comment 7

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/80ff3ca19b1c

Comment 8

[Christian Holler (:decoder)]

Tracer has been long gone on trunk, marking verified.

Comment 9

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug539379.js.