Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

`r7` incorrectly chosen for scratch register by `__builtin_longjmp` on ARM Linux #49171

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR50202
Status NEW
Importance P enhancement
Reported by Tee KOBAYASHI (xtkoba@gmail.com)
Reported on 2021-05-02 16:31:11 -0700
Last modified on 2021-09-01 21:30:36 -0700
Version trunk
Hardware PC Linux
CC htmldeveloper@gmail.com, llvm-bugs@lists.llvm.org, smithp352@googlemail.com, Ties.Stuij@arm.com
Fixed by commit(s)
Attachments ruby-vm-preprocessed.i.gz (198192 bytes, application/gzip)
bug50202-ARM-Int_eh_sjlj_longjmp-scratch-register.patch (1144 bytes, text/plain)
DRAFT0000-bug50202-Int_eh_sjlj_longjmp-GPRnofp.patch (1635 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 24822 Preprocessd C source for reproduction

To reproduce, gunzip the attached file and compile it with the following options:

$ clang --target=armv7a-softfloat-linux-gnueabi -fdeclspec -O3 -fPIC -fstack-protector-strong ruby-vm-preprocessed.i -c

Then

$ llvm-objdump --print-imm-hex -d --disassemble-symbols=rb_f_catch ruby-vm-preprocessed.o

gives the output which is attached at the bottom of this post. Note that at 0x13288 r7 is chosen for the scratch register, which is then overwritten at 0x1328c with the frame pointer saved in the jump buffer. Consequently, at 0x13294 the bx instruction tries to branch to an address pointed to by the frame pointer, which is clearly incorrect, typically leading to an Illegal Instruction error.

000131cc : 131cc: f0 48 2d e9 push {r4, r5, r6, r7, r11, lr} 131d0: 10 d0 4d e2 sub sp, sp, #16 131d4: c0 50 9f e5 ldr r5, [pc, #0xc0] 131d8: 01 00 70 e3 cmn r0, #1 131dc: 05 50 9f e7 ldr r5, [pc, r5] 131e0: 00 20 95 e5 ldr r2, [r5] 131e4: 0c 20 8d e5 str r2, [sp, #0xc] 131e8: 1f 00 00 da ble #0x7c <rb_f_catch+0xa0> 131ec: 02 00 50 e3 cmp r0, #2 131f0: 1d 00 00 aa bge #0x74 <rb_f_catch+0xa0> 131f4: 00 00 50 e3 cmp r0, #0 131f8: 01 00 00 0a beq #0x4 <rb_f_catch+0x38> 131fc: 00 40 91 e5 ldr r4, [r1] 13200: 04 00 00 ea b #0x10 <rb_f_catch+0x4c> 13204: 94 00 9f e5 ldr r0, [pc, #0x94] 13208: 00 00 9f e7 ldr r0, [pc, r0] 1320c: 00 00 90 e5 ldr r0, [r0] 13210: fe ff ff eb bl #-0x8 <rb_f_catch+0x44> 13214: 00 40 a0 e1 mov r4, r0 13218: 78 00 9f e5 ldr r0, [pc, #0x78] 1321c: 00 00 8f e0 add r0, pc, r0 13220: fe ff ff eb bl #-0x8 <rb_f_catch+0x54> 13224: 78 10 9f e5 ldr r1, [pc, #0x78] 13228: 08 30 8d e2 add r3, sp, #8 1322c: 00 60 90 e5 ldr r6, [r0] 13230: 04 00 a0 e1 mov r0, r4 13234: 01 10 8f e0 add r1, pc, r1 13238: 00 20 a0 e3 mov r2, #0 1323c: 00 60 8d e5 str r6, [sp] 13240: 00 70 a0 e3 mov r7, #0 13244: fe ff ff eb bl #-0x8 <rb_f_catch+0x78> 13248: 08 10 9d e5 ldr r1, [sp, #0x8] 1324c: 00 00 51 e3 cmp r1, #0 13250: 08 00 00 1a bne #0x20 <rb_f_catch+0xac> 13254: 00 10 95 e5 ldr r1, [r5] 13258: 0c 20 9d e5 ldr r2, [sp, #0xc] 1325c: 02 00 51 e1 cmp r1, r2 13260: 10 d0 8d 02 addeq sp, sp, #16 13264: f0 88 bd 08 popeq {r4, r5, r6, r7, r11, pc} 13268: fe ff ff eb bl #-0x8 <rb_f_catch+0x9c> 1326c: 00 10 a0 e3 mov r1, #0 13270: 01 20 a0 e3 mov r2, #1 13274: fe ff ff eb bl #-0x8 <rb_f_catch+0xa8> 13278: 0c 00 96 e5 ldr r0, [r6, #0xc] 1327c: 20 10 80 e5 str r1, [r0, #0x20] 13280: 08 00 80 e2 add r0, r0, #8 13284: 08 d0 90 e5 ldr sp, [r0, #0x8] 13288: 04 70 90 e5 ldr r7, [r0, #0x4] 1328c: 00 70 90 e5 ldr r7, [r0] 13290: 00 b0 90 e5 ldr r11, [r0] 13294: 17 ff 2f e1 bx r7

Quuxplusone commented 3 years ago

Attached ruby-vm-preprocessed.i.gz (198192 bytes, application/gzip): Preprocessd C source for reproduction

Quuxplusone commented 3 years ago

I don't know who outside the ARM Backend elects R7 as the scratch register, but I believe she is not guilty because R7 is not reserved when R11 is used as the frame pointer register.

This should be handled in the ARM Backend in any way.

Quuxplusone commented 3 years ago

Probably we need a new register class that excludes potential FP registers. Now I have a patch for that which will be posted soon.

Quuxplusone commented 3 years ago

Attached bug50202-ARM-Int_eh_sjlj_longjmp-scratch-register.patch (1144 bytes, text/plain): Avoid using R7 or R11 as the scratch register

Quuxplusone commented 3 years ago

Attached DRAFT0000-bug50202-Int_eh_sjlj_longjmp-GPRnofp.patch (1635 bytes, text/plain): [DRAFT] Use new register class GPRnofp

Quuxplusone commented 3 years ago

Proposed change: https://reviews.llvm.org/D109129