1139474 - Crash [@ IsCacheableSetPropCallPropertyOp] with --unboxed-objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision c5b90c003be8 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --enable-debug, run with --fuzzing-safe --thread-count=2 --unboxed-objects --ion-eager --ion-offthread-compile=off):

var gTestcases = new Array();
var gTc = gTestcases.length;
function TestCase(d) {
  this.description = d;
  gTestcases[gTc++] = this;
}
function jsTestDriverEnd() {
  for (var i = 0; i < gTestcases.length; i++)
    gTestcases[i].dump();
}
setJitCompilerOption("ion.warmup.trigger", 1);
for (var i = 0; i < 3; ++i)
jsTestDriverEnd();
loadFile('new TestCase (String(["678"])); new TestCase (String(["34 678"]));');
for (var i = 0; i < 4; ++i)
  loadFile("jsTestDriverEnd();");
loadFile("new TestCase();");
function loadFile(lfVarx) {
  try {
    evaluate(lfVarx, { noScriptRval : true, compileAndGo : true }); 
  } catch (lfVare) {}
}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000850874 in IsCacheableSetPropCallPropertyOp (obj=..., obj@entry=(JSObject * const) 0x7ffff57000c0 [object Object], holder=..., holder@entry=(JSObject * const) 0x7ffff57000c0 [object Object], shape=shape@entry=0x1) at js/src/jit/IonCaches.cpp:2089
2089	    if (shape->hasSlot())
#0  0x0000000000850874 in IsCacheableSetPropCallPropertyOp (obj=..., obj@entry=(JSObject * const) 0x7ffff57000c0 [object Object], holder=..., holder@entry=(JSObject * const) 0x7ffff57000c0 [object Object], shape=shape@entry=0x1) at js/src/jit/IonCaches.cpp:2089
#1  0x00000000008b4010 in CanAttachNativeSetProp (checkTypeset=0x7fffffff9840, shape=0x1, holder=(JSObject *) 0x7ffff57000c0 [object Object], needsTypeBarrier=<optimized out>, val=..., id=..., obj=(JSObject * const) 0x7ffff57000c0 [object Object], cx=0x1a32b50) at js/src/jit/IonCaches.cpp:2858
#2  js::jit::SetPropertyIC::update (cx=0x1a32b50, outerScript=0x7ffff565e448, cacheIndex=<optimized out>, obj=(JSObject * const) 0x7ffff57000c0 [object Object], value=JSVAL_VOID) at js/src/jit/IonCaches.cpp:2996
#3  0x00007ffff5587123 in ?? ()
#4  0x0000000000000000 in ?? ()
rax	0x1	1
rbx	0x7fffffff98c0	140737488328896
rcx	0x7fffffff98e0	140737488328928
rdx	0x7fffffff98c0	140737488328896
rsi	0x7ffff57000c0	140737311146176
rdi	0x1	1
rbp	0x7fffffff97d0	140737488328656
rsp	0x7fffffff97c0	140737488328640
r8	0x1b21440	28447808
r9	0x0	0
r10	0x7fffffff9a80	140737488329344
r11	0x1b20548	28443976
r12	0x1b20490	28443792
r13	0x7fffffff98b0	140737488328880
r14	0x1a32b50	27470672
r15	0x1b20548	28443976
rip	0x850874 <IsCacheableSetPropCallPropertyOp(JS::HandleObject, JS::HandleObject, js::HandleShape)+36>
=> 0x850874 <IsCacheableSetPropCallPropertyOp(JS::HandleObject, JS::HandleObject, js::HandleShape)+36>:	testb  $0x40,0x14(%rdi)
   0x850878 <IsCacheableSetPropCallPropertyOp(JS::HandleObject, JS::HandleObject, js::HandleShape)+40>:	jne    0x850888 <IsCacheableSetPropCallPropertyOp(JS::HandleObject, JS::HandleObject, js::HandleShape)+56>

Comment 1

[Boris Zbarsky [:bzbarsky]]

So shape=0x1 is clearly the issue, right?

I don't see anything in CanAttachNativeSetProp that would filter out non-native objects per se, so if LookupPropertyOure on an unboxed object will return shape == 0x1 we would run into this.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: patch

The paths for attaching Ion ICs and, to a lesser extent, baseline ICs, are pretty convoluted, and especially so given the unboxed objects stuff.  After bug 1137180 lands I'd like to make a pass over this stuff and clean things up some.

FWIW I ran into this in bug 1137180 and the patch there includes this fix as well.

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150303042544" and the hash "c75a393d7bfb".
The "bad" changeset has the timestamp "20150303043344" and the hash "9e86bfdf6bd8".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c75a393d7bfb&tochange=9e86bfdf6bd8

Comment 4

[Jan de Mooij [:jandem]]

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

Review of attachment 8573448 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Brian Hackett (:bhackett) from comment #2)
> After bug 1137180 lands I'd like to make a pass over this stuff and clean things
> up some.

Cool. Also, LookupPropertyPure returning a 0x1 shape in some cases is a footgun. It'd be nice to have a LookupResult class or something to make it obvious it's not always a valid-Shape-or-null. (Or maybe that's the part you decided to clean up :)

Comment 5

[Brian Hackett [Laid off!]]

I'd been planning on cleaning up the JIT caches primarily, but you're right that the 0x1 shape thing is ugly and would be handled much better as a specialized class similar to TaggedProto.  I'll try to fix that too.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d43fb3fe56f0

Comment 6

[Phil Ringnalda (:philor)]

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