Last Comment Bug 715201 - GC: missing barriers in BaseShape's new constructor
: GC: missing barriers in BaseShape's new constructor
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Terrence Cole [:terrence]
:
Mentors:
Depends on:
Blocks: 673454
  Show dependency treegraph
 
Reported: 2012-01-04 10:22 PST by Terrence Cole [:terrence]
Modified: 2012-01-05 08:43 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (1.94 KB, patch)
2012-01-04 10:22 PST, Terrence Cole [:terrence]
bhackett1024: review+
Details | Diff | Splinter Review
v2: Fixes applied. (1.87 KB, patch)
2012-01-04 14:41 PST, Terrence Cole [:terrence]
terrence: review+
Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2012-01-04 10:22:24 PST
Created attachment 585805 [details] [diff] [review]
v1

BaseShape's new copy constructor for StackBaseShape needs to be write barriered
like the existing constructor.
Comment 1 Brian Hackett (:bhackett) 2012-01-04 10:29:51 PST
Comment on attachment 585805 [details] [diff] [review]
v1

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

::: js/src/jsscopeinlines.h
@@ +104,5 @@
>      this->flags = base.flags;
>      this->rawGetter = base.rawGetter;
>      this->rawSetter = base.rawSetter;
> +    if ((base.flags & HAS_GETTER_OBJECT) && base.rawGetter) {
> +        this->flags |= HAS_GETTER_OBJECT;

This line is unnecessary.

@@ +108,5 @@
> +        this->flags |= HAS_GETTER_OBJECT;
> +        JSObject::writeBarrierPost(this->getterObj, &this->getterObj);
> +    }
> +    if ((base.flags & HAS_SETTER_OBJECT) && base.rawSetter) {
> +        this->flags |= HAS_SETTER_OBJECT;

Ditto.
Comment 2 Terrence Cole [:terrence] 2012-01-04 14:41:08 PST
Created attachment 585903 [details] [diff] [review]
v2: Fixes applied.

Ah, right, we've already assigned the full flags word, unlike with the other constructor.  Thanks for the review!
Comment 3 Terrence Cole [:terrence] 2012-01-04 14:44:08 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/bb91dfae7007
Comment 4 Marco Bonardo [::mak] 2012-01-05 08:43:24 PST
https://hg.mozilla.org/mozilla-central/rev/bb91dfae7007

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