MSimdUnbox should have the Guard flag set when it's fallible

RESOLVED FIXED in Firefox 40

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sunfish, Assigned: bbouvier)

Tracking

unspecified
mozilla40
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Currently, IonBuilder::inlineSimdCheck manually sets the Guard flag when creating an MSimdUnbox. However, EagerSimdUnbox.cpp and TypePolicy.cpp don't currently set the Guard flag for their SIMD unbox operations, even though they're still supposed to throw if their operand is not actually SIMD.

It seems that MSimdUnbox should set he Guard flag in its constructor as most other classes that use it do, since looking at the CodeGenerator code for LSimdUnbox, it appears that all LSimdUnbox instances are fallible.
(Assignee)

Comment 1

3 years ago
If anybody wants to take it, feel free, otherwise i'll do it when i'm back from pto.
Flags: needinfo?(benj)
(Assignee)

Comment 2

3 years ago
So the patch is easy, but finding a test case that triggers the issue without the patch isn't...
Assignee: nobody → benj
Status: NEW → ASSIGNED
Flags: needinfo?(benj)
(Assignee)

Comment 3

3 years ago
Created attachment 8592278 [details] [diff] [review]
unbox-guard.patch

With a test case that triggers on inbound but not with this patch! (it was hard to trigger, as SIMD values are objects and thus bailouts could be produced by typebarriers or arguments checks)
Attachment #8592278 - Flags: review?(sunfish)
(Reporter)

Comment 4

3 years ago
Comment on attachment 8592278 [details] [diff] [review]
unbox-guard.patch

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

Nice job finding a suitably obstreperous testcase :-).
Attachment #8592278 - Flags: review?(sunfish) → review+
(Assignee)

Comment 5

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1fd18912545
https://hg.mozilla.org/mozilla-central/rev/d1fd18912545
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.