The default bug view has changed. See this FAQ.

Crash [@ js_DisassembleAtPC] with "use asm"

RESOLVED FIXED in mozilla24

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: gkw, Assigned: bbouvier)

Tracking

(Blocks: 2 bugs, {crash, regression, testcase})

Trunk
mozilla24
x86_64
Mac OS X
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [jsbugmon:update], crash signature)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 757035 [details]
stack

disassemble("-r", (function() {
    (function() {
        "use asm"
        return {}
    })()
}))

crashes js debug shell on m-c changeset 18fc62fd8dcc without any CLI arguments at js_DisassembleAtPC

Comment 1

4 years ago
Regression from bug 851421?
Blocks: 840284
(Reporter)

Comment 2

4 years ago
Yep.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/30b977b2b911
user:        Nicholas Nethercote
date:        Thu Mar 14 18:44:03 2013 -0700
summary:     Bug 851421 (part 2) - Don't emit bytecode for asm.js functions unless linking fails.  r=luke.
Blocks: 851421
(Assignee)

Comment 3

4 years ago
Created attachment 759390 [details] [diff] [review]
proposed fix + test case

The script is set to NULL if the function is native; in this case, there is no need to recursively apply the function.
Assignee: general → bbouvier
Status: NEW → ASSIGNED
Attachment #759390 - Flags: review?(luke)

Comment 4

4 years ago
Comment on attachment 759390 [details] [diff] [review]
proposed fix + test case

Thanks!  Do you suppose you could change this so that, in the !script case, we Sprint something like "[native code]"?
Attachment #759390 - Flags: review?(luke) → review+
(Assignee)

Comment 5

4 years ago
The current behaviour of dis() and disassemble() is to throw an error if there is no script: "typein:2:0 Error: only works on scripts".
So this is doable, but this would change dis / disassemble (and similar functions) behaviours.
(Assignee)

Comment 6

4 years ago
Created attachment 759525 [details] [diff] [review]
print "native code" instead of nothing, as discussed
Attachment #759390 - Attachment is obsolete: true
Attachment #759525 - Flags: review?(luke)

Comment 7

4 years ago
Comment on attachment 759525 [details] [diff] [review]
print "native code" instead of nothing, as discussed

Thanks!
Attachment #759525 - Flags: review?(luke) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6db31e46b02
Flags: in-testsuite+
Keywords: checkin-needed
Backed out for test failures (see the comments in bug 878435).
https://hg.mozilla.org/integration/mozilla-inbound/rev/a15f36c774a8

Comment 10

4 years ago
Ugh, I should have caught this; some shell functions are only available in debug builds (like dis and disassemble).  Try "if (disassemble)" (but double check me that this fixes the problem ;)
(Assignee)

Comment 11

4 years ago
Created attachment 759831 [details] [diff] [review]
patch + test case that runs even in opt builds

if (disassemble) was not enough as "disassemble" was not defined. This test case passes in debug and opt builds and does what we expect. One can check by printing the result of disassemble: in debug mode, it will print the disassembled bytecode; in opt mode it won't print anything.
Attachment #759525 - Attachment is obsolete: true
Attachment #759831 - Flags: review?(luke)

Comment 12

4 years ago
Comment on attachment 759831 [details] [diff] [review]
patch + test case that runs even in opt builds

Thanks!
Attachment #759831 - Flags: review?(luke) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5860d85b0006
https://hg.mozilla.org/mozilla-central/rev/5860d85b0006
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(Reporter)

Updated

a year ago
Depends on: 1237403
You need to log in before you can comment on or make changes to this bug.