[lld] [COFF, ARM64] Handle ADRP immediate offsets in relocations

[mstorsjo]

Summary

Also handle overflow correctly in LDR/STR relocations. Even if the offset range of a 8 byte LDR instruction is 15 bit (even if the immediate itself is 12 bit) due to a 3 bit shift, only include up to 12 bits of byte level offset after doing the relocation, by limiting the range of the immediate by the number of shifted bits.

[ruiu]

It's not very clear to me if what this patch does is correct, even though I clearly understand each line of code. Can you write a function comment instead of writing a comment for each line?

[mstorsjo]

Wrote a new per-function comment as suggested by Rui, hopefully explaining better what is done now.

[ruiu]

COFF/Chunks.cpp

202 ↗

(On Diff #107957)

So this is more like a scale than a size?

(Is there any documentation about this relocation?)

211–213 ↗

(On Diff #107957)

You are shifting Imm to the right and then to the left, which looks a bit odd.

213 ↗

(On Diff #107957)

So, if I understand correctly, this is equivalent to

write32le(Off, (Orig & FFB003FF) | (Imm + ...));

(Sorry I cannot calculate it in my head but I think it should be simpler.)

[mstorsjo]

COFF/Chunks.cpp

202 ↗

(On Diff #107957)

Yes, it's actually a shift/scale. I picked the name Size originally, because that's the name of the bits in the opcode, from the ARMv8 architecture reference manual.

I don't know of any documentation of the relocation itself - the official pecoff document only lists the relocation names/numbers, but no description of what they should do. This code is based on testing how link.exe handles them.

211–213 ↗

(On Diff #107957)

I can get reduce the amount of shifting by doing the calculation in scaled units instead of in bytes - I did that for clarity, but perhaps it only obscured things.

In that case, it's probably simplest to extend the existing applyArm64Imm with a parameter that limits the wraparound range/mask from 0xFFF to 0xFFF >> Scale - that's the actual effect of this change.

[mstorsjo]

Updated handling of scaled ldr relocations by using applyArm64Imm as before, but with a new parameter instead. Hopefully this is clearer.

[ruiu]

COFF/Chunks.cpp

187 ↗

(On Diff #108037)

We do not use optional parameters that often, as it basically defines two or more functions with the same name which increases program complexity (even though the increase may be small.) Just pass 0 instead.

test/COFF/arm64-symbol-offset.yaml

1 ↗

(On Diff #108037)

Can you reduce the size of the test? I don't think you need a test case of 200 lines to verify this change.

[mstorsjo]

Removed use of a default parameter, squashed a smaller testcase into the existing arm64-relocs-imports.test instead.

[mstorsjo]

Rui, any further comments on this one?

[ruiu]

LGTM

lld/trunk/COFF/Chunks.cpp

void SectionChunk::applyRelARM(uint8_t *Off, uint16_t Type, OutputSection *OS,
   case IMAGE_REL_ARM_BLX23T:    applyBranch24T(Off, SX - P - 4); break;
   case IMAGE_REL_ARM_SECTION:   applySecIdx(Off, OS); break;
   case IMAGE_REL_ARM_SECREL:    applySecRel(this, Off, OS, S); break;
   default:
     fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
   }
 }
 
-static void applyArm64Addr(uint8_t *Off, uint64_t Imm) {
+// Interpret the existing immediate value as a byte offset to the
+// target symbol, then update the instruction with the immediate as
+// the page offset from the current instruction to the target.
+static void applyArm64Addr(uint8_t *Off, uint64_t S, uint64_t P) {
+  uint32_t Orig = read32le(Off);
+  uint64_t Imm = ((Orig >> 29) & 0x3) | ((Orig >> 3) & 0x1FFFFC);
+  S += Imm;
+  Imm = (S >> 12) - (P >> 12);
   uint32_t ImmLo = (Imm & 0x3) << 29;
   uint32_t ImmHi = (Imm & 0x1FFFFC) << 3;
   uint64_t Mask = (0x3 << 29) | (0x1FFFFC << 3);
-  write32le(Off, (read32le(Off) & ~Mask) | ImmLo | ImmHi);
+  write32le(Off, (Orig & ~Mask) | ImmLo | ImmHi);
 }
 
 // Update the immediate field in a AARCH64 ldr, str, and add instruction.
-static void applyArm64Imm(uint8_t *Off, uint64_t Imm) {
+// Optionally limit the range of the written immediate by one or more bits
+// (RangeLimit).
+static void applyArm64Imm(uint8_t *Off, uint64_t Imm, uint32_t RangeLimit) {
   uint32_t Orig = read32le(Off);
   Imm += (Orig >> 10) & 0xFFF;
   Orig &= ~(0xFFF << 10);
-  write32le(Off, Orig | ((Imm & 0xFFF) << 10));
+  write32le(Off, Orig | ((Imm & (0xFFF >> RangeLimit)) << 10));
 }
 
+// Add the 12 bit page offset to the existing immediate.
+// Ldr/str instructions store the opcode immediate scaled
+// by the load/store size (giving a larger range for larger
+// loads/stores). The immediate is always (both before and after
+// fixing up the relocation) stored scaled similarly.
+// Even if larger loads/stores have a larger range, limit the
+// effective offset to 12 bit, since it is intended to be a
+// page offset.
 static void applyArm64Ldr(uint8_t *Off, uint64_t Imm) {
   uint32_t Orig = read32le(Off);
   uint32_t Size = Orig >> 30;
   // 0x04000000 indicates SIMD/FP registers
   // 0x00800000 indicates 128 bit
   if ((Orig & 0x4800000) == 0x4800000)
     Size += 4;
   if ((Imm & ((1 << Size) - 1)) != 0)
     fatal("misaligned ldr/str offset");
-  applyArm64Imm(Off, Imm >> Size);
+  applyArm64Imm(Off, Imm >> Size, Size);
 }
 
 void SectionChunk::applyRelARM64(uint8_t *Off, uint16_t Type, OutputSection *OS,
                                  uint64_t S, uint64_t P) const {
   switch (Type) {
-  case IMAGE_REL_ARM64_PAGEBASE_REL21: applyArm64Addr(Off, (S >> 12) - (P >> 12)); break;
-  case IMAGE_REL_ARM64_PAGEOFFSET_12A: applyArm64Imm(Off, S & 0xfff); break;
+  case IMAGE_REL_ARM64_PAGEBASE_REL21: applyArm64Addr(Off, S, P); break;
+  case IMAGE_REL_ARM64_PAGEOFFSET_12A: applyArm64Imm(Off, S & 0xfff, 0); break;
   case IMAGE_REL_ARM64_PAGEOFFSET_12L: applyArm64Ldr(Off, S & 0xfff); break;
   case IMAGE_REL_ARM64_BRANCH26:       or32(Off, ((S - P) & 0x0FFFFFFC) >> 2); break;
   case IMAGE_REL_ARM64_ADDR32:         add32(Off, S + Config->ImageBase); break;
   case IMAGE_REL_ARM64_ADDR64:         add64(Off, S + Config->ImageBase); break;
   default:
     fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
   }
 }

 
 void ImportThunkChunkARM::writeTo(uint8_t *Buf) const {
   memcpy(Buf + OutputSectionOff, ImportThunkARM, sizeof(ImportThunkARM));
   // Fix mov.w and mov.t operands.
   applyMOV32T(Buf + OutputSectionOff, ImpSymbol->getRVA() + Config->ImageBase);
 }
 
 void ImportThunkChunkARM64::writeTo(uint8_t *Buf) const {
-  int64_t PageOff = (ImpSymbol->getRVA() >> 12) - (RVA >> 12);
   int64_t Off = ImpSymbol->getRVA() & 0xfff;
   memcpy(Buf + OutputSectionOff, ImportThunkARM64, sizeof(ImportThunkARM64));
-  applyArm64Addr(Buf + OutputSectionOff, PageOff);
+  applyArm64Addr(Buf + OutputSectionOff, ImpSymbol->getRVA(), RVA);
   applyArm64Ldr(Buf + OutputSectionOff + 4, Off);
 }
 
 void LocalImportChunk::getBaserels(std::vector<Baserel> *Res) {
   Res->emplace_back(getRVA());
 }
 
 size_t LocalImportChunk::getSize() const {

lld/trunk/test/COFF/arm64-relocs-imports.test

 # BEFORE:       3c:       00 01 40 fd     ldr     d0, [x8]
 # BEFORE:       40:       00 01 c0 3d     ldr     q0, [x8]
 # BEFORE:       44:       00 01 00 3d     str     b0, [x8]
 # BEFORE:       48:       00 01 00 7d     str     h0, [x8]
 # BEFORE:       4c:       00 01 00 bd     str     s0, [x8]
 # BEFORE:       50:       00 01 00 fd     str     d0, [x8]
 # BEFORE:       54:       00 01 80 3d     str     q0, [x8]
 # BEFORE:       58:       00 05 40 f9     ldr     x0, [x8, #8]
-# BEFORE:       5c:       e0 03 1f 2a     mov      w0, wzr
-# BEFORE:       60:       fe 07 41 f8     ldr     x30, [sp], #16
-# BEFORE:       64:       c0 03 5f d6     ret
-# BEFORE:       68:       08 00 00 00     <unknown>
-# BEFORE:       6c:       00 00 00 00     <unknown>
+# BEFORE:       5c:       20 1a 01 b0     adrp    x0, #36982784
+# BEFORE:       60:       00 fc 4f f9     ldr     x0, [x0, #8184]
+# BEFORE:       64:       e0 03 1f 2a     mov      w0, wzr
+# BEFORE:       68:       fe 07 41 f8     ldr     x30, [sp], #16
+# BEFORE:       6c:       c0 03 5f d6     ret
+# BEFORE:       70:       08 00 00 00     <unknown>
+# BEFORE:       74:       00 00 00 00     <unknown>
 
 # AFTER: Disassembly of section .text:
 # AFTER:  140002000:      fe 0f 1f f8     str     x30, [sp, #-16]!
 # AFTER:  140002004:      e0 ff ff f0     adrp    x0, #-4096
 # AFTER:  140002008:      00 18 00 91     add     x0, x0, #6
-# AFTER:  14000200c:      19 00 00 94     bl      #100
+# AFTER:  14000200c:      1b 00 00 94     bl      #108
 # AFTER:  140002010:      00 21 40 39     ldrb    w0, [x8, #8]
 # AFTER:  140002014:      00 11 40 79     ldrh    w0, [x8, #8]
 # AFTER:  140002018:      00 09 40 b9     ldr     w0, [x8, #8]
 # AFTER:  14000201c:      00 05 40 f9     ldr     x0, [x8, #8]
 # AFTER:  140002020:      00 21 00 39     strb    w0, [x8, #8]
 # AFTER:  140002024:      00 11 00 79     strh    w0, [x8, #8]
 # AFTER:  140002028:      00 09 00 b9     str     w0, [x8, #8]
 # AFTER:  14000202c:      00 05 00 f9     str     x0, [x8, #8]
 # AFTER:  140002030:      00 41 40 3d     ldr     b0, [x8, #16]
 # AFTER:  140002034:      00 21 40 7d     ldr     h0, [x8, #16]
 # AFTER:  140002038:      00 11 40 bd     ldr     s0, [x8, #16]
 # AFTER:  14000203c:      00 09 40 fd     ldr     d0, [x8, #16]
 # AFTER:  140002040:      00 05 c0 3d     ldr     q0, [x8, #16]
 # AFTER:  140002044:      00 41 00 3d     str     b0, [x8, #16]
 # AFTER:  140002048:      00 21 00 7d     str     h0, [x8, #16]
 # AFTER:  14000204c:      00 11 00 bd     str     s0, [x8, #16]
 # AFTER:  140002050:      00 09 00 fd     str     d0, [x8, #16]
 # AFTER:  140002054:      00 05 80 3d     str     q0, [x8, #16]
 # AFTER:  140002058:      00 09 40 f9     ldr     x0, [x8, #16]
-# AFTER:  14000205c:      e0 03 1f 2a     mov      w0, wzr
-# AFTER:  140002060:      fe 07 41 f8     ldr     x30, [sp], #16
-# AFTER:  140002064:      c0 03 5f d6     ret
-# AFTER:  140002068:      10 10 00 40     <unknown>
-# AFTER:  14000206c:      01 00 00 00     <unknown>
-# AFTER:  140002070:      10 00 00 b0     adrp    x16, #4096
-# AFTER:  140002074:      10 1e 40 f9     ldr     x16, [x16, #56]
-# AFTER:  140002078:      00 02 1f d6     br      x16
+# AFTER:  14000205c:      00 00 00 b0     adrp    x0, #4096
+# AFTER:  140002060:      00 fc 47 f9     ldr     x0, [x0, #4088]
+# AFTER:  140002064:      e0 03 1f 2a     mov      w0, wzr
+# AFTER:  140002068:      fe 07 41 f8     ldr     x30, [sp], #16
+# AFTER:  14000206c:      c0 03 5f d6     ret
+# AFTER:  140002070:      10 10 00 40     <unknown>
+# AFTER:  140002074:      01 00 00 00     <unknown>
+# AFTER:  140002078:      10 00 00 b0     adrp    x16, #4096
+# AFTER:  14000207c:      10 1e 40 f9     ldr     x16, [x16, #56]
+# AFTER:  140002080:      00 02 1f d6     br      x16
 
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_ARM64
   Characteristics: [  ]
 sections:
   - Name:            .text
     Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
     Alignment:       4
-    SectionData:     FE0F1FF80000009000080091000000940001403900014079000140B9000140F90001003900010079000100B9000100F90001403D0001407D000140BD000140FD0001C03D0001003D0001007D000100BD000100FD0001803D000540F9E0031F2AFE0741F8C0035FD60800000000000000
+    SectionData:     FE0F1FF80000009000080091000000940001403900014079000140B9000140F90001003900010079000100B9000100F90001403D0001407D000140BD000140FD0001C03D0001003D0001007D000100BD000100FD0001803D000540F9201A01B000FC4FF9E0031F2AFE0741F8C0035FD60800000000000000
     Relocations:
       - VirtualAddress:  4
         SymbolName:      .Lstr
         Type:            4
       - VirtualAddress:  8
         SymbolName:      .Lstr
         Type:            6
       - VirtualAddress:  12
Relocations:
         SymbolName:      .Lglobal16
         Type:            7
       - VirtualAddress:  84
         SymbolName:      .Lglobal16
         Type:            7
       - VirtualAddress:  88
         SymbolName:      .Lglobal
         Type:            7
-      - VirtualAddress:  104
+      - VirtualAddress:  92
+        SymbolName:      .Lglobal16
+        Type:            4
+      - VirtualAddress:  96
+        SymbolName:      .Lglobal0
+        Type:            7
+      - VirtualAddress:  112
         SymbolName:      .Lglobal
         Type:            14
   - Name:            .data
     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
     Alignment:       4
     SectionData:     ''
   - Name:            .bss
     Characteristics: [ IMAGE_SCN_CNT_UNINITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
- Name:            .Lglobal
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_STATIC
   - Name:            .Lglobal16
     Value:           16
     SectionNumber:   4
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            .Lglobal0
+    Value:           0
+    SectionNumber:   4
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
   - Name:            function
     Value:           0
     SectionNumber:   0
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
 ...