[LLD] [COFF] Fix alignment of thunks for ARM/ARM64

[mstorsjo]

Summary

The alignment of ARM64 range extension thunks was fixed in 7c816492197a, but ARM range extension thunks, and import and delay import thunks for both also need aligning (like all code on ARM platforms).

I'm adding a test for alignment of ARM64 import thunks - not specifically adding tests for misalignment of all of them though.

[rnk]

I seem to recall we had some logic for boosting the alignment of known executable sections. Is it appropriate to reuse that for these thunks? It seems like each ISA has its own minimum code alignment.

Otherwise, sounds good, just wanted to check if there is a way to factor this more tightly.

[mstorsjo]

In D77796#1972959, @rnk wrote:

I seem to recall we had some logic for boosting the alignment of known executable sections. Is it appropriate to reuse that for these thunks? It seems like each ISA has its own minimum code alignment.

Hmm, I don't think I remember seeing that - do you remember more in detail where that would be?

[mstorsjo]

In D77796#1972996, @mstorsjo wrote:

In D77796#1972959, @rnk wrote:

I seem to recall we had some logic for boosting the alignment of known executable sections. Is it appropriate to reuse that for these thunks? It seems like each ISA has its own minimum code alignment.

Hmm, I don't think I remember seeing that - do you remember more in detail where that would be?

I tried grepping around a bit more but didn't really see anything such in LLD - @rnk?

[rnk]

lgtm

I must have been thinking of the /functionpadmin code:
https://github.com/llvm/llvm-project/blob/master/lld/COFF/Writer.cpp#L1262
It has nothing to do with alignment, just padding. We could add logic there to enforce a minimum ISA code alignment, but that seems separable.

[mstorsjo]

@tstellar - I think this one would be good for a backport to 10.0.1 - is it enough to tag you here, or do you want me to open a bug for backporting it?

lld/COFF/Chunks.h

