If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

IonMonkey: Assertion failure: type == MIRType_Object , at ../ion/TypeOracle.h:296

RESOLVED DUPLICATE of bug 750894

Status

()

Core
JavaScript Engine
--
major
RESOLVED DUPLICATE of bug 750894
6 years ago
6 years ago

People

(Reporter: decoder, Assigned: nbp)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Other Branch
x86_64
Linux
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [jsbugmon:update,reconfirm,ignore])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
The following testcase asserts on ionmonkey revision b5b6e6aebb36 (run with --ion -n -m --ion-eager):


gczeal(4);
evaluate("var g_rx = /(?:)/;");
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update]
(Reporter)

Comment 1

6 years ago
JSBugMon: The testcase found in this bug no longer reproduces (tried revision de015aff650d).
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,reconfirm]
(Reporter)

Comment 2

6 years ago
JSBugMon: This bug has been automatically confirmed to be still valid (reproduced on revision bc1833f2111e).
(Reporter)

Updated

6 years ago
Whiteboard: [jsbugmon:update,reconfirm] → [jsbugmon:update,reconfirm,ignore]
(Assignee)

Comment 3

6 years ago
Created attachment 618336 [details] [diff] [review]
Fix mismatch between MStoreSlot value and slot type

This bug is also reproduced with ./js --ion-eager ./jit-test/tests/basic/bug740509.js and fails with:

Assertion failure: type == MIRType_Object, at ../git-export/js/src/ion/TypeOracle.h:306

This patch change the type policy to specialize only if the slot type is specialized.
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Attachment #618336 - Flags: review?(jdemooij)
(Assignee)

Updated

6 years ago
Blocks: 742136
(Assignee)

Comment 4

6 years ago
Created attachment 618486 [details] [diff] [review]
Fix mismatch between MStoreSlot value and slot type

InitProp use propertyWrite instead of elementWrite oracle functions.
Attachment #618336 - Attachment is obsolete: true
Attachment #618336 - Flags: review?(jdemooij)
Attachment #618486 - Flags: review?(jdemooij)
Comment on attachment 618486 [details] [diff] [review]
Fix mismatch between MStoreSlot value and slot type

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

We really want to use a typed/specialized store even if the slot type is unknown. How about just changing CodeGenerator*::visitStoreSlotT to pass JSVAL_TYPE_UNKNOWN to emitPreBarrier if the slot type is MIRType_None? (Maybe initialize slotType_ to MIRType_Value instead of MIRType_None while you are here?)

::: js/src/ion/IonBuilder.cpp
@@ +2875,5 @@
>      MStoreSlot *store = MStoreSlot::New(slots, baseObj->dynamicSlotIndex(shape->slot()), value);
>      current->add(store);
> +
> +    if (cx->compartment->needsBarrier() && oracle->propertyWriteNeedsBarrier(script, pc))
> +        store->setNeedsBarrier(true);

initprop does not need a write barrier (it does not overwrite an object/string property value).

@@ +2880,5 @@
> +
> +    MIRType slotType = oracle->propertyWrite(script, pc);
> +    if (slotType == MIRType_None)
> +        slotType = MIRType_Value;
> +    store->setSlotType(slotType);

slotType is used to optimize the store by not writing the type tag if it's known to be the same. We are initializing the slot here so we need to write both the type and the payload.
Attachment #618486 - Flags: review?(jdemooij)
(Assignee)

Comment 6

6 years ago
Created attachment 618739 [details] [diff] [review]
Add BarrierTypeFromMIRType to handle MIRType_None.
Attachment #618486 - Attachment is obsolete: true
Attachment #618739 - Flags: review?(jdemooij)
Comment on attachment 618739 [details] [diff] [review]
Add BarrierTypeFromMIRType to handle MIRType_None.

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

::: js/src/ion/TypeOracle.h
@@ +252,5 @@
>      bool elementWriteIsPacked(JSScript *script, jsbytecode *pc);
>      bool setElementHasWrittenHoles(JSScript *script, jsbytecode *pc);
>      bool propertyWriteCanSpecialize(JSScript *script, jsbytecode *pc);
>      bool elementWriteNeedsBarrier(JSScript *script, jsbytecode *pc);
> +    MIRType specializedType(types::TypeSet *objTypes);

Can we rename this function to specializedPropertyType and make it private?

@@ +313,5 @@
>    }
>  }
>  
> +static inline JSValueType
> +BarrierTypeFromMIRType(MIRType type)

Nice!
Attachment #618739 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 8

6 years ago
(In reply to Jan de Mooij (:jandem) from comment #7)
> Comment on attachment 618739 [details] [diff] [review]
> Add BarrierTypeFromMIRType to handle MIRType_None.
> 
> Review of attachment 618739 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/ion/TypeOracle.h
> @@ +252,5 @@
> >      bool elementWriteIsPacked(JSScript *script, jsbytecode *pc);
> >      bool setElementHasWrittenHoles(JSScript *script, jsbytecode *pc);
> >      bool propertyWriteCanSpecialize(JSScript *script, jsbytecode *pc);
> >      bool elementWriteNeedsBarrier(JSScript *script, jsbytecode *pc);
> > +    MIRType specializedType(types::TypeSet *objTypes);
> 
> Can we rename this function to specializedPropertyType and make it private?

This function was part of elementWrite and I thought it was good to have such function outside of elementWrite because the only thing it is doing is finding the generic MIRType which correspond to a TypeSet.  Unless I misunderstood, it sounds better than getKnownTypeTag.

What do you think ?
(In reply to Nicolas B. Pierron [:pierron] from comment #8)
> 
> This function was part of elementWrite and I thought it was good to have
> such function outside of elementWrite because the only thing it is doing is
> finding the generic MIRType which correspond to a TypeSet.

It looks at all TypeObjects in the TypeSet and compares the types of some property, hence the suggestion to add "Property" to the name of the method. It should probably also take the property name as input, something like this:

specializedPropertyType(TypeSet *objTypes, jsid prop);

elementWrite uses JSID_VOID as property id, it's what TI uses for the type of numeric properties.
(Assignee)

Comment 10

6 years ago
Patch scheduling is a complex thing.  Even if the context switch between patches can be costly, not doing it can create an accumulation of patch in the queue of patches.

This cause some patch-provider landing fixes which are currently fixed by another patch-provider because the scheduler of the second patch-provider is a “Shortest­ Job­ First” as opposed to “First­ Come, First ­Served” of the first patch-provider.

http://en.wikipedia.org/wiki/Scheduling_%28computing%29#Shortest_remaining_time
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 750894
You need to log in before you can comment on or make changes to this bug.