Last Comment Bug 743406 - GC: only set tracing location if we call Mark
: GC: only set tracing location if we call Mark
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Terrence Cole [:terrence]
:
:
Mentors:
Depends on: 737258
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-06 18:05 PDT by Terrence Cole [:terrence]
Modified: 2012-05-11 10:54 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v0: fixup an oversite (419 bytes, patch)
2012-04-06 18:05 PDT, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review
v1: forgot to qref (2.37 KB, patch)
2012-04-06 18:10 PDT, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review
v2: Proven working with the verifier. (3.26 KB, patch)
2012-04-11 17:54 PDT, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2012-04-06 18:05:52 PDT
Created attachment 613047 [details] [diff] [review]
v0: fixup an oversite

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.
Comment 1 Terrence Cole [:terrence] 2012-04-06 18:10:32 PDT
Created attachment 613049 [details] [diff] [review]
v1: forgot to qref

Sorry about that!
Comment 2 Terrence Cole [:terrence] 2012-04-06 18:23:12 PDT
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.
Comment 3 Terrence Cole [:terrence] 2012-04-11 17:54:10 PDT
Created attachment 614239 [details] [diff] [review]
v2: Proven working with the verifier.

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.
Comment 4 Terrence Cole [:terrence] 2012-05-11 10:54:09 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.