569306 - (64-bit) Crash: [@ js_ArrayClass] or [@ js_GetPropertyHelper] or "Assertion failure: JSVAL_IS_OBJECT(v),"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

typeof w & options().i
function g() {
  delete this.uneval;
  uneval
}
g()
(function() {
  for each(e in [
      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
    ]) {
    (function f(aaaaaa) {
      for (x in "") {}
      if (aaaaaa) {
        f(aaaaaa - 1)
      }
    } (2906))
  }
} ())


crashes 64-bit js opt shell on TM tip without -j at js_ArrayClass with js_GetPropertyHelper somewhere on the stack, and asserts 64-bit js debug shell on TM tip without -j at Assertion failure: JSVAL_IS_OBJECT(v), at ../jsapi.h:183

s-s because it crashes at a strange and scary memory address which had seemingly morphed several times during the process of reduction. Assume [sg:critical?] unless otherwise noted.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x000000010018dfe0
0x000000010018dfe0 in js_ArrayClass ()
(gdb) bt
#0  0x000000010018dfe0 in js_ArrayClass ()
#1  0x0000000100072dec in js_LookupPropertyWithFlags ()
#2  0x0000000100074a19 in js_GetPropertyHelper ()
#3  0x0000000100074dae in js_GetMethod ()
#4  0x000000010006802c in GetIterator ()
#5  0x0000000100068bfe in js_ValueToIterator ()
#6  0x00000001000579fd in js_Interpret ()
#7  0x0000000100063f85 in js_Execute ()
#8  0x000000010000d110 in JS_ExecuteScript ()
#9  0x000000010000341a in Process ()
#10 0x0000000100006f4d in main ()
(gdb) x/i $rip
0x10018dfe0 <js_ArrayClass>:    and    %eax,(%rdx)
(gdb) x/b $eax
Value can't be converted to integer.
(gdb) x/b $rdx
0x1012007e4:    0x00

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   41832:b15fd8b568e4
user:        Andreas Gal
date:        Fri May 07 17:52:52 2010 -0700
summary:     fast object iteration (558754, r=brendan, CLOSED TREE).

Comment 2

[Andreas Gal :gal]

Looks like a rooting bug. Nice test case Gary.

(gdb) r x.js
Starting program: /Users/gal/workspace/tracemonkey-repository/js/src/Darwin_DBG.OBJ/js x.js
Reading symbols for shared libraries .++++..................................................................................... done
Assertion failure: JSVAL_IS_OBJECT(v), at ../jsapi.h:183

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x000000010015556e in JS_Assert (s=0x1001ee284 "JSVAL_IS_OBJECT(v)", file=0x1001e5708 "../jsapi.h", ln=183) at ../jsutil.cpp:77
77	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */
(gdb) up
#1  0x000000010014312a in JSVAL_TO_OBJECT (v=-2676586395008836902) at jsapi.h:183
183	    JS_ASSERT(JSVAL_IS_OBJECT(v));
(gdb) p/x v
$1 = 0xdadadadadadadada
(gdb) bt
#0  0x000000010015556e in JS_Assert (s=0x1001ee284 "JSVAL_IS_OBJECT(v)", file=0x1001e5708 "../jsapi.h", ln=183) at ../jsutil.cpp:77
#1  0x000000010014312a in JSVAL_TO_OBJECT (v=-2676586395008836902) at jsapi.h:183
#2  0x0000000100146845 in JSObject::getParent (this=0x101402180) at jsobj.h:366
#3  0x00000001000beffa in js::NewObjectWithGivenProto (cx=0x10083c800, clasp=0x100267540, proto=0x101402180, parent=0x0, objectSize=0) at jsobjinlines.h:645
#4  0x00000001000c51f1 in js::NewObject (cx=0x10083c800, clasp=0x100267540, proto=0x101402180, parent=0x0, objectSize=0) at jsobjinlines.h:700
#5  0x00000001000c52a0 in js_PrimitiveToObject (cx=0x10083c800, vp=0x7fff5fbfeb50) at ../jsobj.cpp:5878
#6  0x00000001000c5341 in js_ValueToObject (cx=0x10083c800, v=4315938820, objp=0x7fff5fbfebc8) at ../jsobj.cpp:5896
#7  0x00000001000b3884 in js_ValueToIterator (cx=0x10083c800, flags=1, vp=0x101014cf8) at ../jsiter.cpp:589
#8  0x000000010008a84f in js_Interpret (cx=0x10083c800) at jsops.cpp:458
#9  0x00000001000ae19a in js_Execute (cx=0x10083c800, chain=0x101402000, script=0x100414810, down=0x0, flags=0, result=0x0) at jsinterp.cpp:837
#10 0x00000001000123af in JS_ExecuteScript (cx=0x10083c800, obj=0x101402000, script=0x100414810, rval=0x0) at ../jsapi.cpp:4831
#11 0x000000010000a168 in Process (cx=0x10083c800, obj=0x101402000, filename=0x7fff5fbffa95 "x.js", forceTTY=0) at ../../shell/js.cpp:422
#12 0x000000010000adad in ProcessArgs (cx=0x10083c800, obj=0x101402000, argv=0x7fff5fbff948, argc=1) at ../../shell/js.cpp:836
#13 0x000000010000af28 in main (argc=1, argv=0x7fff5fbff948, envp=0x7fff5fbff958) at ../../shell/js.cpp:5082
(gdb)

