Closed Bug 769987 Opened 11 years ago Closed 11 years ago

Crash [@ js::mjit::JITScript::nativeToPC] or [@ js::mjit::JITScript::findCodeChunk]

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
Milestone:
mozilla16
Tracking Flags:
Tracking Status
firefox13 --- unaffected
firefox14 --- fixed
firefox15 --- fixed
firefox16 --- fixed
firefox-esr10 --- unaffected

People

(Reporter: gkw, Assigned: luke)

References

Details

(4 keywords, Whiteboard: [js:t][advisory-tracking-][qa-])

Attachments

(2 files)

stack
12.26 KB, text/plain
Details
fix and test
1.27 KB, patch
bhackett1024
: review+
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached file stack — 
function h(code) {
  f = Function(code);
  g()
}
function g() {
  f()
}
h()
h()
h("\
  arguments[\"0\"];\
  gc();\
")

crashes js opt shell on m-c changeset d9d61d199b11 with -m and -a at js::mjit::JITScript::nativeToPC and crashes js debug shell at js::mjit::JITScript::findCodeChunk

s-s because this concerns gc.
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   97976:d4ac6ac2e618
user:        Luke Wagner
date:        Thu Jun 28 22:50:15 2012 -0700
summary:     Bug 767667 - fix getelem on optimized arguments (r=bhackett)
Blocks: 767667
Attached patch fix and test — — Splinter Review 
The bug is actually pre-existing, just much easier to trigger with bug 767667.

The bug is that HAS_PREVPC is not getting set when frames are being bailed from the VM.  It seems ExpandInlineFrames is necessary and not implied by Recompiler::clearStackReferences.  Brian: perhaps clearStackReferences should ExpandInlineFrames?  (I some call sites were dominated by ExpandInlineFrames, but not all.)
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #638236 - Flags: review?(bhackett1024)
Whiteboard: js-triage-needed → [js:t]
Comment on attachment 638236 [details] [diff] [review]
fix and test

Yeah, the recompiler stuff could use some streamlining.
Attachment #638236 - Flags: review?(bhackett1024) → review+
Comment on attachment 638236 [details] [diff] [review]
fix and test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 733950
User impact if declined: possible crash
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): very low
Attachment #638236 - Flags: approval-mozilla-beta?
Attachment #638236 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/07b1a5999430
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment on attachment 638236 [details] [diff] [review]
fix and test

[Triage Comment]
Very low risk fix for a regression in FF14. Approving for branches.
Attachment #638236 - Flags: approval-mozilla-beta?
Attachment #638236 - Flags: approval-mozilla-beta+
Attachment #638236 - Flags: approval-mozilla-aurora?
Attachment #638236 - Flags: approval-mozilla-aurora+
Whiteboard: [js:t] → [js:t][advisory-tracking+]
Keywords: csec-dos
Whiteboard: [js:t][advisory-tracking+] → [js:t][advisory-tracking-]
Group: core-security
Whiteboard: [js:t][advisory-tracking-] → [js:t][advisory-tracking-][qa-]
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/testBug769987.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.