Closed
Bug 743406
Opened 12 years ago
Closed 12 years ago
GC: only set tracing location if we call Mark
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(1 file, 2 obsolete files)
3.26 KB,
patch
|
Details | Diff | Splinter Review |
If we set the trace location, but then don't call mark (for example because !v->isMarkable()), then the location won't be unset and will be wrong for the next mark function we call.
Attachment #613047 -
Flags: review?(wmccloskey)
Assignee | ||
Comment 1•12 years ago
|
||
Sorry about that!
Attachment #613047 -
Attachment is obsolete: true
Attachment #613047 -
Flags: review?(wmccloskey)
Attachment #613049 -
Flags: review?(wmccloskey)
Assignee | ||
Comment 2•12 years ago
|
||
Comment on attachment 613049 [details] [diff] [review] v1: forgot to qref Actually, it seems that this doesn't make much sense for the nested call from MapObject::mark to MarkValueUnbarriered -- in this case we clobber the real address with the stack address. I'm going to give this some thought over the weekend and try to solve this correctly.
Attachment #613049 -
Flags: review?(wmccloskey)
Assignee | ||
Comment 3•12 years ago
|
||
Had to broaden the condition allowing us to set realLocation. It now allows only the outermost call and the reset call after it is used. Unfortunately, it is still pretty subtle to use correctly: e.g. if you are going to mark a Value, you need to check if it isMarkable before calling JS_SET_TRACING_LOCATION because we only clear after marking currently. We could add an unconditional reset at the end of MarkValueInternal and MarkIdInternal, but this data flow is rapidly turning into spaghetti.
Attachment #613049 -
Attachment is obsolete: true
Assignee | ||
Comment 4•12 years ago
|
||
This was a lot simpler than I was making it. Added the trivial fix to: https://hg.mozilla.org/integration/mozilla-inbound/rev/0cd6ed757687 since it was needed there.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•