1375404 - Assertion failure: def->type() == definiteType, at js/src/jit/IonBuilder.cpp:7169

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision e49151136658 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager):

Object.prototype.f = 42;
var T = TypedObject;
var ObjectStruct = new T.StructType({f: T.Object});
Object.prototype.f = 42;
var o = new ObjectStruct();
evaluate(`
  o.f %= p;
`);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x00000000006b9800 in js::jit::IonBuilder::ensureDefiniteType (this=this@entry=0x7ffff69c31c0, def=def@entry=0x7ffff69c4158, definiteType=<optimized out>) at js/src/jit/IonBuilder.cpp:7169
#0  0x00000000006b9800 in js::jit::IonBuilder::ensureDefiniteType (this=this@entry=0x7ffff69c31c0, def=def@entry=0x7ffff69c4158, definiteType=<optimized out>) at js/src/jit/IonBuilder.cpp:7169
#1  0x00000000006b9ba5 in js::jit::IonBuilder::addTypeBarrier (this=this@entry=0x7ffff69c31c0, def=0x7ffff69c4158, observed=observed@entry=0x7ffff69c3608, kind=kind@entry=js::jit::BarrierKind::NoBarrier, pbarrier=pbarrier@entry=0x0) at js/src/jit/IonBuilder.cpp:7093
#2  0x00000000006b9c81 in js::jit::IonBuilder::pushTypeBarrier (this=0x7ffff69c31c0, def=0x7ffff69c4158, observed=0x7ffff69c3608, kind=js::jit::BarrierKind::NoBarrier) at js/src/jit/IonBuilder.cpp:7064
#3  0x00000000006d7ca2 in js::jit::IonBuilder::pushReferenceLoadFromTypedObject (this=this@entry=0x7ffff69c31c0, typedObj=typedObj@entry=0x7ffff69c3e40, byteOffset=..., type=type@entry=js::ReferenceTypeDescr::TYPE_OBJECT, name=name@entry=0x7ffff4600d00) at js/src/jit/IonBuilder.cpp:7937
#4  0x00000000006d8788 in js::jit::IonBuilder::getPropTryReferencePropOfTypedObject (this=this@entry=0x7ffff69c31c0, emitted=emitted@entry=0x7fffffffb8ff, typedObj=typedObj@entry=0x7ffff69c3e40, fieldOffset=<optimized out>, fieldPrediction=..., name=name@entry=0x7ffff4600d00) at js/src/jit/IonBuilder.cpp:10577
#5  0x00000000006d8941 in js::jit::IonBuilder::getPropTryTypedObject (this=this@entry=0x7ffff69c31c0, emitted=emitted@entry=0x7fffffffb8ff, obj=obj@entry=0x7ffff69c3e40, name=name@entry=0x7ffff4600d00) at js/src/jit/IonBuilder.cpp:10523
#6  0x00000000006facb0 in js::jit::IonBuilder::jsop_getprop (this=this@entry=0x7ffff69c31c0, name=0x7ffff4600d00) at js/src/jit/IonBuilder.cpp:10278
#7  0x00000000006fd1f4 in js::jit::IonBuilder::inspectOpcode (this=this@entry=0x7ffff69c31c0, op=op@entry=JSOP_GETPROP) at js/src/jit/IonBuilder.cpp:2178
#8  0x00000000006fe9c2 in js::jit::IonBuilder::visitBlock (this=this@entry=0x7ffff69c31c0, cfgblock=cfgblock@entry=0x7ffff4359188, mblock=<optimized out>) at js/src/jit/IonBuilder.cpp:1539
#9  0x00000000006f3d26 in js::jit::IonBuilder::traverseBytecode (this=this@entry=0x7ffff69c31c0) at js/src/jit/IonBuilder.cpp:1456
#10 0x00000000006f49ce in js::jit::IonBuilder::build (this=this@entry=0x7ffff69c31c0) at js/src/jit/IonBuilder.cpp:846
#11 0x000000000043340d in js::jit::IonCompile (cx=cx@entry=0x7ffff6924000, script=<optimized out>, baselineFrame=baselineFrame@entry=0x0, osrPc=osrPc@entry=0x0, recompile=<optimized out>, optimizationLevel=<optimized out>) at js/src/jit/Ion.cpp:2176
[...]
#19 0x000000000045dded in Evaluate (cx=0x7ffff6924000, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:1656
#20 0x000000000054167f in js::CallJSNative (cx=cx@entry=0x7ffff6924000, native=0x45d420 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293
#21 0x0000000000537083 in js::InternalCallOrConstruct (cx=cx@entry=0x7ffff6924000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:470
#22 0x0000000000537498 in InternalCall (cx=cx@entry=0x7ffff6924000, args=...) at js/src/vm/Interpreter.cpp:515
#23 0x000000000053759a in js::CallFromStack (cx=cx@entry=0x7ffff6924000, args=...) at js/src/vm/Interpreter.cpp:521
#24 0x000000000061201f in js::jit::DoCallFallback (cx=0x7ffff6924000, frame=0x7fffffffcd98, stub_=<optimized out>, argc=<optimized out>, vp=0x7fffffffcd48, res=...) at js/src/jit/BaselineIC.cpp:2458
#25 0x0000337b8476a61b in ?? ()
[...]
#47 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff44e0ade	140737292143326
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffb650	140737488336464
rsp	0x7fffffffb620	140737488336416
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff69c4158	140737330823512
r13	0x7ffff69c31c0	140737330819520
r14	0x7ffff69c3608	140737330820616
r15	0x0	0
rip	0x6b9800 <js::jit::IonBuilder::ensureDefiniteType(js::jit::MDefinition*, js::jit::MIRType)+384>
=> 0x6b9800 <js::jit::IonBuilder::ensureDefiniteType(js::jit::MDefinition*, js::jit::MIRType)+384>:	movl   $0x0,0x0
   0x6b980b <js::jit::IonBuilder::ensureDefiniteType(js::jit::MDefinition*, js::jit::MIRType)+395>:	ud2    


