Closed Bug 570895 Opened 14 years ago Closed 14 years ago

Consider replacing abort() in AvmAssertFail

Categories

(Core :: JavaScript Engine, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: gkw, Assigned: gkw)

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tamarin)

Attachments

(3 files, 1 obsolete file)

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
We own that file, so we can patch it at will. Go for it :)
Assignee: general → gary
Attached patch take one (obsolete) — Splinter Review
Attachment #450039 - Flags: review?(gal)
Attachment #450039 - Attachment is obsolete: true
Attachment #450040 - Flags: review?(gal)
Attachment #450039 - Flags: review?(gal)
Attached patch side patchSplinter Review
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 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 on attachment 450040 [details] [diff] [review]
ignore whitespace nits

Looks good.
Attachment #450040 - Flags: review?(gal) → review+
(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
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
TR: http://hg.mozilla.org/tamarin-redux/rev/47d0a7afb559
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tamarin
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.

Attachment

General

Created:
Updated:
Size: