larcenists / larceny

Larceny Scheme implementation
Other
202 stars 32 forks source link

ARM segfault during multiply #762

Open WillClinger opened 8 years ago

WillClinger commented 8 years ago

This looks like it has been a latent bug in the ARM code generator, but I didn't find it until I tried to free up another ARM register by using sp as STKP.

The newly aggressive register assignment seemed to be working fine until a segfault occurred while compiling 27.sld. That library just imports the R6RS version, and the segfault occurred while reading compiled code for the R6RS version. The segfault is more easily reproduced by evaluating (string->number "1403580.0"). Here is the relevant machine code:

   0xb660c37c:  streq   sp, [r9, #184]  ; 0xb8   ; cant.mul/bnv as r10 r12 r10 L1
   0xb660c380:  smulleq sp, lr, r10, r12
   0xb660c384:  asreq   sp, sp, #31
   0xb660c388:  bne     0xb660c39c
   0xb660c38c:  cmp     lr, sp
   0xb660c390:  ldr     sp, [r9, #184]  ; 0xb8   ; STKP
   0xb660c394:  muleq   r10, r10, r12
   0xb660c398:  beq     0xb660c3a8               ; end of cant.mul/bnv sequence
   0xb660c39c:  mov     r11, r8
   0xb660c3a0:  add     lr, r9, #632    ; 0x278  ; call $m.multiply
   0xb660c3a4:  blx     lr
   0xb660c3a8:  add     sp, sp, #24              ; pop stack frame
   0xb660c3ac:  ldr     pc, [sp]                 ; clobber pc
   0xb660c3b0:  mov     r10, r5                  ; never executed because of segfault

The code generated by cant.mul/bnv assumed STKP is a general register that can be used as a scratch register provided it is saved and restored. Now that STKP is sp, however, we must maintain the multiple-of-4 stack alignment at all times.

But there's another problem: The conditional branch at 0xb660c388 above can skip over the instruction that restores STKP, regardless of whether STKP is sp. That conditional branch wasn't generated directly by cant.mul/bnv, so it must have been added by the optimizer. That probably means there's a bug in the ARM machine code optimizer as well.

Or maybe the optimizer was correct until STKP became sp. Either way, it's a bug now.

No, the optimizer's okay. The conditional branch won't be executed if sp has been clobbered. So it looks as though this may just be a violation of the sp invariant, with processor-dependent behavior.

WillClinger commented 7 years ago

I don't think I ever pushed a version of the ARMv7 register assignments that used sp for any unconventional purpose, and I haven't been able to reproduce this bug without changing the register assignments to use sp as STKP, so I'm downgrading the priority of this issue and changing the milestone.

I'm leaving this issue open because I don't understand why we can't use sp as STKP. It's possible that the bug is unrelated to the register assignment, and just happens to show up with the non-standard register assignment.