Closed Bug 913978 Opened 6 years ago Closed 6 years ago

Assertion failure: IsCallPC(pc), at jit/IonFrames.cpp

Categories

(Core :: JavaScript Engine, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: gkw, Assigned: djvj)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

Attached file stack
x = ({
    get: function(r, name) {
        return ParallelArray(new Uint8ClampedArray)[name];
    },
    set: function(r, val) {}
});
Object.defineProperty(this, "y", {
    get: function() {
        return Proxy.create(x, undefined);
    }
});
for (i = 0; i < 999; ++i) {
    Array.prototype.unshift.call(y, ArrayBuffer());
}

asserts js debug shell on m-c changeset c7cc85e13f7a without any CLI arguments at Assertion failure: IsCallPC(pc), at jit/IonFrames.cpp

My configure flags are:

LD=ld CROSS_COMPILE=1 CXX="clang++ -Qunused-arguments -arch i386" RANLIB=ranlib CC="clang -Qunused-arguments -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments" HOST_CXX="clang++ -Qunused-arguments" sh ./configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/4e6f9d2589dc
user:        Kannan Vijayan
date:        Thu Sep 05 11:37:07 2013 -0400
summary:     Bug 906805 - Implement Baseline JSOP_GETELEM handlers which invoke getters. try 2. r=efaust

Kannan, is bug 906805 a likely regressor?
Flags: needinfo?(kvijayan)
Yeah, looking into this.
Flags: needinfo?(kvijayan)
Fix.
Attachment #801695 - Flags: review?(efaustbmo)
See Also: → 914127
Comment on attachment 801695 [details] [diff] [review]
fix-bug-913978.patch

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

r=me with some thought given to other potential assertion-only problem locations in light of the new invocation site.

::: js/src/jit/IonFrames.cpp
@@ +1469,5 @@
>          parent.baselineScriptAndPc(NULL, &pc);
>  
> +        // Inlined Getters and Setters are never constructing.
> +        // Baseline may call getters from [GET|SET]PROP or [GET|SET]ELEM ops.
> +        if (IsGetterPC(pc) || IsSetterPC(pc) || IsGetElemPC(pc) || IsSetElemPC(pc))

Are there other places that need to be looked at? Bailouts for example? I really don't know, but we should investigate while we have our minds on this.

::: js/src/jsopcode.h
@@ +600,2 @@
>  inline bool
>  IsSetterPC(jsbytecode *pc)

This is not your fault, but this is growing to be a poor name for this function, for something that lives in the js:: namespace. It's not limited to Ion code, but it functionally is, as these new values /are/ getter and setter PCs, but we fear relaxing assertions. Mostly I'm just whining about the state of things, but I'm also open to renaming or moving suggestions.
Attachment #801695 - Flags: review?(efaustbmo) → review+
(In reply to Eric Faust [:efaust] from comment #3)
> Are there other places that need to be looked at? Bailouts for example? I
> really don't know, but we should investigate while we have our minds on this.

Thanks for reminding me of this.  Checked out the bailout code.  At this point, ELEM ops can't inline, so they won't show up within the bailout frame unpacking.  They can show up as the caller (outer wrapping frame) of a bailout sequence, but the only place where this is checked is |iter.isConstruction|, which this patch fixes.


> This is not your fault, but this is growing to be a poor name for this
> function, for something that lives in the js:: namespace. It's not limited
> to Ion code, but it functionally is, as these new values /are/ getter and
> setter PCs, but we fear relaxing assertions. Mostly I'm just whining about
> the state of things, but I'm also open to renaming or moving suggestions.

IsGetterPC and IsSetterPC can become IsGetPropPC and IsSetPropPC.
Duplicate of this bug: 914127
https://hg.mozilla.org/mozilla-central/rev/7284c374c28d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Depends on: 915167
Assignee: general → kvijayan
You need to log in before you can comment on or make changes to this bug.