456284 - Null derefs in js_Interpret with split objects

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

mozilla-central

ecma/ExecutionContexts/10.2.2-1.js

#0  0x00000000 in ?? ()
#1  0x00065a6b in js_Interpret (cx=0x300d10) at jsinterp.cpp:3628
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x809a00, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#3  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x809a00, rval=0x0) at jsapi.cpp:4910
#4  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff635 "10.2.2-1.js", forceTTY=0) at js.cpp:277
#5  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4d4, argc=11) at js.cpp:510
#6  0x000094b4 in main (argc=11, argv=0xbffff4d4, envp=0xbffff504) at js.cpp:3982

==

ecma/ExecutionContexts/10.2.2-2.js

#0  0x00000000 in ?? ()
#1  0x00065a6b in js_Interpret (cx=0x300d10) at jsinterp.cpp:3628
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x2506e0, script=0x30b280, down=0xbfffe250, flags=18, result=0xbfffcadc) at jsinterp.cpp:1550
#3  0x000ab276 in js_obj_eval (cx=0x300d10, obj=0x2500c0, argc=1, argv=0x80e5b8, rval=0xbfffcadc) at jsobj.cpp:1338
#4  0x0009a1af in js_Invoke (cx=0x300d10, argc=1, vp=0x80e5b0, flags=2) at jsinterp.cpp:1306
#5  0x000761c6 in js_Interpret (cx=0x300d10) at jsinterp.cpp:4976
#6  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x809a00, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#7  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x809a00, rval=0x0) at jsapi.cpp:4910
#8  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff635 "10.2.2-2.js", forceTTY=0) at js.cpp:277
#9  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4d4, argc=11) at js.cpp:510
#10 0x000094b4 in main (argc=11, argv=0xbffff4d4, envp=0xbffff504) at js.cpp:3982

==

ecma/ExecutionContexts/10.2.3-1.js

#0  0x00000000 in ?? ()
#1  0x00065a6b in js_Interpret (cx=0x300d10) at jsinterp.cpp:3628
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x30a980, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#3  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x30a980, rval=0x0) at jsapi.cpp:4910
#4  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff635 "10.2.3-1.js", forceTTY=0) at js.cpp:277
#5  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4d4, argc=11) at js.cpp:510
#6  0x000094b4 in main (argc=11, argv=0xbffff4d4, envp=0xbffff504) at js.cpp:3982

ecma/ObjectObjects/15.2.2.1.js

==

#0  0x00000000 in ?? ()
#1  0x00066655 in js_Interpret (cx=0x300d10) at jsinterp.cpp:3632
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x80be00, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#3  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x80be00, rval=0x0) at jsapi.cpp:4910
#4  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff63d "15.2.2.1.js", forceTTY=0) at js.cpp:277
#5  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4dc, argc=11) at js.cpp:510
#6  0x000094b4 in main (argc=11, argv=0xbffff4dc, envp=0xbffff50c) at js.cpp:3982

==

js1_5/Function/15.3.4.4.js

#0  0x00000000 in ?? ()
#1  0x00066655 in js_Interpret (cx=0x300d10) at jsinterp.cpp:3632
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x80d000, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#3  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x80d000, rval=0x0) at jsapi.cpp:4910
#4  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff645 "15.3.4.4.js", forceTTY=0) at js.cpp:277
#5  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4e4, argc=11) at js.cpp:510
#6  0x000094b4 in main (argc=11, argv=0xbffff4e4, envp=0xbffff514) at js.cpp:3982

==

js1_5/Regress/regress-281930.js

#0  0x00000000 in ?? ()
#1  0x00066655 in js_Interpret (cx=0x300d10) at jsinterp.cpp:3632
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x307aa0, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#3  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x307aa0, rval=0x0) at jsapi.cpp:4910
#4  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff63d "regress-281930.js", forceTTY=0) at js.cpp:277
#5  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4dc, argc=11) at js.cpp:510
#6  0x000094b4 in main (argc=11, argv=0xbffff4dc, envp=0xbffff50c) at js.cpp:3982

