[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:
> 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.
I discussed this with Andrew Pinski (who did ILP32 for gcc and the linux kernel) and he came to the same conclusion. I've already filed a defect on gas: https://sourceware.org/bugzilla/show_bug.cgi?id=21098


Repository:
  rL LLVM

https://reviews.llvm.org/D29192