Closed Bug 507295 Opened 15 years ago Closed 15 years ago

TM: Crash [@ nanojit::Assembler::findRegFor] or "Assertion failure: s0->isQuad(), at ../jstracer.cpp"

Categories

(Core :: JavaScript Engine, defect, P1)

x86
macOS
defect

Tracking

()

VERIFIED FIXED
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: gkw, Assigned: dmandelin)

References

Details

(5 keywords, Whiteboard: fixed-in-tracemonkey)

Crash Data

Attachments

(1 file)

(function () { var y; (eval("(function () {\ for (var x = 0; x < 3; ++x) {\ ''.replace(/a/, (y = 3))\ }\ });\ "))() })() crashes js opt shell with -j on TM branch at nanojit::Assembler::findRegFor at 0x000000000004aaf7 and asserts dbg shell with -j on TM branch at Assertion failure: s0->isQuad(), at ../jstracer.cpp:1244
Flags: blocking1.9.2?
Can you bisect, please?
Er, although it's almost certainly the setname patch, I guess, since this wouldn't have traced before then.
(In reply to comment #1) > Can you bisect, please? Yes, autoBisect is happily munching in the background.
Keywords: regression
autoBisect shows this is probably related to bug 495329 : The first bad revision is: changeset: 30697:60a9ef4e1a3d user: David Mandelin date: Mon Jul 27 18:13:53 2009 -0700 summary: Bug 495329: Trace JSOP_BINDNAME/JSOP_SETNAME for closures, r=brendan
Blocks: 495329
Assignee: general → dmandelin
Attached patch Patch (deleted) — Splinter Review
The setname code needs to box the value and then pass it to the native set function. That part is correct, but then I was returning that boxed value, when I should have been returning the unboxed value. Usually, values passed to set are never used again, so the bug doesn't show up unless you use the result of an assignment.
Attachment #391509 - Flags: review?(gal)
By the way, I had a crash in mochitest for my in-progress JSOP_INCNAME patch, and this bug turned out to be closely related, so it put my attention on the problem and made it easy to find. ++Gary and fuzz testing.
Comment on attachment 391509 [details] [diff] [review] Patch The code is really not intuitive (I mean box_jsval in general, not your use of it). I wonder whether there is a better way to structure the helper functions (and also maybe name them differently).
Attachment #391509 - Flags: review?(gal) → review+
Pushed to TM as 2bc1b0dbe35f.
(In reply to comment #7) > (From update of attachment 391509 [details] [diff] [review]) > The code is really not intuitive (I mean box_jsval in general, not your use of > it). I wonder whether there is a better way to structure the helper functions > (and also maybe name them differently). I noticed that as well. Actually, I would say my usage is not intuitive either, but this seemed to be about the best I could do with this API (my first try is much more confusing). How about making box_jsval return the value, which fits in well with the functionaloid design of LIR?
I gave box_jsval a void return value because unbox_jsval has a bool return value for the error value if I remember correctly and I tried to match the two.
(In reply to comment #10) > I gave box_jsval a void return value because unbox_jsval has a bool return > value for the error value if I remember correctly and I tried to match the two. I figured it was something like that. It looks like now they both just have the one output value, which is going out via parameter, so I think we can change them to return values. I'll file a separate bug for further discussion and work.
(In reply to comment #8) > Pushed to TM as 2bc1b0dbe35f. Adding "fixed-in-tracemonkey" to whiteboard.
Whiteboard: fixed-in-tracemonkey
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P1
this was fixed before 1.9.2 branched
js/tests/js1_8_1/regress/regress-507295.js
Flags: in-testsuite? → in-testsuite+
v 1.9.3, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2
Crash Signature: [@ nanojit::Assembler::findRegFor]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: