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

[Peter Smith via Phabricator via llvm-commits]

peter.smith 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;
----------------
joelkevinjones wrote:
> 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`
I spoke to a member of our GNU team and they thought that GAS wasn't doing it right either. My weak preference would be for MC to fault it and raise a bug on GAS. If it is rejected then put the compatibility in then. Happy to hear other opinions on this though.


Repository:
  rL LLVM

https://reviews.llvm.org/D29192