1156914 - MacroAssembler::pushValue(const Address&) is a footgun on 32 bit systems

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Eric Faust [:efaust]]

Attachment: Fix

On ARM, it asserts that the base is not the stack pointer, which makes sense, since it has to push the value in two instructions, and the address passed will be invalidated.

There is no such assertion on X86. The result is simply bogus.

Fix it up so that they both work properly.

Comment 1

[Jan de Mooij [:jandem]]

Comment on attachment 8595495 [details] [diff] [review]
Fix

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

Another footgun gone, nice.

::: js/src/jit/arm/MacroAssembler-arm.h
@@ +59,5 @@
> +        Register baseReg = Register::FromCode(base.base());
> +        // If we are based on StackPointer, pass over the type tag just pushed.
> +        if (baseReg == StackPointer)
> +            return Operand(Regsiter::FromCode(base.base()), base.disp() + sizeof(void*));
> +        else

Nit: Register (typo) and rm else-after-return

::: js/src/jit/x86/MacroAssembler-x86.h
@@ +72,5 @@
> +        // first push.
> +        if (addr.base == StackPointer)
> +            return Operand(address.base, address.offset + 4);
> +        else 
> +            return payloadOf(address);

Nit: some trailing whitespace and remove the else-after-return

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/95f9ab2b60e1

Comment 5

[Pulsebot]

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

Comment 6

[Phil Ringnalda (:philor)]

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