Assertion failure: js::CurrentThreadCanAccessRuntime(runtime_), at ../vm/Runtime.h:1779 and Crash on Heap with Range Analysis

RESOLVED FIXED in Firefox 27

Status

()

Core
JavaScript Engine: JIT
--
critical
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: decoder, Assigned: shu)

Tracking

(Blocks: 2 bugs, 4 keywords)

Trunk
mozilla27
x86_64
Linux
assertion, crash, sec-high, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox25 disabled, firefox26 disabled, firefox27 fixed, firefox-esr17 unaffected, firefox-esr24 disabled, b2g18 unaffected)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
The following testcase asserts on mozilla-central revision e42dce3209da (threadsafe build, run with --fuzzing-safe --ion-check-range-analysis --ion-regalloc=backtracking):


assertArraySeqParResultsEq(range(0, 512), "map", function(e) { return e+'x'; });
function range(n, m) {
  var result = [];
  for (var i = n; i < m; i++)
    result.push(i);
  return result;
}
function assertParallelExecSucceeds(opFunction) {
  for (var i = 0; i < 100; ++i) {
    opFunction({mode:"compile"});
  }
}
function assertArraySeqParResultsEq(arr, op, func) {
  assertParallelExecSucceeds(
    function (m) { 
      return arr[op + "Par"].apply(arr, [func, m]); 
    }
  );
}
(Reporter)

Comment 1

4 years ago
Not sure if this is really related to range analysis or just coincidence that the option is needed. What is very strange is that the assertion pops up multiple times before the program aborts then (to be precise, it appears 605 times before I see "Segmentation fault").
Keywords: crash
I already saw this assertion before, when I was tweaking the fake-scheduling.  I don't think this is related to range analysis.
Marking sec-high because I think some other similar assertion was high.  Adjust as desired.
Keywords: sec-high
needinfo?'ing a PJS guy as this seems to call mapPar :)
Flags: needinfo?(nmatsakis)
Blocks: 349611
(Reporter)

Comment 5

4 years ago
Created attachment 801507 [details]
[crash-signature] Machine-readable crash signature
Niko, any news here? Can you reassign if needed? Thanks.
No real news. Thus far, I have not been able to reproduce. I did see a fix for a similar assertion go by at some point, so perhaps this has been fixed, I will try running with an older revision.
Flags: needinfo?(nmatsakis)
(Reporter)

Comment 8

4 years ago
Created attachment 813739 [details]
[crash-signature] Machine-readable crash signature
Attachment #801507 - Attachment is obsolete: true
(Assignee)

Comment 9

4 years ago
I can repro, but no idea what's going on yet. I think the js::CurrentThreadCanAccessRuntime(runtime_) assert is bogus, I see a crash in jitcode.
(Assignee)

Comment 10

4 years ago
Created attachment 815235 [details] [diff] [review]
Save live ForkJoinSlice register in the fast string concat stub. (r=?)

-i
Attachment #815235 - Flags: review?(jdemooij)
(Assignee)

Updated

4 years ago
Assignee: general → shu
Status: NEW → ASSIGNED
(Assignee)

Comment 11

4 years ago
Oops, that's bzexport screwing up above. So after 4 hours of EXCRUCIATING PAIN I finally figured out it's because I was too clever with the parallel string concat stub with reusing a temp (due to x86 having so few registers), and was clobbering a live pointer to a ForkJoinSlice.

Updated

4 years ago
Component: JavaScript Engine → JavaScript Engine: JIT
Comment on attachment 815235 [details] [diff] [review]
Save live ForkJoinSlice register in the fast string concat stub. (r=?)

Review of attachment 815235 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch. Can you add the testcase as well?
Attachment #815235 - Flags: review?(jdemooij) → review+
fixed in https://hg.mozilla.org/mozilla-central/rev/d64b073e736e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Shu: Before landing patches for security bugs please follow the process at https://wiki.mozilla.org/Security/Bug_Approval_Process

As a sec-high bug this needed sec-approval before landing. Possibly you know something about whether it does or doesn't affect older branches that is not evident in this bug, and if so it would help to add that information /to/ the bug so you don't keep tripping our "Oh Shit" alarm about whether we just potentially 0-day'd all our release users.

Carsten: Please set the matching "status-firefoxXX" tracking flag to "fixed" in addition to setting the target milestone when you land security bugs. Thanks.
status-firefox27: --- → fixed
status-firefox-esr17: --- → unaffected
status-firefox-esr24: --- → ?
Flags: needinfo?(shu)
From the patch, it looks like this only affects PJS, which is only enabled on trunk.
status-b2g18: --- → unaffected
status-firefox25: --- → disabled
status-firefox26: --- → disabled
status-firefox-esr24: ? → disabled
(Assignee)

Comment 17

4 years ago
(In reply to Daniel Veditz [:dveditz] from comment #15)
> Shu: Before landing patches for security bugs please follow the process at
> https://wiki.mozilla.org/Security/Bug_Approval_Process

I've been told I only needed to do that for bugs that affect non-Nightly releases. Or was the problem that I didn't mark the other branches as unaffected?
Flags: needinfo?(shu)

Updated

4 years ago
Keywords: verifyme

Updated

4 years ago
Keywords: verifyme
Whiteboard: [qa-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.