807458 - Eliminate a SkipRoot from NewDenseCopiedArray

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

The values being copied need to be rooted.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Eliminate a SkipRoot from NewDenseCopiedArray

The parameter ordering is pretty wonky, but fixing that is a much larger patch. If you think I should do it, that's fine -- I had to generate that patch first to verify that all callers of the already-rooted version really are already rooted.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 677151 [details] [diff] [review]
Eliminate a SkipRoot from NewDenseCopiedArray

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

Nice!

::: js/src/jsapi.cpp
@@ +4707,5 @@
>  
>  JS_PUBLIC_API(JSObject *)
>  JS_NewArrayObject(JSContext *cx, int length, jsval *vector)
>  {
> +    AutoArrayRooter tvr(cx, length, vector);

Effectively we just moved the SkipRoot forward to here. :-)  Still a win though.

::: js/src/jsstr.cpp
@@ +2757,5 @@
>  
>      /* Step 10. */
>      if (!sepDefined) {
> +        RootedValue v(cx, StringValue(str));
> +        JSObject *aobj = NewDenseCopiedArray(cx, 1, v.address());

That's going to fix one of our fuzz bugs as well.

Comment 3

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/8fd8e9243788

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8fd8e9243788