TM: when f.length is resolved at record time, we can fail to emit a shape guard

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

Other Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
Luke found this test case:

var x = 0, y = 0, f;
for (var i = 0; i < 100; i++) {
    f = function() {};
    f.foo;
    x + f.length;
}

When recording f.length, we trigger a resolve hook which changes f's shape without calling forgetGuardedShapesForObject, which is necessary to invalidate the guardedShapeTable entry for f.

The straightforward fix is to add another call to forgetGuardedShapesForObject in each place where a shape changes that can be reached at record time.
(Assignee)

Comment 1

8 years ago
Created attachment 463173 [details] [diff] [review]
v1

Here is, probably, the shortest possible fix. It means emitting less-than-ideal code, but this case is rare to begin with.

(I tried narrowly checking aobj's shape before and after the lookup, and it fixed the test case at hand, but I was unable to convince myself that was correct. The lookup could change the shapes of other objects.)
Assignee: general → jorendorff
Attachment #463173 - Flags: review?(brendan)
(Assignee)

Comment 2

8 years ago
No difference on SunSpider or V8.
Comment on attachment 463173 [details] [diff] [review]
v1

Worth commenting with a "FIXME: bug 458271" comment?

/be
Attachment #463173 - Flags: review?(brendan) → review+
(Assignee)

Comment 4

8 years ago
FIXME comment added.

http://hg.mozilla.org/tracemonkey/rev/324b0d7b7ded
Summary: TM: when f.length is resolved at run time, we can fail to emit a shape guard → TM: when f.length is resolved at record time, we can fail to emit a shape guard

Updated

8 years ago
Duplicate of this bug: 571626

Comment 6

8 years ago
http://hg.mozilla.org/mozilla-central/rev/324b0d7b7ded
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.