626464 - Tracer generates bad code for {__proto__: ...}, leading to a crash

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: Test case for shell (run without -j, contains options() call)

The attached test case crashes on TM tip both 32 and 64 bit. The gdb trace reveals a corrupted function pointer:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004f45a7 in js::DefaultValue (cx=0xab4da0, obj=0x7ffff690a640, hint=JSTYPE_VOID, vp=0x7ffff6abf148) at jsobj.cpp:5897
5897            if (!obj->getClass()->convert(cx, obj, hint, &v))
(gdb) bt 4
#0  0x00000000004f45a7 in js::DefaultValue (cx=0xab4da0, obj=0x7ffff690a640, hint=JSTYPE_VOID, vp=0x7ffff6abf148) at jsobj.cpp:5897
#1  0x00000000006d9add in js::Interpret (cx=0xab4da0, entryFrame=0x7ffff6abf048, inlineCallCount=1, interpMode=JSINTERP_NORMAL) at jsinterp.cpp:3677
#2  0x00000000004ca661 in js::RunScript (cx=0xab4da0, script=0xaf8670, fp=0x7ffff6abf048) at jsinterp.cpp:657
#3  0x00000000004cbab6 in js::Execute (cx=0xab4da0, chain=0x7ffff6903048, script=0xaf8670, prev=0x0, flags=0, result=0x0) at jsinterp.cpp:1023
(More stack frames follow...)
(gdb) print obj->getClass()->convert
$1 = (JSBool (*)(JSContext *, JSObject *, JSType, js::Value *)) 0x74006f00750071


The function pointer here could be a unicode string, but it's at least corrupted in some way. Locking this because it's possibly exploitable.

Comment 1

[Jason Orendorff [:jorendorff]]

Simpler:

options('tracejit');

for (var i = 0; i < 10; i++)
    "" + {__proto__: 'aaa'};

Comment 2

[Jesse Ruderman]

The first bad revision is:
changeset:   33c58d16d911
user:        Jason Orendorff
date:        Fri Jan 14 16:18:53 2011 -0600
summary:     Bug 559653 - Record assignment before the interpreter goes, mostly. This reduces record_SetPropHit to a narrower callback, record_AddProperty. r=brendan.

Comment 3

[Christian Holler (:decoder)]

(In reply to comment #1)
> Simpler:

With that simpler test case, the pointer is NULL though, in the original test case it was corrupted somehow.

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: v1

The code in TR::recordInitPropertyOp that claims to "duplicate the interpreter's special treatment of __proto__" calls recordSetPropertyOp, which copies sp[-1] to sp[-2] at the end.

decoder, the upshot of the bug is that we read garbage and treat it as a function pointer. It's not too surprising that different test cases result in different garbage (though on my machine the two crashes look the same).

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 504548 [details] [diff] [review]
v1

>     // Duplicate the interpreter's special treatment of __proto__.
>-    if (atom == cx->runtime->atomState.protoAtom)
>-        return recordSetPropertyOp();
>+    if (atom == cx->runtime->atomState.protoAtom) {
>+        bool deferred;
>+        return setProperty(obj, obj_ins, v, v_ins, &deferred);
>+    }

My review missed the bug, so I can field this. The comment should specify JSOP_INITPROP and might even wrap to a second line by stating that JSOP_INITPROP does not write to the stack for that op.

/be

Comment 6

[Jason Orendorff [:jorendorff]]

I changed it to say:

    // Duplicate the interpreter's special treatment of __proto__. Unlike the
    // SET opcodes, JSOP_INIT{PROP,METHOD} do not write to the stack.

Believe it or not, JSOP_INITMETHOD "__proto__" can occur too. (It can even, heaven help us, succeed in making a method property named __proto__ on the object.)

https://hg.mozilla.org/tracemonkey/rev/e4e1e187bc9b

Comment 8

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/e4e1e187bc9b

Comment 10

[Christian Holler (:decoder)]

Fixed for a long time and not affecting old branches, opening this.

Comment 11

[Christian Holler (:decoder)]

Old tracer bug, marking verified.