Closed Bug 913715 Opened 7 years ago Closed 7 years ago

GenerationalGC: Assertion failure: !UninlinedIsInsideNursery(GetIonContext()->runtime, ptr), at jit/CompilerRoot.h

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: gkw, Assigned: terrence)

References

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file stack
try {
    (function() {
        Object.defineProperty(this, "x", {
            get: function() {
                Object.defineProperty(this, "y", {
                    configurable: true,
                    get: function() {
                        return Proxy(this.y)
                    }
                });
                x;
            }
        })
    })()
    x
} catch (e) {}
try {
    x
} catch (e) {}
try {
    x
} catch (e) {}
try {
    y
} catch (e) {}
try {
    y
} catch (e) {}
try {
    y
} catch (e) {}

asserts js debug shell (tested with a threadsafe deterministic 64-bit debug build) on m-c changeset 77ed46bf4c1a without any CLI arguments at Assertion failure: !UninlinedIsInsideNursery(GetIonContext()->runtime, ptr), at jit/CompilerRoot.h

My configure options are:

--enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-gcgenerational --with-ccache --enable-threadsafe <other NSPR options>
Flags: needinfo?(terrence)
Looks like a nursery thing is flowing somewhere in the compiler where it shouldn't. Taking a look.
Assignee: general → terrence
Flags: needinfo?(terrence)
The |get:function| lambda JSFunction object for "y" gets created in the nursery from the jitcode for "x", working as expected. Then later we go to recompile "x" and decide to inline "y" into it. However, "y"'s call object is still in the nursery. This compilation path tries to attach a nursery thing into the MIR. This is blatantly not allowed, hence the assertion.

Brian, what is the expected behavior here?
Flags: needinfo?(bhackett1024)
Generally, we rely on the type set tests to filter out nursery things read from the GC heap, but that doesn't happen here because getPropTryCommonGetter doesn't look at the type information for the actual getter being read (properties with getters have unknown type information, essentially) and relies on shape checks to do what it wants.  I don't know anywhere else in the builder with this behavior, and the test case situation seems bizarre enough that just having getPropTryCommonGetter or one of its helpers test for a function in the nursery seems ok, if unpalatable.
Flags: needinfo?(bhackett1024)
Excellent, that's exactly what I thought the situation was, but did not want to do something so hackish without being sure.

The patch is a little awkward because getPropTryCommonGetter must succeed if a getter is present or, I think, the getter will be ignored. I chose to duplicate the fallback MCallGetProperty instead of messing with the clean organization of jsop_getprop: adding confusion to that method seems like it would be a terrible idea.
Attachment #812867 - Flags: review?(bhackett1024)
Comment on attachment 812867 [details] [diff] [review]
fuzz_913715-v0.diff

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

::: js/src/jit/IonBuilder.cpp
@@ +8412,5 @@
> +        current->push(call);
> +        if (!resumeAfter(call))
> +            return false;
> +        *emitted = true;
> +        return true;

This stuff isn't necessary, just leave *emitted at false and return true.  This function can't be relied on to succeed because there's never any guarantee that type information is perfect, and later attempts to optimize getprop must do the right thing regardless of whether this was turned on.
Attachment #812867 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f6d42c8c7d2

Ah, that simplifies the patch tremendously!
https://hg.mozilla.org/mozilla-central/rev/7f6d42c8c7d2
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.