866789 - GC: Fix some more shell rooting hazards

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

Attachment: Proposed changes

This is a patch to fix some of the remaining shell rooting hazards.

Most are self-explanatory, apart from:

jsatom.cpp: I've made the AtomHasher::Lookup use into temporaries so the analysis doesn't warn about the atom field being unrooted (it's always NULL here anyway).

iter.cpp: I renamed obj to resultObj because the analysis seems to get confused when variables shadow each other.

So these are slightly unfortunate but hopefully not too bad.

Comment 1

[Terrence Cole [:terrence]]

Comment on attachment 743152 [details] [diff] [review]
Proposed changes

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

Looks good!

::: js/src/jsatom.cpp
@@ -249,5 @@
>       * hash, allowing us to avoid rehashing it. Even though the hash is
>       * unchanged, we need to re-lookup the table position because a last-ditch
>       * GC will potentially free some table entries.
>       */
> -    AtomHasher::Lookup lookup(tbchars, length);

I like this change as well. Keeping the lookup on the stack has always seemed a bit silly and error prone to me.

::: js/src/jsiter.cpp
@@ +574,5 @@
>  
>          if (!Invoke(cx, ObjectOrNullValue(obj), method, 0, NULL, vp.address()))
>              return false;
>  
> +        RawObject resultObj = ToObject(cx, vp);

Shadowing confuses human readers too, so I'm perfectly fine with a rename.

Comment 2

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c23b6504db0

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6c23b6504db0