834766 - GC: Fix some more rooting issues found by static analysis

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Proposed fix

Comment 2

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

Comment on attachment 706453 [details] [diff] [review]
Proposed fix

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

::: js/src/jsstr.cpp
@@ +3143,3 @@
>         CallReceiver call)
>  {
> +    RootedLinearString param(cx, paramArg);

I'd rather you switch the param to a Handle. Most of the callers will pass NullPtr() anyway.

::: js/src/jswrapper.cpp
@@ +418,3 @@
>                                                 PropertyDescriptor *desc, unsigned flags)
>  {
> +    RootedObject wrapper(cx, wrapperArg);

How hard would it be to Handle-ify all the params in jswrapper.cpp? They're used in some funky macros, so I couldn't easily tell. If it's a pain, this is fine for now, but I wanted to ask before giving an r+.

::: js/src/vm/ObjectImpl.cpp
@@ +85,3 @@
>                                      PropDesc *unwrapped) const
>  {
> +    RootedObject obj(cx, objArg);

This should definitely be a Handle too. The callers pass a RootedObject, though the declaration is hidden in a macro.

@@ +132,3 @@
>                     PropDesc *desc) const
>  {
> +    RootedObject obj(cx, objArg);

Same. (Same RootedObjects are passed in, even.)

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Proposed fix v2

Thanks for the comments.

I didn't Handleify the jswrapper stuff because it would involve changing the whole API.  I've split that part out from this patch and will attempt that separately.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Also root some out params

Comment 5

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e64eba37805
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6367c0f3f2b

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0e64eba37805
https://hg.mozilla.org/mozilla-central/rev/f6367c0f3f2b