==

js1_5/extensions/regress-164697.js

#0  0x00000000 in ?? ()
#1  0x00065a6b in js_Interpret (cx=0x300d10) at jsinterp.cpp:3628
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250600, script=0x3081b0, down=0x80e514, flags=16, result=0xbfffcadc) at jsinterp.cpp:1550
#3  0x000ab276 in js_obj_eval (cx=0x300d10, obj=0x2500c0, argc=1, argv=0x80e594, rval=0xbfffcadc) at jsobj.cpp:1338
#4  0x0009a1af in js_Invoke (cx=0x300d10, argc=1, vp=0x80e58c, flags=2) at jsinterp.cpp:1306
#5  0x000761c6 in js_Interpret (cx=0x300d10) at jsinterp.cpp:4976
#6  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x813a00, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#7  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x813a00, rval=0x0) at jsapi.cpp:4910
#8  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff635 "regress-164697.js", forceTTY=0) at js.cpp:277
#9  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4d4, argc=11) at js.cpp:510
#10 0x000094b4 in main (argc=11, argv=0xbffff4d4, envp=0xbffff504) at js.cpp:3982

==

js1_6/extensions/regress-312385-01.js

#0  0x00000000 in ?? ()
#1  0x00065a6b in js_Interpret (cx=0x300d10) at jsinterp.cpp:3628
#2  0x00098d6f in js_Execute (cx=0x300d10, chain=0x250100, script=0x809800, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1550
#3  0x00018d06 in JS_ExecuteScript (cx=0x300d10, obj=0x250100, script=0x809800, rval=0x0) at jsapi.cpp:4910
#4  0x00002b1a in Process (cx=0x300d10, obj=0x250100, filename=0xbffff62d "regress-312385-01.js", forceTTY=0) at js.cpp:277
#5  0x000081ee in ProcessArgs (cx=0x300d10, obj=0x250100, argv=0xbffff4cc, argc=11) at js.cpp:510
#6  0x000094b4 in main (argc=11, argv=0xbffff4cc, envp=0xbffff4fc) at js.cpp:3982

Comment 1

[Robert Sayre]

still happening?

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Yeah.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

The comment in the patch says it all...

Comment 4

[Brian Crowder]

Comment on attachment 349349 [details] [diff] [review]
Fix

If it is a "rule" that JSExtendedClass host objects must have equality-hooks, then we should enforce with an assertion somewhere, right?

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #4)
> (From update of attachment 349349 [details] [diff] [review])
> If it is a "rule" that JSExtendedClass host objects must have equality-hooks,
> then we should enforce with an assertion somewhere, right?

We generally don't assert non-null, since a null pointer dereference (excluding insanely buggy OSes) is non-exploitable and as good a blame-arrow as an assert (less clear due to the lack of an assertion message, but more direct because the pc is at the faulting instruction).

/be

Comment 6

[Brian Crowder]

I think in this case, an assertion at the time that the class is inited would be much more helpful to 3rd parties using spidermonkey.  I think it makes the "rule" much more clear, since an external developer could easily ponder over a null-dereference like this for a long while without knowing that the equality hook is a requirement of the API.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Brendan: also, in this case the pc is not at the faulting instruction because we're *calling* the null pointer. I also keep forgetting that equality is not optional since it's the only required JSExtendedClass member.

Comment 8

[Brendan Eich [:brendan]]

Oh, sure -- if you can get coverage early and unconditionally via an assertion, go for it.

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Updated

Not all classes go through JS_InitClass, so this is the most central place we can assert.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 349503 [details] [diff] [review]
Updated

This allows us to test harder!

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 349503 [details] [diff] [review]
Updated

a191=beltzner

Comment 12

[Peter Van der Beken [:peterv]]

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

Next time please follow instructions ("make sure you include a commit message
and an username in the attachment").

Comment 13

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2