JIT code can call js_AddProperty when the property already exists (Assertion failure: !SCOPE_HAS_PROPERTY(scope, sprop), at ../jsbuiltins.cpp:257)

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

Other Branch
Points:
---
Bug Flags:
wanted1.9.1.x ?
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
function f1() {}
function C() {}
var x = C.prototype = {m: f1};
x.m();  // brand scope
var arr = [new C, new C, new C, x];
for (var i = 0; i < 4; i++) {
    arr[i].m = 12;
    x.m();  // should throw last time through
}

Without -j:
  crasher.js:10: TypeError: x.m is not a function
With -j in a debug build:
  Assertion failure: !SCOPE_HAS_PROPERTY(scope, sprop), at ../jsbuiltins.cpp:257
With -j in a release build, no output.

A harmless correctness bug in the (release) browser, unless it affects any real-world web pages, which I doubt.
(Assignee)

Comment 1

9 years ago
Two bugs here. As the summary originally said, we assign a non-function to a function-valued property without bumping the shape. But more importantly, we're calling js_AddProperty here on an object whose scope already contains the desired property. By chance this turns out to be safe, though the behavior is wrong.

I would no longer call this "harmless". It's as important to fix as any assertion.
Summary: Assigning non-function to function-valued property on trace does not change shape (Assertion failure: !SCOPE_HAS_PROPERTY(scope, sprop), at ../jsbuiltins.cpp:257) → JIT code can call js_AddProperty when the property already exists (Assertion failure: !SCOPE_HAS_PROPERTY(scope, sprop), at ../jsbuiltins.cpp:257)
(Assignee)

Comment 2

9 years ago
Created attachment 387299 [details] [diff] [review]
v1

Theoretically this makes some property assignments slower, but I saw no change in bench.sh.
Assignee: general → jorendorff
Attachment #387299 - Flags: review?(gal)

Comment 3

9 years ago
Comment on attachment 387299 [details] [diff] [review]
v1

++jorendorff
Attachment #387299 - Flags: review?(gal) → review+

Updated

9 years ago
Flags: wanted1.9.1.x?
(Assignee)

Comment 4

9 years ago
http://hg.mozilla.org/tracemonkey/rev/6661bcf160cb
Whiteboard: fixed-in-tracemonkey

Comment 5

9 years ago
http://hg.mozilla.org/mozilla-central/rev/6661bcf160cb
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 6

8 years ago
trace-test testBug502914
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.