public:
   explicit ImportThunkChunkX86(Defined *s) : ImportThunkChunk(s) {}
   size_t getSize() const override { return sizeof(importThunkX86); }
   void getBaserels(std::vector<Baserel> *res) override;
   void writeTo(uint8_t *buf) const override;
 };
 
 class ImportThunkChunkARM : public ImportThunkChunk {
 public:
-  explicit ImportThunkChunkARM(Defined *s) : ImportThunkChunk(s) {}
+  explicit ImportThunkChunkARM(Defined *s) : ImportThunkChunk(s) {
+    setAlignment(2);
+  }
   size_t getSize() const override { return sizeof(importThunkARM); }
   void getBaserels(std::vector<Baserel> *res) override;
   void writeTo(uint8_t *buf) const override;
 };
 
 class ImportThunkChunkARM64 : public ImportThunkChunk {
 public:
-  explicit ImportThunkChunkARM64(Defined *s) : ImportThunkChunk(s) {}
+  explicit ImportThunkChunkARM64(Defined *s) : ImportThunkChunk(s) {
+    setAlignment(4);
+  }
   size_t getSize() const override { return sizeof(importThunkARM64); }
   void writeTo(uint8_t *buf) const override;
 };
 
 class RangeExtensionThunkARM : public NonSectionChunk {
 public:
-  explicit RangeExtensionThunkARM(Defined *t) : target(t) {}
+  explicit RangeExtensionThunkARM(Defined *t) : target(t) { setAlignment(2); }
   size_t getSize() const override;
   void writeTo(uint8_t *buf) const override;
 
   Defined *target;
 };
 
 class RangeExtensionThunkARM64 : public NonSectionChunk {
 public:

lld/COFF/DLL.cpp

public:
   }
 
   Chunk *desc = nullptr;
   Defined *helper = nullptr;
 };
 
 class ThunkChunkARM : public NonSectionChunk {
 public:
-  ThunkChunkARM(Defined *i, Chunk *tm) : imp(i), tailMerge(tm) {}
+  ThunkChunkARM(Defined *i, Chunk *tm) : imp(i), tailMerge(tm) {
+    setAlignment(2);
+  }
 
   size_t getSize() const override { return sizeof(thunkARM); }
 
   void writeTo(uint8_t *buf) const override {
     memcpy(buf, thunkARM, sizeof(thunkARM));
     applyMOV32T(buf + 0, imp->getRVA() + config->imageBase);
     applyBranch24T(buf + 8, tailMerge->getRVA() - rva - 12);
   }
 
   void getBaserels(std::vector<Baserel> *res) override {
     res->emplace_back(rva + 0, IMAGE_REL_BASED_ARM_MOV32T);
   }
 
   Defined *imp = nullptr;
   Chunk *tailMerge = nullptr;
 };
 
 class TailMergeChunkARM : public NonSectionChunk {
 public:
-  TailMergeChunkARM(Chunk *d, Defined *h) : desc(d), helper(h) {}
+  TailMergeChunkARM(Chunk *d, Defined *h) : desc(d), helper(h) {
+    setAlignment(2);
+  }
 
   size_t getSize() const override { return sizeof(tailMergeARM); }
 
   void writeTo(uint8_t *buf) const override {
     memcpy(buf, tailMergeARM, sizeof(tailMergeARM));
     applyMOV32T(buf + 14, desc->getRVA() + config->imageBase);
     applyBranch24T(buf + 22, helper->getRVA() - rva - 26);
   }
 
   void getBaserels(std::vector<Baserel> *res) override {
     res->emplace_back(rva + 14, IMAGE_REL_BASED_ARM_MOV32T);
   }
 
   Chunk *desc = nullptr;
   Defined *helper = nullptr;
 };
 
 class ThunkChunkARM64 : public NonSectionChunk {
 public:
-  ThunkChunkARM64(Defined *i, Chunk *tm) : imp(i), tailMerge(tm) {}
+  ThunkChunkARM64(Defined *i, Chunk *tm) : imp(i), tailMerge(tm) {
+    setAlignment(4);
+  }
 
   size_t getSize() const override { return sizeof(thunkARM64); }
 
   void writeTo(uint8_t *buf) const override {
     memcpy(buf, thunkARM64, sizeof(thunkARM64));
     applyArm64Addr(buf + 0, imp->getRVA(), rva + 0, 12);
     applyArm64Imm(buf + 4, imp->getRVA() & 0xfff, 0);
     applyArm64Branch26(buf + 8, tailMerge->getRVA() - rva - 8);
   }
 
   Defined *imp = nullptr;
   Chunk *tailMerge = nullptr;
 };
 
 class TailMergeChunkARM64 : public NonSectionChunk {
 public:
-  TailMergeChunkARM64(Chunk *d, Defined *h) : desc(d), helper(h) {}
+  TailMergeChunkARM64(Chunk *d, Defined *h) : desc(d), helper(h) {
+    setAlignment(4);
+  }
 
   size_t getSize() const override { return sizeof(tailMergeARM64); }
 
   void writeTo(uint8_t *buf) const override {
     memcpy(buf, tailMergeARM64, sizeof(tailMergeARM64));
     applyArm64Addr(buf + 44, desc->getRVA(), rva + 44, 12);
     applyArm64Imm(buf + 48, desc->getRVA() & 0xfff, 0);
     applyArm64Branch26(buf + 52, helper->getRVA() - rva - 52);

lld/test/COFF/arm64-import2.test

 # REQUIRES: aarch64
 
 # RUN: yaml2obj < %s > %t.obj
 # RUN: llvm-objdump -d %t.obj | FileCheck %s --check-prefix BEFORE
 # RUN: lld-link /entry:main /subsystem:console /out:%t.exe %t.obj %p/Inputs/library-arm64.lib %p/Inputs/library2-arm64.lib
 # RUN: llvm-objdump -d %t.exe | FileCheck %s --check-prefix AFTER
 # RUN: llvm-readobj --coff-imports %t.exe | FileCheck %s -check-prefix IMPORTS
 
 # BEFORE: Disassembly of section .text:
 # BEFORE-EMPTY:
 # BEFORE:        0:       00 00 00 94     bl      0x0
 # BEFORE:        4:       00 00 00 94     bl      0x4
 # BEFORE:        8:       c0 03 5f d6     ret
+# BEFORE:        c:       ff              <unknown>
 
 # AFTER: Disassembly of section .text:
 # AFTER-EMPTY:
-# AFTER:  140001000:      03 00 00 94     bl      0x14000100c
-# AFTER:  140001004:      05 00 00 94     bl      0x140001018
+# AFTER:  140001000:      04 00 00 94     bl      0x140001010
+# AFTER:  140001004:      06 00 00 94     bl      0x14000101c
 # AFTER:  140001008:      c0 03 5f d6     ret
-# AFTER:  14000100c:      10 00 00 b0     adrp    x16, #4096
-# AFTER:  140001010:      10 32 40 f9     ldr     x16, [x16, #96]
-# AFTER:  140001014:      00 02 1f d6     br      x16
-# AFTER:  140001018:      10 00 00 b0     adrp    x16, #4096
-# AFTER:  14000101c:      10 3a 40 f9     ldr     x16, [x16, #112]
-# AFTER:  140001020:      00 02 1f d6     br      x16
+# AFTER:  14000100c:      ff cc cc cc     <unknown>
+# AFTER:  140001010:      10 00 00 b0     adrp    x16, #4096
+# AFTER:  140001014:      10 32 40 f9     ldr     x16, [x16, #96]
+# AFTER:  140001018:      00 02 1f d6     br      x16
+# AFTER:  14000101c:      10 00 00 b0     adrp    x16, #4096
+# AFTER:  140001020:      10 3a 40 f9     ldr     x16, [x16, #112]
+# AFTER:  140001024:      00 02 1f d6     br      x16
 
 # IMPORTS: Import {
 # IMPORTS:   Name: library.dll
 # IMPORTS:   ImportLookupTableRVA: 0x2040
 # IMPORTS:   ImportAddressTableRVA: 0x2060
 # IMPORTS:   Symbol: function (2)
 # IMPORTS: }
 # IMPORTS: Import {
 # IMPORTS:   Name: library2.dll
 # IMPORTS:   ImportLookupTableRVA: 0x2050
 # IMPORTS:   ImportAddressTableRVA: 0x2070
 # IMPORTS:   Symbol: function2 (0)
 # IMPORTS: }
 
 --- !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:     0000009400000094C0035FD6
+    SectionData:     0000009400000094C0035FD6FF
     Relocations:
       - VirtualAddress:  0
         SymbolName:      function
         Type:            IMAGE_REL_ARM64_BRANCH26
       - VirtualAddress:  4
         SymbolName:      function2
         Type:            IMAGE_REL_ARM64_BRANCH26
 symbols: