714913 - GC: missing barriers in Generator's Stack Frame

Status: RESOLVED DUPLICATE of bug 714109

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Typically the values in StackFrames live on the stack.  When we store them in a generator, however, we need to add all of the internal pointers into the Nursery to the remembered set.  Because of this dual use, we need to apply the barriers manually, but only when the StackFrame is stored inside of a generator.

Comment 1

[Terrence Cole [:terrence]]

Attachment: V1

This looks like we discussed IRL.

Comment 2

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

Comment on attachment 585613 [details] [diff] [review]
V1

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

::: js/src/vm/Stack.cpp
@@ +192,5 @@
> +        }
> +    }
> +    if (hasReturnValue())
> +        HeapValue::writeBarrierPost(rval_, &rval_);
> +}

Instead of this function, would it be possible to write an operator= for StackFrame? It would have the same branch structure, but instead of the writeBarrierPost calls it would use assignment. If it could be made to work, it seems like it would be cleaner than this.

Comment 3

[Terrence Cole [:terrence]]

An intriguing thought.  How would we tell when the target is stored on the stack?  I think it would be okay to use an explicit memcpy in pushGeneratorFrame instead of explicitly barriering in js_NewGenerator and popGeneratorFrame.  Is this preferable?

Comment 4

[Terrence Cole [:terrence]]

Comment on attachment 585613 [details] [diff] [review]
V1

On Luke's advise, I think I can make this much prettier with a template.  Cancelling review until I get a chance to rewrite this a bit.

Comment 5

[Terrence Cole [:terrence]]

Merged these changes into 714109 and 715943 to both get the important stuff in-tree faster and make reviewing the larger chunk easier.