[AArch64] Fix incorrect relocation encodings for ILP32

[joelkevinjones]

Summary

Only effects code where the ABI is "ilp32"

Patch by Joel Jones

[peter.smith]

I've checked that the relocation name is correct. I'm a bit suspicious of allowing the fixup_aarch64_ldst_imm12_scale8 in LP32, with a 32-bit sized GOT I think that this is likely to be a programming mistake that we should probably error on. I'm not hugely familiar with lp32 programming so there may be a case for it that I'm missing?

lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp

253

I think it is more a bad fixup rather than Bad LP64 relocation. The other error messages are of the form "invalid fixup for ..."
Possible suggestion "invalid fixup for got relative load/store instruction on LP64".

I don't have a particularly strong opinion here though.

266

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].

test/MC/AArch64/arm32-elf-relocs.s

226

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.

[joelkevinjones]

lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp

266

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`

test/MC/AArch64/arm32-elf-relocs.s

226

Concur

[peter.smith]

lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp

266

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.

[joelkevinjones]

lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp

266

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

[peter.smith]

Thanks for the update, I don't have any more comments.

[joelkevinjones]

I haven't abandoned this patch. In working on this one, I've found several issues that I am going to deal with first.

[joelkevinjones]

This revision is included in an omnibus ILP32 relocation change: https://reviews.llvm.org/D32072

include/llvm/Support/ELFRelocs/AArch64.def

 ELF_RELOC(R_AARCH64_P32_LDST32_ABS_LO12_NC,          0x00f)
 ELF_RELOC(R_AARCH64_P32_LDST64_ABS_LO12_NC,          0x010)
 ELF_RELOC(R_AARCH64_P32_MOVW_PREL_G0,                0x016)
 ELF_RELOC(R_AARCH64_P32_MOVW_PREL_G0_NC,             0x017)
 ELF_RELOC(R_AARCH64_P32_MOVW_PREL_G1,                0x018)
 ELF_RELOC(R_AARCH64_P32_LDST128_ABS_LO12_NC,         0x011)
 ELF_RELOC(R_AARCH64_P32_GOT_LD_PREL19,               0x019)
 ELF_RELOC(R_AARCH64_P32_ADR_GOT_PAGE,                0x01a)
-ELF_RELOC(R_AARCH64_P32_LD64_GOT_LO12_NC,            0x01b)
+ELF_RELOC(R_AARCH64_P32_LD32_GOT_LO12_NC,            0x01b)
 ELF_RELOC(R_AARCH64_P32_LD32_GOTPAGE_LO14,           0x01c)
 ELF_RELOC(R_AARCH64_P32_TLSLD_MOVW_DTPREL_G1,        0x057)
 ELF_RELOC(R_AARCH64_P32_TLSLD_MOVW_DTPREL_G0,        0x058)
 ELF_RELOC(R_AARCH64_P32_TLSLD_MOVW_DTPREL_G0_NC,     0x059)
 ELF_RELOC(R_AARCH64_P32_TLSLD_ADD_DTPREL_HI12,       0x05a)
 ELF_RELOC(R_AARCH64_P32_TLSLD_ADD_DTPREL_LO12,       0x05b)
 ELF_RELOC(R_AARCH64_P32_TLSLD_ADD_DTPREL_LO12_NC,    0x05c)
 ELF_RELOC(R_AARCH64_P32_TLSLD_LDST8_DTPREL_LO12,     0x05d)

lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp

case AArch64::fixup_aarch64_ldst_imm12_scale4:
       if (SymLoc == AArch64MCExpr::VK_DTPREL && !IsNC)
         return R_CLS(TLSLD_LDST32_DTPREL_LO12);
       if (SymLoc == AArch64MCExpr::VK_DTPREL && IsNC)
         return R_CLS(TLSLD_LDST32_DTPREL_LO12_NC);
       if (SymLoc == AArch64MCExpr::VK_TPREL && !IsNC)
         return R_CLS(TLSLE_LDST32_TPREL_LO12);
       if (SymLoc == AArch64MCExpr::VK_TPREL && IsNC)
         return R_CLS(TLSLE_LDST32_TPREL_LO12_NC);
+      if (SymLoc == AArch64MCExpr::VK_GOT && IsNC) {
+        if (IsILP32) {
+          return ELF::R_AARCH64_P32_LD32_GOT_LO12_NC;
+        } else {
+          Ctx.reportError(Fixup.getLoc(),
+                          "Bad LP64 reloc: fixup_aarch64_ldst_imm12_scale4 "
+                          "VK_GOT IsNC");
+          return ELF::R_AARCH64_NONE;
+        }
+      }
 
       Ctx.reportError(Fixup.getLoc(),
                       "invalid fixup for 32-bit load/store instruction");
       return ELF::R_AARCH64_NONE;
     case AArch64::fixup_aarch64_ldst_imm12_scale8:
       if (SymLoc == AArch64MCExpr::VK_ABS && IsNC)
         return R_CLS(LDST64_ABS_LO12_NC);
       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;
       if (SymLoc == AArch64MCExpr::VK_DTPREL && !IsNC)
         return R_CLS(TLSLD_LDST64_DTPREL_LO12);
       if (SymLoc == AArch64MCExpr::VK_DTPREL && IsNC)
         return R_CLS(TLSLD_LDST64_DTPREL_LO12_NC);
       if (SymLoc == AArch64MCExpr::VK_TPREL && !IsNC)
         return R_CLS(TLSLE_LDST64_TPREL_LO12);
       if (SymLoc == AArch64MCExpr::VK_TPREL && IsNC)
         return R_CLS(TLSLE_LDST64_TPREL_LO12_NC);

test/MC/AArch64/adrp-relocation.s

 // RUN: llvm-mc -triple=aarch64-linux-gnu -filetype=obj -o - %s| llvm-readobj -r - | FileCheck %s
+// RUN: llvm-mc -target-abi=ilp32 -triple=aarch64-linux-gnu -filetype=obj \
+// RUN: -o - %s| llvm-readobj -r - | FileCheck -check-prefix=CHECK-ILP32 %s
         .text
 // These should produce an ADRP/ADD pair to calculate the address of
 // testfn. The important point is that LLVM shouldn't think it can deal with the
 // relocation on the ADRP itself (even though it knows everything about the
 // relative offsets of testfn and foo) because its value depends on where this
 // object file's .text section gets relocated in memory.
         adrp x0, sym
         adrp x0, :got:sym
         adrp x0, :gottprel:sym
         adrp x0, :tlsdesc:sym
 
         .global sym
 sym:
 // CHECK: R_AARCH64_ADR_PREL_PG_HI21 sym
 // CHECK: R_AARCH64_ADR_GOT_PAGE sym
 // CHECK: R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21 sym
 // CHECK: R_AARCH64_TLSDESC_ADR_PAGE21 sym
+// CHECK-ILP32: R_AARCH64_P32_ADR_PREL_PG_HI21 sym
+// CHECK-ILP32: R_AARCH64_P32_ADR_GOT_PAGE sym
+// CHECK-ILP32: R_AARCH64_P32_TLSIE_ADR_GOTTPREL_PAGE21 sym
+// CHECK-ILP32: R_AARCH64_P32_TLSDESC_ADR_PAGE21 sym

test/MC/AArch64/arm32-elf-relocs.s

 // CHECK: ldr w1, [x2, :tprel_lo12:sym]
 // CHECK: ldrsw x3, [x4, :tprel_lo12_nc:sym]
 // CHECK: ldr s4, [x5, :tprel_lo12_nc:sym]
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSLE_LDST32_TPREL_LO12 sym
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSLE_LDST32_TPREL_LO12_NC sym
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSLE_LDST32_TPREL_LO12_NC sym
 
    ldr x28, [x27, :lo12:sym]
-   ldr d26, [x25, #:lo12:sym]
+   ldr d26, [x25, :lo12:sym]
 // CHECK: ldr x28, [x27, :lo12:sym]
 // CHECK: ldr d26, [x25, :lo12:sym]
 // CHECK-OBJ-ILP32 R_AARCH64_P32_LDST64_ABS_LO12_NC sym
 // CHECK-OBJ-ILP32 R_AARCH64_P32_LDST64_ABS_LO12_NC sym
 
-   ldr x24, [x23, #:got_lo12:sym]
+   ldr x24, [x23, :got_lo12:sym]
    ldr d22, [x21, :got_lo12:sym]
 // CHECK: ldr x24, [x23, :got_lo12:sym]
 // CHECK: ldr d22, [x21, :got_lo12:sym]
-// CHECK-OBJ-ILP32 R_AARCH64_LD32_GOT_LO12_NC sym
-// CHECK-OBJ-ILP32 R_AARCH64_LD32_GOT_LO12_NC sym
+// CHECK-OBJ-ILP32 R_AARCH64_P32_LD32_GOT_LO12_NC sym
+// CHECK-OBJ-ILP32 R_AARCH64_P32_LD32_GOT_LO12_NC sym
 
    ldr x24, [x23, :dtprel_lo12_nc:sym]
-   ldr d22, [x21, #:dtprel_lo12:sym]
+   ldr d22, [x21, :dtprel_lo12:sym]
 // CHECK: ldr x24, [x23, :dtprel_lo12_nc:sym]
 // CHECK: ldr d22, [x21, :dtprel_lo12:sym]
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSLD_LDST64_DTPREL_LO12_NC sym
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSLD_LDST64_DTPREL_LO12 sym
 
-   ldr x24, [x23, #:tprel_lo12:sym]
+   ldr x24, [x23, :tprel_lo12:sym]
    ldr d22, [x21, :tprel_lo12_nc:sym]
 // CHECK: ldr x24, [x23, :tprel_lo12:sym]
 // CHECK: ldr d22, [x21, :tprel_lo12_nc:sym]
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSLE_LDST64_TPREL_LO12 sym
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSLE_LDST64_TPREL_LO12_NC sym
 
-#   ldr x24, [x23, :gottprel_lo12:sym]
-#   ldr d22, [x21, #:gottprel_lo12:sym]
+   ldr x24, [x23, :gottprel_lo12:sym]
+   ldr d22, [x21, :gottprel_lo12:sym]
 
-   ldr x24, [x23, #:tlsdesc_lo12:sym]
+   ldr x24, [x23, :tlsdesc_lo12:sym]
    ldr d22, [x21, :tlsdesc_lo12:sym]
 // CHECK: ldr x24, [x23, :tlsdesc_lo12:sym]
 // CHECK: ldr d22, [x21, :tlsdesc_lo12:sym]
 // Why is there a "_NC" at the end? "ELF for the ARM 64-bit architecture
 // (AArch64) beta" doesn't have that.
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSDESC_LD32_LO12_NC sym
 // CHECK-OBJ-ILP32 R_AARCH64_P32_TLSDESC_LD64_LO12_NC sym
 
    ldr q20, [x19, #:lo12:sym]
 // CHECK: ldr q20, [x19, :lo12:sym]
 // CHECK-OBJ-ILP32 R_AARCH64_P32_LDST128_ABS_LO12_NC sym
 
 // Since relocated instructions print without a '#', that syntax should
 // certainly be accepted when assembling.
    add x3, x5, :lo12:imm
 // CHECK: add x3, x5, :lo12:imm