Closed Bug 1055034 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript Engine: JIT, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla34
Tracking Status
firefox33 --- unaffected
firefox34 --- verified
firefox-esr24 --- unaffected
firefox-esr31 --- unaffected
b2g-v2.1 --- fixed

People

(Reporter: decoder, Assigned: jonco)

References

Details

(Keywords: assertion, sec-critical, testcase, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

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; });
This is on ARM simulator. Marked s-s because the assertion looks like trouble.
Whiteboard: [jsbugmon:update,bisect]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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.
The assertion suggests we need to flush the icache, nice catch if that's indeed the issue here.
Flags: needinfo?(jcoppeard)
(In reply to Jan de Mooij [:jandem] from comment #3)
That's a very good point, we do need an icache flush here.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
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.
Attachment #8475103 - Flags: review?(mrosenberg)
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!
Attachment #8475103 - Flags: review?(mrosenberg) → review+
No longer blocks: harmony:symbols
Marking sec-critical based on IRC discussion.
Keywords: sec-critical
Does this affect all branches with GGC?
Flags: needinfo?(jcoppeard)
https://hg.mozilla.org/mozilla-central/rev/fc196bae989e
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
(In reply to Andrew McCreight [:mccr8] from comment #9)
No, this was introduced by one of the patches landed in bug 650161 last week.
Depends on: 650161
Flags: needinfo?(jcoppeard)
Depends on: 1057803
This may be a cause for the topcrasher in Nightly right now; see bug 1058567.
Group: core-security
You need to log in before you can comment on or make changes to this bug.