855133 - PJS: (ARM) Assertion failure: topIonScript_, at /home/shu/moz/inbound/js/src/ion/Bailouts.h:187

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Shu-yu Guo [:shu]]

Happens in several jit-tests, like flatten-1.

Comment 1

[Shu-yu Guo [:shu]]

This is due to us not implementing parallel bailouts on ARM... at all. The obvious fix doesn't work because something wonky is going on with retargeting multiple labels on ARM to go to a common target, as is the case with |ensureOutOfLineParallelAbort|. Namely, this assertion in Assembler-arm.cpp is failing in |retarget|: 

            DebugOnly<uint32_t> prev = target->use(label->offset());
            JS_ASSERT((int32_t)prev == Label::INVALID_OFFSET);

mjrosenb, could you advise on how to retarget multiple labels to a common, unbound target?

Comment 2

[Shu-yu Guo [:shu]]

Attachment: fix

This seems to work.

Comment 3

[Shu-yu Guo [:shu]]

Attachment: fix

Less horribly wrong version.

Comment 4

[Marty Rosenberg [:mjrosenb]]

Comment on attachment 730054 [details] [diff] [review]
fix

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

The logic in Assembler-arm.cpp is a bit confusing, it could probably use a comment or two?

::: js/src/ion/arm/Assembler-arm.cpp
@@ +2096,5 @@
>              bind(label, BufferOffset(target));
> +        } else if (target->used()) {
> +            // The target is not bound but used. Prepend label's branch list
> +            // onto target's.
> +            int32_t prev = target->use(label->offset());

Could you move the definition of prev closer to where it is used?

Comment 5

[Shu-yu Guo [:shu]]

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

try run: https://tbpl.mozilla.org/?tree=Try&rev=b4563bd3b8e7

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Can we get tests for this?

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> https://hg.mozilla.org/mozilla-central/rev/fa15e3c18ecf
> 
> Can we get tests for this?

This bug was crashing a number of existing ParallelArray tests already, so there wasn't need to make a new one. The problem, I guess, is that jit-tests aren't run on tbpl for ARM, or so mjrosenb tells me.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

What about a jsreftest?

Comment 9

[Shu-yu Guo [:shu]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> What about a jsreftest?

Does a reftest make sense for an ARM-only crash on an experimental/unstandardized API?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

(In reply to Shu-yu Guo [:shu] from comment #9)
> (In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> > What about a jsreftest?
> 
> Does a reftest make sense for an ARM-only crash on an
> experimental/unstandardized API?

Your call, I just get antsy at the thought of possibly regressing a security bug.

Comment 11

[Daniel Veditz [:dveditz]]

Does this affect the b2g18 branch?

Comment 12

[Shu-yu Guo [:shu]]

(In reply to Daniel Veditz [:dveditz] from comment #11)
> Does this affect the b2g18 branch?

It shouldn't, ParallelArray is ifdef'd off on b2g.

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Was this in Firefox 21?

Comment 14

[Daniel Veditz [:dveditz]]

I'm assuming comment 13 was directed at shu

Comment 15

[Shu-yu Guo [:shu]]

PJS is ifdef'd off on everything but Nightly, so no worries on Firefox 21.