Marking s-s because this assertion could indicate some form of type confusion and has been problematic in the past. Also the test involves TypedObject.

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/55f700adddec
user:        Boris Zbarsky
date:        Fri Mar 20 21:34:19 2015 -0400
summary:     Bug 1145491 part 7.  Stop checking compileAndGo before emitting GNAME ops.  r=luke

This iteration took 0.753 seconds to run.

Comment 2

[Boris Zbarsky [:bzbarsky]]

OK, so def->type() is Object, but definiteType is Int32.

"def" is an MLoadUnboxedObjectOrNull, so having its definiteType be Int32 is quite unusual.

We're coming from IonBuilder::pushReferenceLoadFromTypedObject.  "name" in that function is "f", not surprisingly.  I don't see why the gname change (here applicable to "o") would affect this, except perhaps if it made things get ion-compiled where they did not use to before... I'm also not sure why the second assignment to Object.prototype.f matters (but it does!) it should be a no-op, because that value is already 42.

Anyway, in IonBuilder::pushReferenceLoadFromTypedObject when we do the bytecodeTypes(pc) call we get a typeset that has:

    flags = 8
    objectSet = 0x0000000000000000

which is what we see later (8 is TYPE_FLAG_INT32), but of course the .f get should be returning an object, right?

So the question is why the bytecodeTypes(pc) is getting bad type info...

Comment 3

[Julien Cristau [:jcristau]]

tracking this sec-high for 55/56

Comment 4

[Jan de Mooij [:jandem]]

Slightly simpler testcase below.

The main problem is that PropertyReadNeedsTypeBarrier returns BarrierKind::NoBarrier because we don't track type information for the o.f TypedObject property? It's possible this only affects TypedObjects and therefore is Nightly-only, but I'm not sure.

The object vs int32 thing bz mentioned is a red herring. That's the result of a bad heuristic in PropertyReadNeedsTypeBarrier: we attempt to update the observed typeset for an o.x lookup (when the observed set is empty) by looking at the property types for o.x. However in the non-singleton case we skip the receiver and look at the proto - there's no good reason for this and it can cause bad type information like this when properties are shadowed.

---

Object.prototype.f = 42;
var T = TypedObject;
var ObjectStruct = new T.StructType({f: T.Object});
Object.prototype.f = 42;
var o = new ObjectStruct();
evaluate(`
    foo = o.f;
`);

Comment 5

[Brian Hackett [Laid off!]]

Attachment: patch

I think this is a bogus assert.  Typed object property types do not reflect the initial values of their properties --- null for object references, undefined for value properties --- and IonBuilder adds bailouts on these initial values if they are not included in the observed types for a read, e.g. pushReferenceLoadFromTypedObject.  Since there aren't any non-null values which can be read from this object, the MLoadUnboxedObjectOrNull will always bailout.  I don't see any callers which actually require the result of ensureDefiniteType to have the provided type, so this patch just removes the assert.

Comment 6

[Brian Hackett [Laid off!]]

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

[Security approval request comment]
This bug is not security sensitive but I don't seem to be able to mark it as such in the new bugzilla UI.

Comment 7

[Jan de Mooij [:jandem]]

Edit bug, then you can uncheck under Security.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9c4ffc88192
Remove bogus assert, r=jandem.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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