1055034 - Assertion failure: memcmp(reinterpret_cast<void*>(instr), cache_page->cachedData(offset), SimInstruction::kInstrSize) == 0, at jit/arm/Simulator-arm.cpp:1067

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 0aaa2d3d15cc (run with --fuzzing-safe --ion-eager --arm-sim-icache-checks):


function range(n, m) {
  var result = [];
  for (var i = n; i < m; i++)
    result.push(i);
  return result;
}
function run(arr, func) {
  var expected = arr["map"].apply(arr, [func]);
  function f(m) { return arr["mapPar"].apply(arr, [func, m]); }
    f({mode:"compile"});
    f({mode:"seq"});
 }
run(range(0, 1024), function (i) { var a = []; a.length = i; });

Comment 1

[Christian Holler (:decoder)]

This is on ARM simulator. Marked s-s because the assertion looks like trouble.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/2c1859b7cd6d
user:        Jon Coppeard
date:        Thu Aug 14 11:46:55 2014 +0100
summary:     Bug 650161 - Update pointers in TraceDataRelocations r=mjrosenb

This iteration took 569.633 seconds to run.

Comment 3

[Jan de Mooij [:jandem]]

The assertion suggests we need to flush the icache, nice catch if that's indeed the issue here.

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Jan de Mooij [:jandem] from comment #3)
That's a very good point, we do need an icache flush here.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: bug1055034-instruction-patching

I added a call to AutoFlushICache::flush() after ma_movPatchable() in the same way as it's used in other places.

I also updated the MIPS code.  I'm not sure what out review policy is for that, but I ran the tests under the simulator and they passed so I don't think I broke anything.

Comment 6

[Marty Rosenberg [:mjrosenb]]

Comment on attachment 8475103 [details] [diff] [review]
bug1055034-instruction-patching

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

Whoops, thought that was implicitly done on some code path via the MacroAssembler.  It /probably/ should be!

Comment 7

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc196bae989e

Comment 8

[Christian Holler (:decoder)]

Marking sec-critical based on IRC discussion.

Comment 9

[Andrew McCreight [:mccr8]]

Does this affect all branches with GGC?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fc196bae989e

Comment 11

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 12

[Jon Coppeard (:jonco)]

(In reply to Andrew McCreight [:mccr8] from comment #9)
No, this was introduced by one of the patches landed in bug 650161 last week.

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

This may be a cause for the topcrasher in Nightly right now; see bug 1058567.