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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

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]); 
    }
  );
}

Comment 1

[Christian Holler (:decoder)]

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").

Comment 2

[Nicolas B. Pierron [:nbp]]

I already saw this assertion before, when I was tweaking the fake-scheduling.  I don't think this is related to range analysis.

Comment 3

[Andrew McCreight [:mccr8]]

Marking sec-high because I think some other similar assertion was high.  Adjust as desired.

Comment 4

[Jan de Mooij [:jandem]]

needinfo?'ing a PJS guy as this seems to call mapPar :)

Comment 5

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 6

[Jan de Mooij [:jandem]]

Niko, any news here? Can you reassign if needed? Thanks.

Comment 7

[Niko Matsakis [:nmatsakis]]

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.

Comment 8

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 9

[Shu-yu Guo [:shu]]

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.

Comment 10

[Shu-yu Guo [:shu]]

Attachment: Save live ForkJoinSlice register in the fast string concat stub. (r=?)

-i

Comment 11

[Shu-yu Guo [:shu]]

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.

Comment 12

[Jan de Mooij [:jandem]]

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?

Comment 13

[Shu-yu Guo [:shu]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d64b073e736e

Comment 14

[Carsten Book [:Tomcat]]

fixed in https://hg.mozilla.org/mozilla-central/rev/d64b073e736e

Comment 15

[Daniel Veditz [:dveditz]]

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.

Comment 16

[Andrew McCreight [:mccr8]]

From the patch, it looks like this only affects PJS, which is only enabled on trunk.

Comment 17

[Shu-yu Guo [:shu]]

(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?