528048 - TM: Crash on trace with recursion, explicit GC

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

for (j = 0; j < 3; j++) {}
m = [];
m.concat();
n = [];
n.concat([]);
Function("\
  for (i = 0; i < 8; i++)\
  ((function f1(b, c) {\
  if (c) {\
    return (gc)()\
  }\
  f1(b, 1);\
  ((function f2(d, e) {\
    return d.length == e ? 0 : d[e] + f2(d, e + 1)\
  })([{}, /x/, /x/], 0))\
  })())\
")()


Spun off from bug 523793 comment 3. dvander suggests security-sensitive till more is known.

Comment 1

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

autoBisect shows this is probably due to bug 459301:

http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=89e665eb9944&tochange=d04601f54db5

Comment 2

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

Opt shell with -j stack:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x000000000000001c
Crashed Thread:  0

Thread 0 Crashed:
0   ???                           	0x001e3c4c 0 + 1981516
1   js-opt-tm-darwin              	0x000fd8d6 __ZL11ExecuteTreeP9JSContextP12TreeFragmentRjPP10VMSideExit + 806
2   js-opt-tm-darwin              	0x0011c21a js_MonitorLoopEdge(JSContext*, unsigned int&, RecordReason) + 1322
3   js-opt-tm-darwin              	0x0005a0b3 js_Interpret + 48371
4   js-opt-tm-darwin              	0x0005e22c js_Execute + 444
5   js-opt-tm-darwin              	0x0000d1cc JS_ExecuteScript + 60
6   js-opt-tm-darwin              	0x00003cb8 __ZL7ProcessP9JSContextP8JSObjectPci + 1336
7   js-opt-tm-darwin              	0x00007db4 main + 2212
8   js-opt-tm-darwin              	0x00001c3b _start + 209
9   js-opt-tm-darwin              	0x00001b69 start + 41

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

At first I thought there might be a GC hazard but I don't think I'll be so lucky.

From initial debugging it seems like we build a trace which gets obj.proto.proto.valueOf, and |obj| is a RegexObj. Later |obj| is a normal scripted object, and going two links up to the prototype chain is invalid (NULL).

Why this is happening, I'm not yet sure.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

I got real lucky here, it is a rooting bug. A property cache guard burns an object address into trace but doesn't root it. It gets freed and reallocated, and later the guard succeeds but on a different object, with a different shape and prototype chain.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

This affects 1.9.2, though the test case does not trigger there since we don't trace recursion.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: more fixes

There are a few more possible misuses of INS_CONSTWORD.

Comment 7

[Andreas Gal :gal]

Comment on attachment 412115 [details] [diff] [review]
more fixes

With some beautifications as discussed.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: prettier patch

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 412117 [details] [diff] [review]
prettier patch

>+TraceRecorder::insImmVal(jsval val)
>+{
>+    if (!JSVAL_IS_INT(val) && !JSVAL_IS_SPECIAL(val) && val != JSVAL_NULL)

This should be just JSVAL_IS_GCTHING(val) of course.

>+        treeInfo->gcthings.addUnique(val);
>+    return lir->insImmWord(val);
>+}

/be

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

Whoops, thanks, didn't realize that existed.

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: prettier patch v2

use JSVAL_IS_TRACEABLE

Comment 12

[Andreas Gal :gal]

Comment on attachment 412255 [details] [diff] [review]
prettier patch v2

Very nice.

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/d996400c949a

Comment 14

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d996400c949a

Comment 15

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f2cad56bfdcb

Comment 16

[chris hofmann]

It was hoped that this patch would fix  Bug 530955 and we would verify it in 3.6b5.

doesn't seem like that is the case.

Comment 17

[chris hofmann]

(In reply to comment #15)
> 2009-12-04 08:01:34 PST
> http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f2cad56bfdcb

maybe the patch landing just missed the cut off? b5 builds were also made right around then.

Comment 18

[David Mandelin [:dmandelin]]

From pushlog, it looks like that patch (f2cad56bfdcb) did make it in time for b5:

http://hg.mozilla.org/releases/mozilla-1.9.2/pushloghtml?startdate=12%2F2%2F2009&enddate=12%2F6%2F2009

Comment 19

[Christian Holler (:decoder)]

Tracer has been long gone on trunk, marking verified.

Comment 20

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929