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

RESOLVED WONTFIX

Status

()

Core
JavaScript Engine
RESOLVED WONTFIX
7 years ago
5 years ago

People

(Reporter: gwagner, Assigned: gwagner)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

7 years ago
Assignee: general → anygregor
(Assignee)

Comment 1

7 years ago
Created attachment 514356 [details] [diff] [review]
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]
(Assignee)

Updated

7 years ago
Attachment #514356 - Flags: review?(gal)

Comment 2

7 years ago
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?
(Assignee)

Comment 3

7 years ago
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

7 years ago
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.
(Assignee)

Comment 5

7 years ago
Created attachment 514553 [details] [diff] [review]
patch

Move the check to js_TraceStackFrame
Attachment #514356 - Attachment is obsolete: true
Attachment #514356 - Flags: review?(gal)
Blocks: 629821
(Assignee)

Comment 6

7 years ago
Created attachment 514831 [details] [diff] [review]
patch
Attachment #514553 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #514831 - Flags: review?(gal)
Attachment #514831 - Flags: review?(gal)
Seems like this bug must be obsolete by now.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.