bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

GC: Fix some more shell rooting hazards

RESOLVED FIXED in mozilla23

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 743152 [details] [diff] [review]
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.
Attachment #743152 - Flags: review?(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.
Attachment #743152 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/6c23b6504db0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.