Closed
Bug 570895
Opened 14 years ago
Closed 14 years ago
Consider replacing abort() in AvmAssertFail
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: gkw, Assigned: gkw)
Details
(Whiteboard: fixed-in-nanojit, fixed-in-tamarin)
Attachments
(3 files, 1 obsolete file)
1.06 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
1.20 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
1021 bytes,
patch
|
Details | Diff | Splinter Review |
The abort() call in AvmAssertFail could be replaced by the components in JS_Assert. Currently abort() calls cause testcase reduction issues (in Lithium) in Windows when a LIR assertion is hit. http://mxr.mozilla.org/mozilla-central/source/js/src/nanojit/avmplus.cpp#65 http://mxr.mozilla.org/mozilla-central/source/js/src/jsutil.cpp#67
Comment 1•14 years ago
|
||
We own that file, so we can patch it at will. Go for it :)
Assignee: general → gary
Assignee | ||
Comment 2•14 years ago
|
||
Attachment #450039 -
Flags: review?(gal)
Assignee | ||
Comment 3•14 years ago
|
||
Attachment #450039 -
Attachment is obsolete: true
Attachment #450040 -
Flags: review?(gal)
Attachment #450039 -
Flags: review?(gal)
Assignee | ||
Comment 4•14 years ago
|
||
Does this side patch make sense? I'd thought to synchronize the "Assertion failed" vs "Assertion failure" message, but this doesn't seem important except from a cosmetic point-of-view, and I'm not sure if there was any reason for calling it the former rather than the latter.
Attachment #450042 -
Flags: feedback?(gal)
Comment 5•14 years ago
|
||
Comment on attachment 450042 [details] [diff] [review] side patch nanojit.h is shared with adobe. The patch is fine but there is a certain process to apply it (commit to nanojit-central). njn knows the drill.
Attachment #450042 -
Flags: feedback?(gal) → review+
Comment 6•14 years ago
|
||
Comment on attachment 450040 [details] [diff] [review] ignore whitespace nits Looks good.
Attachment #450040 -
Flags: review?(gal) → review+
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6) > (From update of attachment 450040 [details] [diff] [review]) > Looks good. http://hg.mozilla.org/tracemonkey/rev/2ae7cb9510b3
Whiteboard: fixed-in-tracemonkey
Comment 8•14 years ago
|
||
I pushed these for Gary: http://hg.mozilla.org/projects/nanojit-central/rev/c7e84fb8ce71 http://hg.mozilla.org/projects/nanojit-central/rev/49982fe529dd
Whiteboard: fixed-in-tracemonkey → fixed-in-nanojit
Comment 9•14 years ago
|
||
The previous patch used "raise" and "SIGABRT", but didn't include signal.h. This broke the nanojit-central build both on ARM/Ubuntu and x64/Ubuntu, so I've pushed a trivial patch to fix it. http://hg.mozilla.org/projects/nanojit-central/rev/0a8efeddb4d9
Assignee | ||
Comment 10•14 years ago
|
||
Thanks Jacob!
Comment 11•14 years ago
|
||
TR: http://hg.mozilla.org/tamarin-redux/rev/47d0a7afb559
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tamarin
Comment 12•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/2ae7cb9510b3
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•