Subclasses of JSNonFinalObject with gc'able children need to implement visitChildren()

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

The ClonedArguments class is missing a visitChildren.  Ditto for the Element class in jsc.cpp.

[Mark Lam]

Created attachment 254374 [details]
the patch.

[Geoffrey Garen]

Comment on attachment 254374 [details]
the patch.

r=me

ClonedArguments should be testable. Just allocate a lot of function.arguments for transient function objects, then force gc, and then access .callee on each one. You'll crash pretty quickly.

[Mark Lam]

(In reply to comment #2)
> ClonedArguments should be testable. Just allocate a lot of
> function.arguments for transient function objects, then force gc, and then
> access .callee on each one. You'll crash pretty quickly.

k, I’ll work on a test.

[Mark Lam]

(In reply to comment #2)
> ClonedArguments should be testable. Just allocate a lot of
> function.arguments for transient function objects, then force gc, and then
> access .callee on each one. You'll crash pretty quickly.

Actually, this turned out to be harder than we thought.  Here's why:
1. With one exception (the StackVisitor), ClonedArguments are only created by op_create_out_of_band_arguments.

2. The BytecodeGenerator will only emit an op_create_out_of_band_arguments if the function is in strict mode.

3. Since ES5, access to arguments.callee is forbidden for a strict mode function.  See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments/callee).

Hence, I can't get a scenario in JS code where I can naturally see this issue manifest.

I mentioned that there was an exception i.e. the StackVisitor, which returns a ClonedArguments object when StackVisitor::arguments() is called.  The remaining question is whether we can build a case around the use of StackVisitor::arguments().

[Mark Lam]

I've found a way to reproduce this now.  The StackVisitor:;Frame::createArguments() (not StackVisitor::arguments()) lead takes me to Function.arguments which does reproduce the issue without strict mode.

[Mark Lam]

Created attachment 254399 [details]
patch for landing with test added

[Mark Lam]

Thanks for the review.  Landed in r185277: <http://trac.webkit.org/r185277>.