Closed Bug 1130845 Opened 6 years ago Closed 6 years ago

Assertion failure: from->toStackSlot()->slot() % SimdMemoryAlignment == 0, at jit/LIR.cpp:557 or Crash [@ ??] with SIMD


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox37 --- unaffected
firefox38 --- verified
firefox39 --- verified
firefox-esr31 --- unaffected
b2g-master --- fixed


(Reporter: decoder, Assigned: bbouvier)



(Keywords: crash, regression, testcase, Whiteboard: [jsbugmon:update])

Crash Data


(1 file)

The following testcase crashes on mozilla-central revision be65d1fde126 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-debug, run with --ion-offthread-compile=off --ion-eager):

var int32x4 = SIMD.int32x4;
function test() {
  var a = int32x4(  ) ;
  var b = int32x4(10, 20, 30, 40);
  var c = SIMD.int32x4.and(a, b);
  assertEq(c.x, 0);
for (var j=0; j<u.length; ++j)
    v[test()] = t;


Program received signal SIGSEGV, Segmentation fault.
0xf55f2cae in ?? ()
#0  0xf55f2cae in ?? ()
#1  0xf7fc87a6 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
eax	0xf6300230	-164625872
ebx	0x301	769
ecx	0xf5858660	-175798688
edx	0x9369824	154572836
esi	0xf7fd03d3	-134413357
edi	0x936fb68	154598248
ebp	0xffffd37c	4294955900
esp	0xffffd300	4294955776
eip	0xf55f2cae	4116655278
=> 0xf55f2cae:	movdqa %xmm0,0x78(%esp)
   0xf55f2cb4:	mov    $0xf5844040,%ecx

Marking s-s because the crash in the optimized build does not look safe.
This is probably related to the addition of SIMD into Ion.
This crash is only on nightlies, and it is safe because this is a miss-alignment issue.

(In reply to Christian Holler (:decoder) from comment #0)
> Program received signal SIGSEGV, Segmentation fault.
> => 0xf55f2cae:	movdqa %xmm0,0x78(%esp)

movdqa  expects the address to be something like  0x.....0
esp is the stack pointer.
0x78 is greater than 0.

(In reply to Christian Holler (:decoder) from comment #0)
> esp	0xffffd300	4294955776

esp + 0x78  == 0xffffd378  and does not match   0x.....0

So this is a miss-alignment issue, and this cannot be use to make any exploit.

I will investigate this issue later.
Flags: needinfo?(nicolas.b.pierron)
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:
user:        Victor Carlquist
date:        Thu Feb 05 15:13:14 2015 +0100
summary:     Bug 1127929 - IonMonkey: Inline SIMD.int32x4.and calls. r=nbp

This iteration took 539.840 seconds to run.
Blocks: 1127929
Benjamin would be looking at this issue,  I guess this issue is not directly related to Victor's patch but just highlighted by it as it is adding more surface to an existing issue.
Flags: needinfo?(nicolas.b.pierron) → needinfo?(benj)
In this case, there's a SimdUnbox in the block right before the loop header, that gets spilled onto the stack because there are too many live registers, and then it gets reloaded at the end of the block.  Moves implied by regalloc assume that the stack is correctly aligned, and thus that we can do aligned moves, but that's not the case here.

Testing with nbp's folded alignment patch, it still crashes, so there might be something wrong with one of the alignment patches, or the assertion is somehow wrong? Investigating.
So the issue was that on a call to spill(), regalloc was re-using another already allocated, but available stack slot (available since the corresponding definition was dead).  The issue in this particular example is that the stack slot had a size of 32 bits and no alignment requirement, while SIMD expect 128 bits and do have alignment requirements.

Odd enough, this was never hit with backtracking, while the code for allocating stack slots looks to be shared among the two.  Any idea why?

I've just copied the same logic as for the doubles slots list, and it works fine.

About the security: this is nightly-only, but this could be the source of stack corruption, I suppose (in the case where the former 32-bits slot is aligned, we could clobber up to 3 * 32 = 96 bits on the stack).  Should I ask for security rating?
Assignee: nobody → benj
Flags: needinfo?(benj)
Attachment #8563406 - Flags: review?(sunfish)
Comment on attachment 8563406 [details] [diff] [review]

Review of attachment 8563406 [details] [diff] [review]:

Looks right to me. Backtracking already knows about this (quadSlots).
Attachment #8563406 - Flags: review?(sunfish) → review+
Blocks: 1131811
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Follow-up patch to fix test on branches which lack SIMD:
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx38
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.