IonMonkey: Handle JSOP_THIS primitive this case

RESOLVED FIXED in mozilla26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks 1 bug)

unspecified
mozilla26
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Assignee

Description

6 years ago
IonBuilder aborts if JSOP_THIS is used and |this| is not guaranteed to be an object. It's pretty easy to trigger and the test in bug 856128 contains a small function that we're not Ion-compiling because of this (no pun intended).
Assignee

Comment 1

6 years ago
Posted patch PatchSplinter Review
Refactors BoxNonStrictThis to return a JSObject * and calls this function from Ion.
Attachment #800754 - Flags: review?(bhackett1024)
Attachment #800754 - Flags: review?(bhackett1024) → review+
This patch appears to crash SunSpider 1.0.1.  Also Octane results are less.

https://crash-stats.mozilla.com/report/index/b7390028-8388-4440-99b3-90e912130910

Good : http://hg.mozilla.org/integration/mozilla-inbound/rev/a2013b29212c
Bad  : http://hg.mozilla.org/integration/mozilla-inbound/rev/d660739f7498


Mozilla/5.0 (Windows NT 6.3; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Assignee

Comment 4

6 years ago
Thanks Gary, I will look into the crash. I don't think this should affect Octane though. Which Octane test is slower for you with this patch?
I was wrong about Octane being slower.
Assignee

Comment 6

6 years ago
This seems to fix the crash. TypeScript::ThisTypes contains the original |this| types before wrapping it, so it's incorrect to use that as the result typeset of ComputeThis.
Attachment #802439 - Flags: review?(bhackett1024)
Attachment #802439 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/d660739f7498
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26

Comment 8

6 years ago
As of http://hg.mozilla.org/mozilla-central/rev/f73bed2856a8 
(mozilla-central non-PGO hourly, virgin profile) sunspider still crashes for me.

Instead of crashing immediately, it runs through a few iterations (maybe halfway through?) first, so I suppose it could be a different issue.

32 bit windows build on an underpowered win7 laptop.
Assignee

Comment 9

6 years ago
I didn't land the fix yet, here we go:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab7da1011bf2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 10

6 years ago
Oops, misunderstood comment 7.

Works fine for me on latest inbound pgo hourly.
https://hg.mozilla.org/mozilla-central/rev/ab7da1011bf2
https://hg.mozilla.org/mozilla-central/rev/211b9c5a464d
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Depends on: 914671
Depends on: 916761
Assignee

Updated

6 years ago
No longer depends on: 916761
Depends on: 946243
You need to log in before you can comment on or make changes to this bug.