[llvm] 5be7f2a - [MC,AArch64] Suppress local symbol to STT_SECTION conversion for GOT relocations

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 29 11:30:14 PDT 2023


Author: Fangrui Song
Date: 2023-08-29T11:07:12-07:00
New Revision: 5be7f2a94371b2a5c129309e2bcca9254ec8dc42

URL: https://github.com/llvm/llvm-project/commit/5be7f2a94371b2a5c129309e2bcca9254ec8dc42
DIFF: https://github.com/llvm/llvm-project/commit/5be7f2a94371b2a5c129309e2bcca9254ec8dc42.diff

LOG: [MC,AArch64] Suppress local symbol to STT_SECTION conversion for GOT relocations

Assemblers change certain relocations referencing a local symbol to
reference the section symbol instead. This conversion is disabled for
many conditions (`shouldRelocateWithSymbol`), e.g. TLS symbol, for most
targets (including AArch32, x86, PowerPC, and RISC-V) GOT-generating
relocations.

However, AArch64 encodes the GOT-generating intent in MCValue::RefKind
instead of MCSymbolRef::Kind (see commit
0999cbd0b9ed8aa893cce10d681dec6d54b200ad (2014)), therefore not affected
by the code `case MCSymbolRefExpr::VK_GOT:`. As GNU ld and ld.lld
create GOT entries based on the symbol, ignoring addend, the two ldr
instructions will share the same GOT entry, which is not expected:
```
ldr     x1, [x1, :got_lo12:x]  // converted to .data+0
ldr     x1, [x1, :got_lo12:y]  // converted to .data+4

.data
// .globl x, y  would suppress STT_SECTION conversion
x:
.zero 4
y:
.long 42
```

This patch changes AArch64 to suppress local symbol to STT_SECTION
conversion for GOT relocations, matching most other targets. x and y
will use different GOT entries, which IMO is the most sensable behavior.

With this change, the ABI decision on https://github.com/ARM-software/abi-aa/issues/217
will only affect relocations explicitly referencing STT_SECTION symbols, e.g.
```
ldr     x1, [x1, :got_lo12:(.data+0)]
ldr     x1, [x1, :got_lo12:(.data+4)]
// I consider this unreasonable uses
```

IMO all reasonable use cases are unaffected.

Link: https://github.com/llvm/llvm-project/issues/63418
GNU assembler PR: https://sourceware.org/bugzilla/show_bug.cgi?id=30788

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D158577

Added: 
    

Modified: 
    llvm/include/llvm/MC/MCELFObjectWriter.h
    llvm/lib/MC/ELFObjectWriter.cpp
    llvm/lib/MC/MCELFObjectTargetWriter.cpp
    llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
    llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
    llvm/lib/Target/Lanai/MCTargetDesc/LanaiELFObjectWriter.cpp
    llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
    llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
    llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
    llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp
    llvm/lib/Target/VE/MCTargetDesc/VEELFObjectWriter.cpp
    llvm/lib/Target/Xtensa/MCTargetDesc/XtensaELFObjectWriter.cpp
    llvm/test/MC/AArch64/arm64-elf-relocs.s

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/MC/MCELFObjectWriter.h b/llvm/include/llvm/MC/MCELFObjectWriter.h
index aca77f5f2687a3..bc6a62f8e57b11 100644
--- a/llvm/include/llvm/MC/MCELFObjectWriter.h
+++ b/llvm/include/llvm/MC/MCELFObjectWriter.h
@@ -88,7 +88,7 @@ class MCELFObjectTargetWriter : public MCObjectTargetWriter {
   virtual unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                                 const MCFixup &Fixup, bool IsPCRel) const = 0;
 
