567059 - Crash [@ js_LookupPropertyWithFlags] or "Assertion failure: isNative(),"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

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

for (a = 0; a < 8; ++a) {
    if (a) {
        Proxy.create({}).t()
    }
}

crashes js opt shell on TM tip with -j at js_LookupPropertyWithFlags and asserts js debug shell on TM tip with -j at Assertion failure: isNative(), at ../jsscope.h:549

Tested on 64-bit Ubuntu Linux 10.04 and changeset tm-42457-f423fbc93e3f. Assuming related to harmony:proxies. Doesn't seem related to bug 566908.

Console stdout:

Program received signal SIGSEGV, Segmentation fault.
0x000000000046673d in js_LookupPropertyWithFlags ()
(gdb) bt
#0  0x000000000046673d in js_LookupPropertyWithFlags ()
#1  0x0000000000519d87 in js::TraceRecorder::test_property_cache(JSObject*, nanojit::LIns*, JSObject*&, js::PCVal&) ()
#2  0x0000000000524424 in js::TraceRecorder::record_JSOP_CALLPROP() ()
#3  0x0000000000527e4d in js::TraceRecorder::monitorRecording(JSOp) ()
#4  0x000000000054d64a in js_Interpret ()
#5  0x0000000000458421 in js_Execute ()
#6  0x000000000040b6d6 in JS_ExecuteScript ()
#7  0x00000000004069e5 in Process(JSContext*, JSObject*, char*, int) ()
#8  0x0000000000407269 in main ()
(gdb) x/i $rip
=> 0x46673d <js_LookupPropertyWithFlags+189>:	cmp    (%rdx),%rbx
(gdb) x/b $rdx
0xffffffff:	Cannot access memory at address 0xffffffff

Comment 1

[Andreas Gal :gal]

Attachment: patch

Our soon to be native xml and with objects would have hit this, too. Good thing we found it early.

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Our soon to be native xml and with objects would have hit this, too.

s/native/non-native/

We have non-native Java objects in 3.6 still. Wonder what the test does with such a LiveConnect critter instead of Proxy.create()'s return value.

/be

Comment 3

[Andreas Gal :gal]

map will be null so its probably a fairly safe crash

Comment 4

[Brendan Eich [:brendan]]

map will point to a singleton shared map statically pointed at by the non-native JSObjectOps singleton.

/be

Comment 5

[Andreas Gal :gal]

The actual effects are still a bit unclear but best case we generate bad jit code and more likely we crash.

Comment 6

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/89d31a27e5b8

Comment 7

[Andreas Gal :gal]

Aehm ...

http://hg.mozilla.org/tracemonkey/rev/89d31a27e5b8

Comment 8

[Andreas Gal :gal]

Double aehm ... Follow-up fix. 2-liner and we got it still wrong. That will teach me not to thoroughly test trivial fixes.

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

Comment 9

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

Andreas: can you provide a roll-up patch for mozilla-1.9.2 landing and nominate for approval1.9.2.5?

Comment 10

[Andreas Gal :gal]

#9: pulling 1.9.2. Patch today.

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/89d31a27e5b8

Comment 12

[LegNeato]

Any hope for branch patches for this?

Comment 13

[Andreas Gal :gal]

Attachment: 1.9.2 patch

Comment 14

[Andreas Gal :gal]

Comment on attachment 454234 [details] [diff] [review]
1.9.2 patch

Can someone land this for me please? (its a blocker so I am not sure I even need approval)

Comment 15

[Brendan Eich [:brendan]]

Attachment: 1.9.2 patch with missing ! restored

Comment 16

[Andreas Gal :gal]

Thanks for catching this. Pulling 1.9.1 right now over the hotel's 9600 baud connection.

Comment 17

[Brendan Eich [:brendan]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1804452e19b1
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/98888b2f3bf8

I wasn't able to build 1.9.2 js shell (debug or opt) on 64-bit Mac:

c++ -o jsregexp.o -c -fvisibility=hidden -DOSTYPE=\"Darwin10.4.0\" -DOSARCH=Darwin -DEXPORT_JS_API  -DJS_USE_SAFE_ARENA  -I.. -I. -I./dist/include -I./dist/include/nsprpub     -I..    -fPIC   -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Wno-long-long -fno-strict-aliasing -fpascal-strings -fno-common -pthread -pipe  -DNDEBUG -DTRIMMED -O3 -fstrict-aliasing    -DMOZILLA_CLIENT -include ./js-confdefs.h -Wp,-MD,.deps/jsregexp.pp ../jsregexp.cpp
{standard input}:17427:suffix or operands invalid for `call'
make[1]: *** [jsregexp.o] Error 1
make: *** [default] Error 2

So I went in blind. Then opened my eyes and read the patch, and fixed the bug in it :-|.

Setting .6-fixed but if this goes in .7 due to another release intervening, please fix the status.

/be

Comment 18

[Andreas Gal :gal]

Yeah that's why I didn't test either. Can't build 1.9.1 either. Trying to build for 32-bit. Maybe that works.

Comment 19

[Reed Loden [:reed]]

(In reply to comment #17)
> Setting .6-fixed but if this goes in .7 due to another release intervening,
> please fix the status.

Is there a reason why this landed without approval? All branch patches require approval from drivers before landing.

Comment 20

[Reed Loden [:reed]]

(In reply to comment #14)
> Can someone land this for me please? (its a blocker so I am not sure I even
> need approval)

*All* patches on the branches require approval. It seems dveditz needs to do his yearly reminder mail...

http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/9b31b7523bd8cc48/5a289dbac727fead

Comment 21

[Brendan Eich [:brendan]]

Reed, you are not helping here.

/be

Comment 22

[LegNeato]

a=LegNeato for 1.9.2.7, though we only want it if we can be sure we will be getting the 1.9.1 patch as well (don't want to leave those users vulnerable).

Comment 23

[Brendan Eich [:brendan]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ae51fe8759e3

/be

Comment 24

[Carsten Book [:Tomcat]]

no crash with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.7pre) Gecko/20100701 Firefox/3.6.7pre 

Garry can you reproduce the crash ?

Comment 25

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

Proxies are not defined for 1.9.1 and 1.9.2 so the underlying issue cannot be tested using the original testcase in comment #0.

Comment 26

[Christian Holler (:decoder)]

Filter on qa-project-auto-change:

Bug in removed tracer code, setting in-testsuite- flag.