1024567 - Assertion failure: !js::gc::IsInsideNursery(this), at jsobj.h:684

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 9e8e3e903484 (threadsafe build, run with --fuzzing-safe):


test([0], "map", function (... Object)  {
  for (var i = 0; function() {} ^ this ; i++) {}
});
function test(arr, op, func) {
  for (var i = 0; i < 1000; i++)
    arr[op + "Par"].apply(arr, [func, undefined]);
}

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

Marked s-s because this is GC-related.

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140609215111" and the hash "a98e6f80344c".
The "bad" changeset has the timestamp "20140609220510" and the hash "573458d10426".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a98e6f80344c&tochange=573458d10426

Comment 4

[Christian Holler (:decoder)]

Needinfo from Lars based on bisection in comment 3.

Comment 5

[Lars T Hansen [:lth]]

Seems probable that this could be the same underlying cause as bug 1024756, since this test case uses rest arguments and those were affected by the fix to that bug.

Comment 6

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 48eee276b1ee).

Comment 7

[Lars T Hansen [:lth]]

Reproduces easily here (Linux64 again) with --fuzzing-safe in rev 9e8e3e903484, debug no-optimize build.

The problem is that the assert is not valid with the new PJS GC.  It was valid before, when PJS allocated directly into the tenured area.

The problem is present in the latest code, according to dxr.  Whether it is reproducible probably depends somewhat on GC timing, though for this test case I fail to see how it could not reproduce.  It is reproducible in revision 48eee2 locally (reproduces on every attempt).

This is not a security bug, just a stale assertion - the code is correct, from what I can tell.  :decoder or somebody with privileges, please clear the security flag.

Comment 8

[Lars T Hansen [:lth]]

Also reproduces immediately on Linux64 debug no-optimize build with --fuzzing-safe in current mozilla-inbound tip (rev d591d9ed638b).

Comment 9

[Lars T Hansen [:lth]]

Also reproduces without --fuzzing-safe, thankfully (I'd have no way of explaining it if that were not the case).

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Lars T Hansen [:lth] from comment #7)
> :decoder or somebody with privileges, please clear the security flag.

Done.

Comment 11

[Lars T Hansen [:lth]]

Attachment: Patch, with test case

Small changes + test case.  A little extra churn because I had to move a function from jsobj.h to jsobjinlines.h.

Ongoing try run off mozilla-inbound:
https://tbpl.mozilla.org/?tree=Try&rev=26873d31d968

Comment 12

[Shu-yu Guo [:shu]]

Comment on attachment 8440565 [details] [diff] [review]
Patch, with test case

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

I'm lukewarm on the patch, but since it's just a DEBUG assert fix, r=me.

::: js/src/jsgc.h
@@ +1293,5 @@
> +// This tests whether something is inside the GGC's nursery only;
> +// use sparingly, mostly testing for any nursery, using IsInsideNursery,
> +// is appropriate.
> +bool
> +IsInsideGGCNursery(const gc::Cell *cell);

This is... okay, I guess. I think I would rather see something like |IsPostBarrierNeededOnCell| than exposing asking for which nursery a Cell is in.

Comment 13

[Lars T Hansen [:lth]]

(In reply to Shu-yu Guo [:shu] from comment #12)
> Comment on attachment 8440565 [details] [diff] [review]
> Patch, with test case
> 
> Review of attachment 8440565 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> [...]
>
> This is... okay, I guess. I think I would rather see something like
> |IsPostBarrierNeededOnCell| than exposing asking for which nursery a Cell is
> in.

I agree that this feels like a bit of a hack (though I'll land it anyway).  I've made an annotation on bug 1020290 since that pertains to cleaning up the situation around having multiple nurseries.

(Try run is green, see comment 11.)

Comment 14

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/46ce2f922051

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/46ce2f922051