Closed Bug 625757 Opened 13 years ago Closed 13 years ago

Crash in methodjit generated code on 64-bit

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: jandem, Assigned: dvander)

References

Details

(Keywords: regression, testcase, Whiteboard: [hardblocker][fixed-in-tracemonkey])

Attachments

(1 file, 1 obsolete file)

fix v2
13.66 KB, patch
cdleary
: review+
Details | Diff | Splinter Review 
This crashes with -m (64-bit OS X):
--
for(var i=0; i<20; i++) {
    var x = 5e-324;
}
--
I can also reproduce this with revision http://hg.mozilla.org/tracemonkey/rev/b824cec25ece, before the PIC-patches landed.

Unfortunately cdleary was unable to reproduce this on Linux/X64, but I can reproduce on OS X in release and debug builds. Will bisect this now.
No longer blocks: 588021
blocking2.0: --- → ?
The first bad revision is:
changeset:   60013:f497fca35415
user:        David Anderson <danderson@mozilla.com>
date:        Sat Jan 08 16:27:48 2011 -0800
summary:     Remove unsound global optimizations (bug 618007, r=brendan).
Blocks: 618007
Severity: normal → critical
Keywords: regression, testcase
Attached patch fix (obsolete) — — Splinter Review 
Whoops, PunboxAssembler wasn't using the macro assembler interface to give back patchable addresses. The problem is some instructions look like:

PREFIX MOV_BYTE MODRM ADDRESS_OFFSET IMM32
                                            ^
Here, patching from the end of the instruction won't get you the address.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #503932 - Flags: review?(cdleary)
Blame is a red herring, bug 618007 didn't touch this code but it did increase the occurrence of this IC.
No longer blocks: 618007
blocking2.0: ? → betaN+
Whiteboard: hardblocker
Comment on attachment 503932 [details] [diff] [review]
fix

Makes perfect sense. Should have realized this could happen.
Attachment #503932 - Flags: review?(cdleary) → review+
Comment on attachment 503932 [details] [diff] [review]
fix

Oh, and we should add the test case in there as well for the push.
Attached patch fix v2 — — Splinter Review 
This one fixes NunboxAssembler too.
Attachment #503932 - Attachment is obsolete: true
Attachment #503974 - Flags: review?(cdleary)
Attachment #503974 - Flags: review?(cdleary) → review+
http://hg.mozilla.org/tracemonkey/rev/b034f8e72b2f
Whiteboard: hardblocker → [hardblocker][fixed-in-tracemonkey]
http://hg.mozilla.org/mozilla-central/rev/4275fce7591b
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.