716042 - GC: missing barriers in TradeGuts

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Trigger post barriers when we TradeGuts.  Pre barriers are taken care of already by manually marking.  We need similar treatment for post barriers, taking care to handle fixed slots correctly.

Comment 1

[Terrence Cole [:terrence]]

Attachment: v1

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 586561 [details] [diff] [review]
v1

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

::: js/src/jsobj.cpp
@@ +3578,5 @@
> +         * below, in common with the other case.
> +         */
> +        for (size_t i = 0; i < a->numFixedSlots(); ++i) {
> +            Value *slotA = const_cast<Value *>(a->getFixedSlot(i));
> +            Value *slotB = const_cast<Value *>(b->getFixedSlot(i));

I think this would look better if you did:
    HeapValue *slotA = &a->getFixedSlotRef(i);

@@ +3642,5 @@
> +#ifdef JSGC_GENERATIONAL
> +    Shape::writeBarrierPost(a->shape_, (void *)&a->shape_);
> +    Shape::writeBarrierPost(b->shape_, (void *)&b->shape_);
> +    types::TypeObject::writeBarrierPost(a->type_, (void *)&a->type_);
> +    types::TypeObject::writeBarrierPost(b->type_, (void *)&b->type_);

I don't think you need these (void *) casts. I think they're used in gc/Barrier.h because some of the stuff is const.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Also, please don't put two spaces after the period in the comment. See the SpiderMonkey style guide.

Comment 4

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3a0eab64d04

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e3a0eab64d04