-  virtual bool needsRelocateWithSymbol(const MCSymbol &Sym,
+  virtual bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                        unsigned Type) const;
 
   virtual void sortRelocs(const MCAssembler &Asm,

diff  --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 6a6befdd305492..816aa21321095d 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -227,8 +227,7 @@ class ELFObjectWriter : public MCObjectWriter {
 
   bool hasRelocationAddend() const;
 
-  bool shouldRelocateWithSymbol(const MCAssembler &Asm,
-                                const MCSymbolRefExpr *RefA,
+  bool shouldRelocateWithSymbol(const MCAssembler &Asm, const MCValue &Val,
                                 const MCSymbolELF *Sym, uint64_t C,
                                 unsigned Type) const;
 
@@ -1297,10 +1296,11 @@ void ELFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
 // to use a relocation with a section if that is possible. Using the section
 // allows us to omit some local symbols from the symbol table.
 bool ELFObjectWriter::shouldRelocateWithSymbol(const MCAssembler &Asm,
-                                               const MCSymbolRefExpr *RefA,
+                                               const MCValue &Val,
                                                const MCSymbolELF *Sym,
                                                uint64_t C,
                                                unsigned Type) const {
+  const MCSymbolRefExpr *RefA = Val.getSymA();
   // A PCRel relocation to an absolute value has no symbol (or section). We
   // represent that with a relocation to a null section.
   if (!RefA)
@@ -1419,7 +1419,7 @@ bool ELFObjectWriter::shouldRelocateWithSymbol(const MCAssembler &Asm,
   if (Asm.isThumbFunc(Sym))
     return true;
 
-  if (TargetObjectWriter->needsRelocateWithSymbol(*Sym, Type))
+  if (TargetObjectWriter->needsRelocateWithSymbol(Val, *Sym, Type))
     return true;
   return false;
 }
@@ -1484,7 +1484,7 @@ void ELFObjectWriter::recordRelocation(MCAssembler &Asm,
   const auto *Parent = cast<MCSectionELF>(Fragment->getParent());
   // Emiting relocation with sybmol for CG Profile to  help with --cg-profile.
   bool RelocateWithSymbol =
-      shouldRelocateWithSymbol(Asm, RefA, SymA, C, Type) ||
+      shouldRelocateWithSymbol(Asm, Target, SymA, C, Type) ||
       (Parent->getType() == ELF::SHT_LLVM_CALL_GRAPH_PROFILE);
   uint64_t Addend = 0;
 

diff  --git a/llvm/lib/MC/MCELFObjectTargetWriter.cpp b/llvm/lib/MC/MCELFObjectTargetWriter.cpp
index a81eab9ca296be..c35e1f26dc1efa 100644
--- a/llvm/lib/MC/MCELFObjectTargetWriter.cpp
+++ b/llvm/lib/MC/MCELFObjectTargetWriter.cpp
@@ -17,7 +17,8 @@ MCELFObjectTargetWriter::MCELFObjectTargetWriter(bool Is64Bit_, uint8_t OSABI_,
     : OSABI(OSABI_), ABIVersion(ABIVersion_), EMachine(EMachine_),
       HasRelocationAddend(HasRelocationAddend_), Is64Bit(Is64Bit_) {}
 
-bool MCELFObjectTargetWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
+bool MCELFObjectTargetWriter::needsRelocateWithSymbol(const MCValue &,
+                                                      const MCSymbol &,
                                                       unsigned Type) const {
   return false;
 }

diff  --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
index 5d6a41558bc624..9de40661298cca 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
@@ -39,6 +39,8 @@ class AArch64ELFObjectWriter : public MCELFObjectTargetWriter {
 protected:
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                         const MCFixup &Fixup, bool IsPCRel) const override;
+  bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
+                               unsigned Type) const override;
   bool IsILP32;
 };
 
@@ -459,6 +461,12 @@ unsigned AArch64ELFObjectWriter::getRelocType(MCContext &Ctx,
   llvm_unreachable("Unimplemented fixup -> relocation");
 }
 
+bool AArch64ELFObjectWriter::needsRelocateWithSymbol(const MCValue &Val,
+                                                     const MCSymbol &,
+                                                     unsigned) const {
+  return (Val.getRefKind() & AArch64MCExpr::VK_GOT) == AArch64MCExpr::VK_GOT;
+}
+
 MCSectionELF *
 AArch64ELFObjectWriter::getMemtagRelocsSection(MCContext &Ctx) const {
   return Ctx.getELFSection(".memtag.globals.static",

diff  --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
index caebace2eb786f..f2ca6f91477f44 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
@@ -38,7 +38,7 @@ namespace {
     unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                           const MCFixup &Fixup, bool IsPCRel) const override;
 
-    bool needsRelocateWithSymbol(const MCSymbol &Sym,
+    bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                  unsigned Type) const override;
 
     void addTargetSectionFlags(MCContext &Ctx, MCSectionELF &Sec) override;
@@ -51,7 +51,8 @@ ARMELFObjectWriter::ARMELFObjectWriter(uint8_t OSABI)
                             ELF::EM_ARM,
                             /*HasRelocationAddend*/ false) {}
 
-bool ARMELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
+bool ARMELFObjectWriter::needsRelocateWithSymbol(const MCValue &,
+                                                 const MCSymbol &,
                                                  unsigned Type) const {
   // FIXME: This is extremely conservative. This really needs to use an
   // explicit list with a clear explanation for why each realocation needs to

diff  --git a/llvm/lib/Target/Lanai/MCTargetDesc/LanaiELFObjectWriter.cpp b/llvm/lib/Target/Lanai/MCTargetDesc/LanaiELFObjectWriter.cpp
index 919d43ad9b9b0c..a21518e44116e3 100644
--- a/llvm/lib/Target/Lanai/MCTargetDesc/LanaiELFObjectWriter.cpp
+++ b/llvm/lib/Target/Lanai/MCTargetDesc/LanaiELFObjectWriter.cpp
@@ -26,7 +26,7 @@ class LanaiELFObjectWriter : public MCELFObjectTargetWriter {
 protected:
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                         const MCFixup &Fixup, bool IsPCRel) const override;
-  bool needsRelocateWithSymbol(const MCSymbol &SD,
+  bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                unsigned Type) const override;
 };
 
@@ -72,7 +72,8 @@ unsigned LanaiELFObjectWriter::getRelocType(MCContext & /*Ctx*/,
   return Type;
 }
 
-bool LanaiELFObjectWriter::needsRelocateWithSymbol(const MCSymbol & /*SD*/,
+bool LanaiELFObjectWriter::needsRelocateWithSymbol(const MCValue &,
+                                                   const MCSymbol &,
                                                    unsigned Type) const {
   switch (Type) {
   case ELF::R_LANAI_21:

diff  --git a/llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp b/llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
index 84e8c9f071fb1b..181b82f14bfe86 100644
--- a/llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
+++ b/llvm/lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
@@ -62,7 +62,7 @@ class MipsELFObjectWriter : public MCELFObjectTargetWriter {
 
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                         const MCFixup &Fixup, bool IsPCRel) const override;
-  bool needsRelocateWithSymbol(const MCSymbol &Sym,
+  bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                unsigned Type) const override;
   void sortRelocs(const MCAssembler &Asm,
                   std::vector<ELFRelocationEntry> &Relocs) override;
@@ -505,14 +505,15 @@ void MipsELFObjectWriter::sortRelocs(const MCAssembler &Asm,
     Relocs[CopyTo++] = R.R;
 }
 
-bool MipsELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
+bool MipsELFObjectWriter::needsRelocateWithSymbol(const MCValue &Val,
+                                                  const MCSymbol &Sym,
                                                   unsigned Type) const {
   // If it's a compound relocation for N64 then we need the relocation if any
   // sub-relocation needs it.
   if (!isUInt<8>(Type))
-    return needsRelocateWithSymbol(Sym, Type & 0xff) ||
-           needsRelocateWithSymbol(Sym, (Type >> 8) & 0xff) ||
-           needsRelocateWithSymbol(Sym, (Type >> 16) & 0xff);
+    return needsRelocateWithSymbol(Val, Sym, Type & 0xff) ||
+           needsRelocateWithSymbol(Val, Sym, (Type >> 8) & 0xff) ||
+           needsRelocateWithSymbol(Val, Sym, (Type >> 16) & 0xff);
 
   switch (Type) {
   default:

diff  --git a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
index 7609ac461ea872..6a72b7b9ad0534 100644
--- a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
+++ b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
@@ -28,7 +28,7 @@ namespace {
     unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                           const MCFixup &Fixup, bool IsPCRel) const override;
 
-    bool needsRelocateWithSymbol(const MCSymbol &Sym,
+    bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                  unsigned Type) const override;
   };
 }
@@ -472,7 +472,8 @@ unsigned PPCELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target,
   return Type;
 }
 
-bool PPCELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
+bool PPCELFObjectWriter::needsRelocateWithSymbol(const MCValue &,
+                                                 const MCSymbol &Sym,
                                                  unsigned Type) const {
   switch (Type) {
     default:

diff  --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
index 7a959dbb6390b6..0799267eaf7c76 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
@@ -27,7 +27,7 @@ class RISCVELFObjectWriter : public MCELFObjectTargetWriter {
 
   // Return true if the given relocation must be with a symbol rather than
   // section plus offset.
-  bool needsRelocateWithSymbol(const MCSymbol &Sym,
+  bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                unsigned Type) const override {
     // TODO: this is very conservative, update once RISC-V psABI requirements
     //       are clarified.

diff  --git a/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp b/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp
index c48beab0122925..f17d3e997452d4 100644
--- a/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp
+++ b/llvm/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp
@@ -32,9 +32,8 @@ namespace {
     unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                           const MCFixup &Fixup, bool IsPCRel) const override;
 
-    bool needsRelocateWithSymbol(const MCSymbol &Sym,
+    bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                  unsigned Type) const override;
-
   };
 }
 
@@ -124,8 +123,9 @@ unsigned SparcELFObjectWriter::getRelocType(MCContext &Ctx,
   return ELF::R_SPARC_NONE;
 }
 
-bool SparcELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
-                                                 unsigned Type) const {
+bool SparcELFObjectWriter::needsRelocateWithSymbol(const MCValue &,
+                                                   const MCSymbol &,
+                                                   unsigned Type) const {
   switch (Type) {
     default:
       return false;

diff  --git a/llvm/lib/Target/VE/MCTargetDesc/VEELFObjectWriter.cpp b/llvm/lib/Target/VE/MCTargetDesc/VEELFObjectWriter.cpp
index 1c89d6444d1145..b2cdf29e4f3828 100644
--- a/llvm/lib/Target/VE/MCTargetDesc/VEELFObjectWriter.cpp
+++ b/llvm/lib/Target/VE/MCTargetDesc/VEELFObjectWriter.cpp
@@ -31,7 +31,7 @@ class VEELFObjectWriter : public MCELFObjectTargetWriter {
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                         const MCFixup &Fixup, bool IsPCRel) const override;
 
-  bool needsRelocateWithSymbol(const MCSymbol &Sym,
+  bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                unsigned Type) const override;
 };
 } // namespace
@@ -134,7 +134,8 @@ unsigned VEELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target,
   return ELF::R_VE_NONE;
 }
 
-bool VEELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
+bool VEELFObjectWriter::needsRelocateWithSymbol(const MCValue &,
+                                                const MCSymbol &,
                                                 unsigned Type) const {
   switch (Type) {
   default:

diff  --git a/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaELFObjectWriter.cpp b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaELFObjectWriter.cpp
index 7788790ee66c24..7472371932f119 100644
--- a/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaELFObjectWriter.cpp
+++ b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaELFObjectWriter.cpp
@@ -32,7 +32,7 @@ class XtensaObjectWriter : public MCELFObjectTargetWriter {
 protected:
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                         const MCFixup &Fixup, bool IsPCRel) const override;
-  bool needsRelocateWithSymbol(const MCSymbol &Sym,
+  bool needsRelocateWithSymbol(const MCValue &Val, const MCSymbol &Sym,
                                unsigned Type) const override;
 };
 } // namespace
@@ -60,7 +60,8 @@ llvm::createXtensaObjectWriter(uint8_t OSABI, bool IsLittleEndian) {
   return std::make_unique<XtensaObjectWriter>(OSABI);
 }
 
-bool XtensaObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
+bool XtensaObjectWriter::needsRelocateWithSymbol(const MCValue &,
+                                                 const MCSymbol &,
                                                  unsigned Type) const {
   return false;
 }

diff  --git a/llvm/test/MC/AArch64/arm64-elf-relocs.s b/llvm/test/MC/AArch64/arm64-elf-relocs.s
index e958f300b47ab6..8813c4bd7d1aa1 100644
--- a/llvm/test/MC/AArch64/arm64-elf-relocs.s
+++ b/llvm/test/MC/AArch64/arm64-elf-relocs.s
@@ -316,3 +316,24 @@ trickQuestion:
 // CHECK: ldr d22, :got:sym
 // CHECK-OBJ-LP64: R_AARCH64_GOT_LD_PREL19 sym
 // CHECK-OBJ-LP64: R_AARCH64_GOT_LD_PREL19 sym
+
+// GOT relocations referencing local symbols are not converted to reference
+// STT_SECTION symbols. https://github.com/llvm/llvm-project/issues/63418
+  ldr x0, [x0, :got_lo12:local0]
+  ldr x1, [x1, :got_lo12:local1]
+  ldr x2, [x2, :gotpage_lo15:local2]
+  adrp x3, :got:local3
+// CHECK:      ldr x0, [x0, :got_lo12:local0]
+// CHECK-NEXT: ldr x1, [x1, :got_lo12:local1]
+// CHECK-NEXT: ldr x2, [x2, :gotpage_lo15:local2]
+// CHECK-NEXT: adrp x3, :got:local3
+// CHECK-OBJ-LP64:      R_AARCH64_LD64_GOT_LO12_NC local0{{$}}
+// CHECK-OBJ-LP64-NEXT: R_AARCH64_LD64_GOT_LO12_NC local1{{$}}
+// CHECK-OBJ-LP64-NEXT: R_AARCH64_LD64_GOTPAGE_LO15 local2{{$}}
+// CHECK-OBJ-LP64-NEXT: R_AARCH64_ADR_GOT_PAGE local3{{$}}
+
+.data
+local0: .long 0
+local1: .long 0
+local2: .long 0
+local3: .long 0


        


More information about the llvm-commits mailing list