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

VERIFIED FIXED in Firefox 34

Status

()

defect
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: decoder, Assigned: jonco)

Tracking

(Blocks 1 bug, {assertion, sec-critical, testcase})

Trunk
mozilla34
ARM
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox33 unaffected, firefox34 verified, firefox-esr24 unaffected, firefox-esr31 unaffected, b2g-v2.1 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment)

Reporter

Description

5 years ago
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; });
Reporter

Comment 1

5 years ago
This is on ARM simulator. Marked s-s because the assertion looks like trouble.
Whiteboard: [jsbugmon:update,bisect]
Reporter

Updated

5 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Reporter

Comment 2

5 years ago
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)
Assignee

Comment 4

5 years ago
(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)
Assignee

Comment 5

5 years ago
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
Reporter

Comment 8

5 years ago
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: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Reporter

Updated

5 years ago
Status: RESOLVED → VERIFIED
Reporter

Comment 11

5 years ago
JSBugMon: This bug has been automatically verified fixed.
Assignee

Comment 12

5 years ago
(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.