799907 - IonMonkey: Assertion failure: mir->type() == MIRType_Value, at ../ion/shared/Lowering-shared-inl.h:413 or Crash [@ js::ion::LinearScanAllocator::populateSafepoints]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: Testcase for shell

The attached testcase asserts on mozilla-central revision ec10630b1a54 (run with --ion-eager).

Comment 1

[Christian Holler (:decoder)]

Crash trace from opt-build:

==32163== Invalid read of size 4
==32163==    at 0x83890CA: js::ion::LinearScanAllocator::populateSafepoints() (LIR.h:90)
==32163==    by 0x8345128: js::ion::CompileBackEnd(js::ion::IonBuilder*) (Ion.cpp:881)
==32163==    by 0x92C0B07: ???
==32163==    by 0x92BC087: ???
==32163==  Address 0x1c is not stack'd, malloc'd or (recently) free'd


S-s due to GC-relatedness and fragile test.

Comment 2

[Christian Holler (:decoder)]

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

The first bad revision is:
changeset:   109322:825de541f566
user:        Kannan Vijayan
date:        Fri Oct 05 00:28:36 2012 -0400
summary:     Bug 786126 - part 3 - Allow null/undefined in typeset of target object of GetProp ICs. (r=sstangl)

This iteration took 91.551 seconds to run.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

IonMonkey's type information can sometimes be more accurate than TI's; in this case TI's typeset said "value" but Ion said "Object", so the explicit unbox was asserting. GetPropertyCache has an unboxing type policy though, so we can rely on that working instead.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80482a232127

Comment 5

[Sean Stangl [:sstangl]]

(In reply to David Anderson [:dvander] from comment #3)
> Created attachment 670163 [details] [diff] [review]
> fix
> 
> IonMonkey's type information can sometimes be more accurate than TI's; in
> this case TI's typeset said "value" but Ion said "Object", so the explicit
> unbox was asserting. GetPropertyCache has an unboxing type policy though, so
> we can rely on that working instead.

Another problem here is that we're looking at obj->type() to determine the type of a MIR node -- when it is actually a redundant MPhi of (Object, Object), but during the IonBuilder phase it is typed as MIRType_Value. It's another one of those situations where we're looking at the MIR in order to build the MIR.

It's safe to just remove the erroneous condition, since it's redundant.

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/80482a232127

Comment 7

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 8

[Bob Clary [:bc] (inactive)]

crash automation hit this on Aurora/18 on Windows/Linux at
https://docs.google.com/presentation/d/1FC1pyFCnK_8-2tAcPGmVUkWDginw_Okks_x0kL_OVG0/edit#slide=id.g20e781bb_0_26

Comment 9

[u279076]

Can this be put in testsuite?

Comment 10

[Al Billings [:abillings - ex-MoCo]]

This never got a security rating. Can someone suggest one?

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

This one is not really clear, it's either not exploitable at all or it's sg:critical. I'd just go with sg:critical.