Closed Bug 831581 Opened 7 years ago Closed 7 years ago

GC: rooting analysis failure in jaeger/testPropCallElem.js with --ion-eager

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file)

With gcc-4.6.3 run with --ion-eager:

#0  0x0000000000407c27 in js::Shape::isNative (this=0x7fffdad33948) at ../../mozilla/js/src/jsscope.h:543
#1  0x0000000000409192 in js::ObjectImpl::isNative (this=0x7ffff5d37680) at ../../mozilla/js/src/vm/ObjectImpl-inl.h:205
#2  0x0000000000919bed in IsPropertyInlineable (obj=(JSObject *) 0x7ffff5d37680 Cannot access memory at address 0x7fffdad33948, cache=...) at /home/terrence/moz/branch/green_our_tests/mozilla/js/src/ion/IonCaches.cpp:1400
#3  0x000000000091a36e in js::ion::SetPropertyCache (cx=0xf91900, cacheIndex=0x1, obj=(JSObject * const) 0x7ffff5d37680 Cannot access memory at address 0x7fffdad33948, value=$jsval((JSObject *) 0x7ffff5d37700 [object Function "a.b"]), 
    isSetName=0x0) at /home/terrence/moz/branch/green_our_tests/mozilla/js/src/ion/IonCaches.cpp:1522
#4  0x00007ffff7feba20 in ?? ()
#5  0x00007fffffffb3b0 in ?? ()
#6  0x00007fffffffb398 in ?? ()
#7  0xfffafffff5e00c60 in ?? ()
#8  0x0000000000f7d3a0 in ?? ()
#9  0x00007ffff5e2e430 in ?? ()
#10 0x00007ffff7fda650 in ?? ()
#11 0x00000000000005c0 in ?? ()
#12 0x0000000000000001 in ?? ()
#13 0x00007ffff5d37680 in ?? ()
#14 0xfffbfffff5d37700 in ?? ()
#15 0x0000000000000000 in ?? ()
Sean and I managed to track this down to the shape pointer being poisoned while stored in the assembly buffer's inline buffer during compilation of an IC. I'm not sure what we need to do here yet.

I expect the other tests under bug 831580 are hitting the same failure.
Assignee: general → terrence
Attached patch v0Splinter Review
As promised.
Attachment #703673 - Flags: review?(bhackett1024)
Comment on attachment 703673 [details] [diff] [review]
v0

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

::: js/src/assembler/assembler/AssemblerBuffer.h
@@ +64,5 @@
>              : m_buffer(m_inlineBuffer)
>              , m_capacity(inlineCapacity)
>              , m_size(0)
>              , m_oom(false)
> +            , m_skipInline(js::TlsPerThreadData.get(), &m_inlineBuffer)

Will this TlsPerThreadData access be optimized away in builds which nop the SkipRoot?  If not, can you #ifdef JSGC_ROOT_ANALYSIS the field?

::: js/src/gc/RootMarking.cpp
@@ +78,5 @@
>  MarkExactStackRoots(JSTracer *trc)
>  {
>      for (unsigned i = 0; i < THING_ROOT_LIMIT; i++) {
> +        for (ContextIter cx(trc->runtime); !cx.done(); cx.next())
> +            MarkExactStackRootList(trc, cx->thingGCRooters[i], ThingRootKind(i));

Oof, I think I had to make these exact changes twice today.
Attachment #703673 - Flags: review?(bhackett1024) → review+
(In reply to Brian Hackett (:bhackett) from comment #3)
> Comment on attachment 703673 [details] [diff] [review]
> ::: js/src/assembler/assembler/AssemblerBuffer.h
> @@ +64,5 @@
> >              : m_buffer(m_inlineBuffer)
> >              , m_capacity(inlineCapacity)
> >              , m_size(0)
> >              , m_oom(false)
> > +            , m_skipInline(js::TlsPerThreadData.get(), &m_inlineBuffer)
> 
> Will this TlsPerThreadData access be optimized away in builds which nop the
> SkipRoot?  If not, can you #ifdef JSGC_ROOT_ANALYSIS the field?

Good catch. I wanted to move the TLS access into SkipRoot, but then we'd need jsfriendapi.h to be included in Root.h, which isn't acceptable. The #ifdef is definitely the better option.
 
> ::: js/src/gc/RootMarking.cpp
> @@ +78,5 @@
> >  MarkExactStackRoots(JSTracer *trc)
> >  {
> >      for (unsigned i = 0; i < THING_ROOT_LIMIT; i++) {
> > +        for (ContextIter cx(trc->runtime); !cx.done(); cx.next())
> > +            MarkExactStackRootList(trc, cx->thingGCRooters[i], ThingRootKind(i));
> 
> Oof, I think I had to make these exact changes twice today.

Yeah, same story here with the exact rooting bits that you have in Bug 832042 :-).
https://hg.mozilla.org/mozilla-central/rev/fb87010ac009
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.