Closed Bug 845519 Opened 7 years ago Closed 7 years ago

Rootanalysis build failures

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: sfink, Assigned: sfink)

References

Details

Attachments

(4 files)

The rootanalysis build is still failing.
The build is being run on gcc 4.5 with optimization. That triggers a reordering of operations within Rooted<JSObject*> and Rooted<DerivedFromJSObject*>'s constructors. This patch changes the types of their fields to be the same.
Attachment #718624 - Flags: review?(bhackett1024)
Attachment #718624 - Flags: review?(bhackett1024) → review+
getType(cx) is used all over the place without checking its return value. Getting the right return value for all of the places it can fail is tricky.
Attachment #718626 - Flags: review?(bhackett1024)
Comment on attachment 718626 [details] [diff] [review]
Check getType(cx) return value (also fixes rooting hazard)

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

::: js/src/ion/IonBuilder.cpp
@@ +3871,5 @@
>  IonBuilder::getSingletonPrototype(JSFunction *target)
>  {
>      if (!target->hasSingletonType())
>          return NULL;
> +    types::TypeObject *targetType = target->getType(cx);

targetType should be null checked here

::: js/src/jsobj.h
@@ +445,5 @@
>       * Constructs a new, unique shape for the object.
>       */
>      static inline bool setSingletonType(JSContext *cx, js::HandleObject obj);
>  
> +    inline js::Unrooted<js::types::TypeObject*> getType(JSContext *cx);

rm the Unrooted.  I think the plan is to get rid of these, it'd be good to avoid adding more.

::: js/src/jsobjinlines.h
@@ +757,5 @@
>      obj->type_ = type;
>      return true;
>  }
>  
> +inline js::Unrooted<js::types::TypeObject*>

Ditto.
Attachment #718626 - Flags: review?(bhackett1024) → review+
Hm. Should we have a bug on file?
Attachment #718694 - Flags: review?(terrence)
(In reply to Brian Hackett (:bhackett) from comment #3)
> Comment on attachment 718626 [details] [diff] [review]
> Check getType(cx) return value (also fixes rooting hazard)
> 
> targetType should be null checked here

fixed

> rm the Unrooted.  I think the plan is to get rid of these, it'd be good to
> avoid adding more.

sorry; I meant to rip these out. They were from the initial investigation.
Comment on attachment 718694 [details] [diff] [review]
Use SkipRoots for jschar pointers until we need to start moving them

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

::: js/src/frontend/TokenStream.h
@@ +757,5 @@
>          const jschar *base_;            /* base of buffer */
>          const jschar *limit_;           /* limit for quick bounds check */
>          const jschar *ptr;              /* next char to get */
> +
> +        // We are not yet moving strings

Missing a . at the end.
Attachment #718694 - Flags: review?(terrence) → review+
The stack of patches in this bug are enough to get a green root analysis build.

https://tbpl.mozilla.org/?tree=Try&rev=f4de277690fd
Attachment #719636 - Flags: review?(terrence)
Attachment #719636 - Flags: review?(terrence) → review+
Attachment #718624 - Flags: checkin+
Attachment #718626 - Flags: checkin+
Attachment #718694 - Flags: checkin+
Attachment #719636 - Flags: checkin+
You need to log in before you can comment on or make changes to this bug.