815652 - IonMonkey: Assertion failure: addr % Cell::CellSize == 0, at ../../gc/Heap.h:818

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 1489b6c2d1d2 (run with --ion-eager):


gczeal(9, 2)
function testScatterConflict() {
  var p = new ParallelArray([1,2,3,4,5]);
  var r = p.scatter([0,1,0,3,(0)], 9, function (a,b) { return a+b; });
  function assertEqParallelArray(a, b)
    assertEq(a instanceof ParallelArray, true);
  assertEqParallelArray(r, new ParallelArray([4,2,(false),4,5]));
}
testScatterConflict();

Comment 1

[Christian Holler (:decoder)]

Marking s-s due to GC involved. The opt build crashed with a less reduced version, but now it only shows some unconditional jumps in valgrind:

==3037== Conditional jump or move depends on uninitialised value(s)
==3037==    at 0x8283E8D: js::gc::MarkGCThingRoot(JSTracer*, void**, char const*) (Marking.cpp:334)
==3037==    by 0x83806D2: js::ion::AutoTempAllocatorRooter::trace(JSTracer*) (IonFrames.cpp:666)
==3037==    by 0x80C57B3: JS::AutoGCRooter::traceAll(JSTracer*) (jsgc.cpp:2382)
==3037==    by 0x80C586A: _ZL11MarkRuntimeP8JSTracerb.isra.312 (jsgc.cpp:2470)
==3037==    by 0x80CEA50: IncrementalCollectSlice(JSRuntime*, long long, js::gcreason::Reason, js::JSGCInvocationKind) (jsgc.cpp:3395)
==3037==    by 0x80CF503: GCCycle(JSRuntime*, bool, long long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:4558)
==3037==    by 0x80CF952: _ZL7CollectP9JSRuntimebxN2js18JSGCInvocationKindENS1_8gcreason6ReasonE.part.355 (jsgc.cpp:4673)
==3037==    by 0x80D0559: js::gc::RunDebugGC(JSContext*) (jsgc.cpp:4596)
==3037==    by 0x8348EE9: js::ion::IonCode::New(JSContext*, unsigned char*, unsigned int, JSC::ExecutablePool*) (jsgcinlines.h:479)
==3037==    by 0x847BCBA: js::ion::CodeGenerator::link() (IonLinker.h:55)
==3037==    by 0x834C20C: js::ion::SequentialCompileContext::compile(js::ion::IonBuilder*, js::ion::MIRGraph*, js::ion::AutoDestroyAllocator&) (Ion.cpp:1192)
==3037==    by 0x834CB62: _ZN2js3ionL10IonCompileINS0_24SequentialCompileContextEEEbP9JSContextP8JSScriptP10JSFunctionPhbRT_.constprop.387 (Ion.cpp:1142)
==3037== 
==3037== Conditional jump or move depends on uninitialised value(s)
==3037==    at 0x83806D7: js::ion::AutoTempAllocatorRooter::trace(JSTracer*) (IonFrames.cpp:665)
==3037==    by 0x80C57B3: JS::AutoGCRooter::traceAll(JSTracer*) (jsgc.cpp:2382)
==3037==    by 0x80C586A: _ZL11MarkRuntimeP8JSTracerb.isra.312 (jsgc.cpp:2470)
==3037==    by 0x80CEA50: IncrementalCollectSlice(JSRuntime*, long long, js::gcreason::Reason, js::JSGCInvocationKind) (jsgc.cpp:3395)
==3037==    by 0x80CF503: GCCycle(JSRuntime*, bool, long long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:4558)
==3037==    by 0x80CF952: _ZL7CollectP9JSRuntimebxN2js18JSGCInvocationKindENS1_8gcreason6ReasonE.part.355 (jsgc.cpp:4673)
==3037==    by 0x80D0559: js::gc::RunDebugGC(JSContext*) (jsgc.cpp:4596)
==3037==    by 0x8348EE9: js::ion::IonCode::New(JSContext*, unsigned char*, unsigned int, JSC::ExecutablePool*) (jsgcinlines.h:479)
==3037==    by 0x847BCBA: js::ion::CodeGenerator::link() (IonLinker.h:55)
==3037==    by 0x834C20C: js::ion::SequentialCompileContext::compile(js::ion::IonBuilder*, js::ion::MIRGraph*, js::ion::AutoDestroyAllocator&) (Ion.cpp:1192)
==3037==    by 0x834CB62: _ZN2js3ionL10IonCompileINS0_24SequentialCompileContextEEEbP9JSContextP8JSScriptP10JSFunctionPhbRT_.constprop.387 (Ion.cpp:1142)
==3037==    by 0x834D029: _ZN2js3ionL7CompileEP9JSContextP8JSScriptP10JSFunctionPhb.part.380 (Ion.cpp:1325)


One thing I'm wondering about if the gczeal level here. I cannot decrease it to 4, but this is also not a root analysis build and to my knowledge, higher gczeal levels should only work in those builds?

Comment 2

[Terrence Cole [:terrence]]

(In reply to Christian Holler (:decoder) from comment #1)
> One thing I'm wondering about if the gczeal level here. I cannot decrease it
> to 4, but this is also not a root analysis build and to my knowledge, higher
> gczeal levels should only work in those builds?

Only 6 and 7 require a root analysis build.  According to JS_GC_ZEAL=help, zeal mode 9 is "Incremental GC in two slices: 1) mark all 2) new marking and finish", which I think was added by Jon awhile back.

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, failed due to error (try manually).

Comment 4

[Shu-yu Guo [:shu]]

This crashes while marking CompilerRoots. I have had no success in trying to determine how ParallelArray comes into play here.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: fix

We were constructing a temporary CompilerRoot object in MInstanceOf and then invoking operator= to copy it to the real one.

This is sort of a dangerous pattern. If we ever accidentally use one of these as a temporary, we'll crash. Hopefully the private stuff will make this less likely.

Comment 6

[Brian Hackett [Laid off!]]

Comment on attachment 685779 [details] [diff] [review]
fix

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

::: js/src/ion/CompilerRoot.h
@@ +45,5 @@
> +
> +  private:
> +    CompilerRoot();
> +    CompilerRoot(const CompilerRoot<T> &);
> +    CompilerRoot<T> &operator =(const CompilerRoot<T> &);

Can these be MOZ_DELETE?

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59281806d3a6

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/59281806d3a6

Comment 9

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 10

[bhavana bajaj [:bajaj]]

What's the rating on this bug? Do we think that B2G is impacted?

Comment 11

[Daniel Veditz [:dveditz]]

What is the severity of this bug -- do we need to take it on branches (particularly the Fx18-based B2g branch)? It should not have landed without a sec-approval request unless it was rated less than sec-high or sec-critical (and unrated counts as "possibly critical in worst case").

Comment 12

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

Was 19 affected by this? Did we ship this problem?

Comment 13

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This regressed in bug 814861, which landed for 20. It was backported to 18 and 19, but the backports had the fix for this bug.