873209 - Should root vp for specialized DOM getters

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

They certainly seem to assume it's rooted when they do things like:

  *vp = something;
  return JS_WrapValue(cx, vp);

right?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Trace the *vp of specialized DOM getters in Ion.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 750631 [details] [diff] [review]
Trace the *vp of specialized DOM getters in Ion.

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

::: js/src/ion/CodeGenerator.cpp
@@ +6242,5 @@
>  
>      masm.checkStackAlignment();
>  
> +    /* Make space for the outparam.  Pre-initialize it to
> +       UndefinedValue so we can trace it at GC time. */

Pre-existing nit: //-style comment

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Pre-existing nit: //-style comment

Fixed.  Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/2a1149f381ed and then backed it out: https://hg.mozilla.org/integration/mozilla-inbound/rev/989f42c1bc87

This was hitting the assert at the end of visitGetDOMProperty:

    JS_ASSERT(masm.framePushed() == initialStack);

The reason for that is that pushValue does NOT adjust the value returned by masm.framePushed().

Should it?  If not, should I be doing something else in this code?  Looking around, maybe what I really wanted was:

    masm.Push(UndefinedValue());

but I'd like to verify that before I push this again.

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Boris Zbarsky (:bz) from comment #3)
> Should it?  If not, should I be doing something else in this code?  Looking
> around, maybe what I really wanted was:
> 
>     masm.Push(UndefinedValue());
> 
> but I'd like to verify that before I push this again.

Ah, yeah masm.Push(UndefinedValue()) is what you want.

Comment 5

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6e13bdaea06

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f6e13bdaea06