Closed Bug 898857 Opened 11 years ago Closed 11 years ago

Assertion failure: ins->mirRaw()->isAsmJSUDiv() || ins->mirRaw()->isAsmJSUMod(), at ion/arm/CodeGenerator-arm.cpp:1910

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: decoder, Assigned: jonco)

References

Details

(Keywords: assertion, testcase, Whiteboard: [jsbugmon:ignore])

Attachments

(1 file)

The following testcase asserts on mozilla-central revision 73b69c146ca6 (run with --ion-eager):


RegExp({ toString: fillHeap });
function fillHeap() {
  var x = 1, tmp;
  for (var i = 0; i < 50000; ++i)
    tmp <<=  x / 3;
}
This doesn't crash but the assertion seems to be about some low-level instructions and I'd like to make sure this is not somehow security-related before opening.

Marty, can you take a look?
Flags: needinfo?(mrosenberg)
Whiteboard: [jsbugmon:ignore]
Odd, I just ran this with the latest m-c revision, and got this:
mjrosenb@eve:~/src/central/central/js/objs/arm-dbg$ ./shell/js --ion-eager ~/tests/898857.js 
Assertion failure: funVal.isObject(), at /home/mjrosenb/src/central/central/js/src/vm/SelfHosting.cpp:964
Segmentation fault (core dumped)

I'll try some other stuff.
Flags: needinfo?(mrosenberg)
It looks like the code that is asserting here is making sure that the unsigned operation came from an unsigned MIR node, but it looks like it is possible if both operands are positive, and are used in an unsigned context, we'll generate an unsigned divide instead.  My best guess is the assertion is totally bogus, and worst case scenario is we're generating an unsigned divide when we should be generating a signed divide.  A wrong answer is the worst case scenario.
Group: core-security
needinfo'ing jonco because he wrote this code.
Flags: needinfo?(jcoppeard)
Well I didn't write the assertion, that was there already :)

The changes in bug 895883 should only change the generated code, not the circumstances in which unsigned divide LIR nodes are generated. 

It sounds like the assertion is bogus since we definitely generate unsigned divide LIR nodes for MDivs.
Flags: needinfo?(jcoppeard)
So if I run the testcase on a chromebook I get a different assertion triggered:

Assertion failure: a.isGeneralReg(), at ../ion/shared/CodeGenerator-shared-inl.h:33
Program received signal SIGSEGV, Segmentation fault.
0x00508836 in js::ion::ToRegister (a=...) at ../ion/shared/CodeGenerator-shared-inl.h:33
33	    JS_ASSERT(a.isGeneralReg());
(gdb) bt
#0  0x00508836 in js::ion::ToRegister (a=...)
    at ../ion/shared/CodeGenerator-shared-inl.h:33
#1  0x0050886e in js::ion::ToRegister (a=0x7deb08)
    at ../ion/shared/CodeGenerator-shared-inl.h:40
#2  0x0050eb42 in js::ion::CodeGeneratorARM::visitUDiv (this=0x7cf240, ins=0x7deae0)
    at /home/jon/work/arm/js/src/ion/arm/CodeGenerator-arm.cpp:1874

This one is totally my fault however (I didn't appreciate that lowerForALU might result in one operand being a constant rather than a register).

I'll fix both assertions in this bug.
Assignee: general → jcoppeard
Status: NEW → ASSIGNED
Attached patch fix-udiv-assertSplinter Review
Attachment #784359 - Flags: review?(mrosenberg)
Attachment #784359 - Flags: review?(mrosenberg) → review+
https://hg.mozilla.org/mozilla-central/rev/33a9f192d945
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: