Closed Bug 455413 Opened 13 years ago Closed 13 years ago

TM: "Assertion failure: (m != JSVAL_INT) || isInt32(*vp)" with watch, pow

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: dmandelin)

Details

(Keywords: assertion, testcase)

Attachments

(1 file, 1 obsolete file)

~/tracemonkey/js/src/Darwin_DBG.OBJ/js -j 
js> this.watch('x', Math.pow); (function() { for(var j=0;j<4;++j){x=1;} })();

Assertion failure: (m != JSVAL_INT) || isInt32(*vp), at jstracer.cpp:1548
Please make sure to put "TM:" in front of jit bugs.
Summary: "Assertion failure: (m != JSVAL_INT) || isInt32(*vp)" with watch, pow → TM: "Assertion failure: (m != JSVAL_INT) || isInt32(*vp)" with watch, pow
What exactly does watch do here?
I will try to single-step through it.
Assignee: general → brendan
I stole this bug as a start on helping with TM bugs. Feel free to steal it back.
Assignee: brendan → dmandelin
Attached patch Patch (obsolete) — Splinter Review
This patch should make SM refuse to start recording if any watchpoints are active. The check should be cheap; SunSpider with 50 trials came out 1.006x as fast.
Attachment #341032 - Flags: review?(gal)
Attached patch Patch 2Splinter Review
Here's an improved patch that mrbkap suggested. It should more selectively abort only when the specific property being accessed has a watchpoint while still being a cheap check.
Attachment #341032 - Attachment is obsolete: true
Attachment #341043 - Flags: review?(brendan)
Attachment #341032 - Flags: review?(gal)
Comment on attachment 341043 [details] [diff] [review]
Patch 2

r=me, thanks.

/be
Attachment #341043 - Flags: review?(brendan) → review-
Attachment #341043 - Flags: review- → review+
Comment on attachment 341032 [details] [diff] [review]
Patch

Looks good but I am afraid we need the same in js_ExecuteTree, otherwise we can execute trace code that ignores watchpoints.
Attachment #341032 - Flags: review+
I think the generic method is good for js_ExecuteTree.
Adding a watchpoint for a property set that was recorded will change the shape. See jsscope.cpp, js_ChangeScopePropertyAttrs.

/be
Patch 2 pushed as 71b214a5ecca. Does comment 11 mean that js_ExecuteTree is already taken care of by the flush-on-shape-change policy?
(In reply to comment #12)
> Patch 2 pushed as 71b214a5ecca. Does comment 11 mean that js_ExecuteTree is
> already taken care of by the flush-on-shape-change policy?

Write a test, prove me wrong :-).

/be
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-455413.js,v  <--  regress-455413.js
initial revision: 1.1

http://hg.mozilla.org/mozilla-central/rev/b04c04268a94
Flags: in-testsuite+
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.