[PATCH] D29192: [AArch64] Fix incorrect relocation encodings for ILP32

[Joel Jones via Phabricator via llvm-commits]

joelkevinjones added inline comments.


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp:266
       if (SymLoc == AArch64MCExpr::VK_GOT && IsNC)
-        return R_CLS(LD64_GOT_LO12_NC);
+        return IsILP32 ? ELF::R_AARCH64_P32_LD32_GOT_LO12_NC
+                       : ELF::R_AARCH64_LD64_GOT_LO12_NC;
----------------
peter.smith wrote:
> Is there a case where this fixup is valid in ILP32 given the size of the .got is 32-bits? I think that from your test cases you would expect it for // CHECK: ldr x24, [x23, :got_lo12:sym]  
> This should probably be an error so that we only accept ldr w24, [x23, :got_lo12:sym].
> 
>   
I think you're right, but it does match what the gcc implementation of ilp32 does:

`crb6 % cat ldr-reloc.s
    .global foo
    .type   foo, %function
    ldr x24, [x23, :got_lo12:sym]
    ldr w24, [x23, :got_lo12:sym]
crb6 % gcc -mabi=ilp32 -c ldr-reloc.s
crb6 % objdump -rd ldr-reloc.o

ldr-reloc.o:     file format elf32-littleaarch64


Disassembly of section .text:

00000000 <.text>:
   0:	f94002f8 	ldr	x24, [x23]
			0: R_AARCH64_P32_LD32_GOT_LO12_NC	sym
   4:	b94002f8 	ldr	w24, [x23]
			4: R_AARCH64_P32_LD32_GOT_LO12_NC	sym
crb6 % gcc -c ldr-reloc.s
crb6 % objdump -rd ldr-reloc.o

ldr-reloc.o:     file format elf64-littleaarch64


Disassembly of section .text:

0000000000000000 <.text>:
   0:	f94002f8 	ldr	x24, [x23]
			0: R_AARCH64_LD64_GOT_LO12_NC	sym
   4:	b94002f8 	ldr	w24, [x23]
			4: R_AARCH64_LD64_GOT_LO12_NC	sym`


================
Comment at: test/MC/AArch64/arm32-elf-relocs.s:226
 
-#   ldr x24, [x23, :gottprel_lo12:sym]
-#   ldr d22, [x21, #:gottprel_lo12:sym]
+   ldr x24, [x23, :gottprel_lo12:sym]
+   ldr d22, [x21, :gottprel_lo12:sym]
----------------
peter.smith wrote:
> The removal of the # has uncommented out these instructions, but I don't see any extra CHECKs. If they are superfluous I recommend removing them.
Concur


Repository:
  rL LLVM

https://reviews.llvm.org/D29192