Crash in methodjit generated code on 64-bit

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: jandem, Assigned: dvander)

Tracking

({regression, testcase})

unspecified
x86_64
Mac OS X
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [hardblocker][fixed-in-tracemonkey])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
This crashes with -m (64-bit OS X):
--
for(var i=0; i<20; i++) {
    var x = 5e-324;
}
--
(Reporter)

Comment 1

7 years ago
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: --- → ?
(Reporter)

Comment 2

7 years ago
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).
(Reporter)

Updated

7 years ago
Blocks: 618007
(Reporter)

Updated

7 years ago
Severity: normal → critical
Keywords: regression, testcase
(Assignee)

Comment 3

7 years ago
Created attachment 503932 [details] [diff] [review]
fix

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)
(Assignee)

Comment 4

7 years ago
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.
(Assignee)

Comment 7

7 years ago
Created attachment 503974 [details] [diff] [review]
fix v2

This one fixes NunboxAssembler too.
Attachment #503932 - Attachment is obsolete: true
Attachment #503974 - Flags: review?(cdleary)
Attachment #503974 - Flags: review?(cdleary) → review+
(Assignee)

Comment 8

7 years ago
http://hg.mozilla.org/tracemonkey/rev/b034f8e72b2f
Whiteboard: hardblocker → [hardblocker][fixed-in-tracemonkey]
http://hg.mozilla.org/mozilla-central/rev/4275fce7591b
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/b034f8e72b2f
Blocks: 588021
Group: core-security
You need to log in before you can comment on or make changes to this bug.