636044 - Don't mark scripts of another compartment during per-compartment GC

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: patch

Seen in bug 635873:
 1  mozjs.dll!js::Shape::trace(JSTracer *) [jsscope.cpp : 1488 + 0x30]
    eip = 0x0081a045   esp = 0x0012abe8   ebp = 0x0012abfc
    Found by: call frame info
 2  mozjs.dll!js::Bindings::trace(JSTracer *) [jsscript.cpp : 294 + 0xb]
    eip = 0x0081c380   esp = 0x0012ac04   ebp = 0x0012ac10
    Found by: call frame info
 3  mozjs.dll!js_TraceScript(JSTracer *,JSScript *) [jsscript.cpp : 1709 + 0xe]
    eip = 0x0082036a   esp = 0x0012ac18   ebp = 0x0012ac34
    Found by: call frame info
 4  mozjs.dll!js_TraceStackFrame(JSTracer *,JSStackFrame *) [jsgc.cpp : 1489 +
0x11]
    eip = 0x0073d124   esp = 0x0012ac3c   ebp = 0x0012ac44
    Found by: call frame info
 5  mozjs.dll!js::StackSpace::mark(JSTracer *) [jscntxt.cpp : 242 + 0xc]

Comment 2

[Andreas Gal :gal]

Comment on attachment 514356 [details] [diff] [review]
patch

I don't like this. The script object should not be found either. Why do we even arrive at js_TraceScript? We are coming through an object or do we scan JSScript* directly through the conservative scanner?

Comment 3

[Gregor Wagner [:gwagner]]

Not from the conservative stack scanner. It comes from the JS stack:

 3  mozjs.dll!js_TraceScript(JSTracer *,JSScript *) [jsscript.cpp : 1709 + 0xe]
    eip = 0x0082036a   esp = 0x0012ac18   ebp = 0x0012ac34
    Found by: call frame info
 4  mozjs.dll!js_TraceStackFrame(JSTracer *,JSStackFrame *) [jsgc.cpp : 1489 +
0x11]
    eip = 0x0073d124   esp = 0x0012ac3c   ebp = 0x0012ac44
    Found by: call frame info
 5  mozjs.dll!js::StackSpace::mark(JSTracer *) [jscntxt.cpp : 242 + 0xc]
    eip = 0x006e46eb   esp = 0x0012ac4c   ebp = 0x0012ac6c
    Found by: call frame info
 6  mozjs.dll!JSThreadData::mark(JSTracer *) [jscntxt.cpp : 536 + 0xe]
    eip = 0x006e6116   esp = 0x0012ac74   ebp = 0x0012ac7c
    Found by: call frame info
 7  mozjs.dll!js::MarkRuntime(JSTracer *) [jsgc.cpp : 1739 + 0x12]
    eip = 0x0073e08b   esp = 0x0012ac84   ebp = 0x0012acd0
    Found by: call frame info
 8  mozjs.dll!MarkAndSweepCompartment [jsgc.cpp : 2292 + 0x8]

Comment 4

[Andreas Gal :gal]

I see. So we find a stack frame with a script thats in the wrong compartment. We should abort right there inside the script since all parts of the script are same compartment. No need to scan any of it.

Comment 5

[Gregor Wagner [:gwagner]]

Attachment: patch

Move the check to js_TraceStackFrame

Comment 6

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 7

[David Mandelin [:dmandelin]]

Seems like this bug must be obsolete by now.