Comment 3

[Jesse Ruderman]

A rooting bug?  Does that mean the garbage collector is running at some point?

Comment 4

[Andreas Gal :gal]

Yes.

(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /Users/gal/workspace/tracemonkey-repository/js/src/Darwin_DBG.OBJ/js x.js
Reading symbols for shared libraries . done

Breakpoint 1, js_GC (cx=0x10083c800, gckind=GC_LOCK_HELD) at ../jsgc.cpp:3175
3175	    JSRuntime *rt = cx->runtime;
(gdb) 

If you look at the stack trace you see the value v=-2676586395008836902.

(gdb) p/x -2676586395008836902
$2 = 0xdadadadadadadada

0xdadadadadadadada is the value the GC clears dead objects with. Whenever you see that in a stack trace, a rooting bug is very likely.

Comment 5

[Andreas Gal :gal]

I will grab this for now. If anyone else has free cycles, please steal.

Comment 6

[Andreas Gal :gal]

Attachment: patch

Very old rooting bug.

Comment 7

[Andreas Gal :gal]

Comment on attachment 448627 [details] [diff] [review]
patch

Note that the code triggers a GC _exactly_ and _reproducibly_ between the GetClassPrototype and the NewObejectWithGivenProto call. Amazing.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 448627 [details] [diff] [review]
patch

I talked about this with gal and we found the underlying cause of this: we were resolving string twice. The reduced testcase to observe misbehavior is:

var s = new String();
delete uneval;
uneval;
s instanceof String;

should return true, instead returns false. gal is re-patching.

Comment 9

[Andreas Gal :gal]

Attachment: patch

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 448667 [details] [diff] [review]
patch

r=me pending tryserver success.

Comment 11

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/580ca37b66ad

Comment 12

[Robert Sayre]

three tests are failing:

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/tracemonkey-macosx-opt-unittest-jsreftest/build/jsreftest/tests/jsreftest.html?test=js1_5/extensions/regress-352604.js | Unknown file:///builds/slave/tracemonkey-macosx-opt-unittest-jsreftest/build/jsreftest/tests/js1_5/extensions/regress-352604.js:58: Function is not defined item 1
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/tracemonkey-macosx-opt-unittest-jsreftest/build/jsreftest/tests/jsreftest.html?test=js1_5/Regress/regress-352604.js | Unknown file:///builds/slave/tracemonkey-macosx-opt-unittest-jsreftest/build/jsreftest/tests/js1_5/Regress/regress-352604.js:56: Function is not defined item 1
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/tracemonkey-macosx-opt-unittest-jsreftest/build/jsreftest/tests/jsreftest.html?test=js1_8_5/regress/regress-533876.js | Unknown file:///builds/slave/tracemonkey-macosx-opt-unittest-jsreftest/build/jsreftest/tests/js1_8_5/regress/regress-533876.js:19: eval is not defined item 1

Comment 13

[Andreas Gal :gal]

Looking.

Comment 14

[Andreas Gal :gal]

I can reproduce.

Comment 15

[Andreas Gal :gal]

352604 is an invalid test:

  function f() {}
  delete Function;
  var g = new Function('');

  expect = f.__proto__;
  actual = g.__proto__;

if you delete Function, is gone now. resolve hook won't rematerialize it.

Comment 16

[Andreas Gal :gal]

533876 is invalid too:

delete eval;  // force dictionary scope for global
gc();
var f = eval("(function () { return /x/; })");
x.watch('x', f);  // clone property from global to x, including SPROP_IN_DICTIONARY flag

Disabling both.

Comment 17

[Andreas Gal :gal]

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

Comment 18

[Jesse Ruderman]

Rather than disabling the tests, you should change the expected result, so we'll notice if we accidentally go back to the old wrong behavior.

Comment 19

[Andreas Gal :gal]

I will make a better test. The test are testing a but failing on b. That's a horrible test.

Comment 20

[Andreas Gal :gal]

Test: http://hg.mozilla.org/tracemonkey/rev/86e25d9034b3

Comment 21

[Jesse Ruderman]

They're fuzz-generated tests. What do you expect? ;)

Comment 22

[Robert Sayre]

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

Comment 23

[LegNeato]

Does this patch apply to the branches?

Comment 24

[Andreas Gal :gal]

I am really busy with compartment stuff. Any chance that someone can backport this for me? Assigning to sayrer to find an owner. Bounce it back to me if nobody can be found. Thanks.

Comment 25

[Daniel Veditz [:dveditz]]

When are you (or another js team member) not going to be busy with compartments? We need a branch patch.

Comment 26

[Andreas Gal :gal]

Lets do this the other way around. Give me a date when you need this and I will make sure we have a patch.

Comment 27

[LegNeato]

Deferring to a future point release as we have run out of time. If this absolutely needs to be fixed and can land today or tomorrow, please send a note to [e-mail]