Look through variables when computing relocations.
authorRafael Espindola <rafael.espindola@gmail.com>
Thu, 20 Mar 2014 02:12:01 +0000 (02:12 +0000)
committerRafael Espindola <rafael.espindola@gmail.com>
Thu, 20 Mar 2014 02:12:01 +0000 (02:12 +0000)
Given

bar = foo + 4
.long bar

MC would eat the 4. GNU as includes it in the relocation. The rule seems to be
that a variable that defines a symbol is used in the relocation and one that
does not define a symbol is evaluated and the result included in the relocation.

Fixing this unfortunately required some other changes:

* Since the variable is now evaluated, it would prevent the ELF writer from
  noticing the weakref marker the elf streamer uses. This patch then replaces
  that with a VariantKind in MCSymbolRefExpr.

* Using VariantKind then requires us to look past other VariantKind to see

.weakref bar,foo
call bar@PLT

  doing this also fixes

zed = foo +2
call zed@PLT

  so that is a good thing.

* Looking past VariantKind means that the relocation selection has to use
  the fixup instead of the target.

This is a reboot of the previous fixes for MC. I will watch the sanitizer
buildbot and wait for a build before adding back the previous fixes.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@204294 91177308-0d34-0410-b5e6-96231b3b80d8

14 files changed:
include/llvm/MC/MCELFSymbolFlags.h
include/llvm/MC/MCExpr.h
include/llvm/MC/MCFixup.h
lib/MC/CMakeLists.txt
lib/MC/ELFObjectWriter.cpp
lib/MC/MCELFStreamer.cpp
lib/MC/MCExpr.cpp
lib/MC/MCFixup.cpp [new file with mode: 0644]
lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
lib/Target/SystemZ/MCTargetDesc/SystemZMCObjectWriter.cpp
lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp
test/MC/ELF/relocation.s
test/MC/X86/reloc-undef-global.s [new file with mode: 0644]

index 782e7d612677b9d8e721993be0d13134a985c898..5b82a58f40074dd8589448e6bfe7f0c6245edcd4 100644 (file)
@@ -51,8 +51,7 @@ namespace llvm {
       ELF_STV_Hidden    = (ELF::STV_HIDDEN    << ELF_STV_Shift),
       ELF_STV_Protected = (ELF::STV_PROTECTED << ELF_STV_Shift),
 
-      ELF_Other_Weakref = (1                  << ELF_Other_Shift),
-      ELF_Other_ThumbFunc = (2                << ELF_Other_Shift)
+      ELF_Other_ThumbFunc = (1                << ELF_Other_Shift)
   };
 
 } // end namespace llvm
index a822513ffd899e73ce88ac10c3651b4dd73d9dcd..b1d68ecb9f3665c87988c5c9d156b3abac6c63bb 100644 (file)
@@ -160,6 +160,7 @@ public:
     VK_DTPOFF,
     VK_TLVP,      // Mach-O thread local variable relocation
     VK_SECREL,
+    VK_WEAKREF,   // The link between the symbols in .weakref foo, bar
 
     VK_ARM_NONE,
     VK_ARM_TARGET1,
index 16e9eb730b4e88322ea906226a833c0520830957..e6d675fba3676b661ad27fe58c90bf1a6e089eb1 100644 (file)
@@ -10,6 +10,7 @@
 #ifndef LLVM_MC_MCFIXUP_H
 #define LLVM_MC_MCFIXUP_H
 
+#include "llvm/MC/MCExpr.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SMLoc.h"
@@ -87,6 +88,8 @@ public:
 
   MCFixupKind getKind() const { return MCFixupKind(Kind); }
 
+  MCSymbolRefExpr::VariantKind getAccessVariant() const;
+
   uint32_t getOffset() const { return Offset; }
   void setOffset(uint32_t Value) { Offset = Value; }
 
index 06099640a1a35addf46307321cc7d1d49522ffbb..eb3f9e52d73b358115e2735e5930fa3d108af3cb 100644 (file)
@@ -16,6 +16,7 @@ add_llvm_library(LLVMMC
   MCELF.cpp
   MCELFObjectTargetWriter.cpp
   MCELFStreamer.cpp
+  MCFixup.cpp
   MCFunction.cpp
   MCExpr.cpp
   MCExternalSymbolizer.cpp
index 909ea800830978bf1742071d971f1979bf841e5a..1dae2f467d3026a88073807758af46b822ba86c1 100644 (file)
@@ -778,7 +778,7 @@ void ELFObjectWriter::RecordRelocation(const MCAssembler &Asm,
         Index = 0;
       }
     } else {
-      if (Asm.getSymbolData(Symbol).getFlags() & ELF_Other_Weakref)
+      if (Target.getSymA()->getKind() == MCSymbolRefExpr::VK_WEAKREF)
         WeakrefUsedInReloc.insert(RelocSymbol);
       else
         UsedInReloc.insert(RelocSymbol);
@@ -823,8 +823,14 @@ ELFObjectWriter::getSymbolIndexInSymbolTable(const MCAssembler &Asm,
 bool ELFObjectWriter::isInSymtab(const MCAssembler &Asm,
                                  const MCSymbolData &Data,
                                  bool Used, bool Renamed) {
-  if (Data.getFlags() & ELF_Other_Weakref)
-    return false;
+  const MCSymbol &Symbol = Data.getSymbol();
+  if (Symbol.isVariable()) {
+    const MCExpr *Expr = Symbol.getVariableValue();
+    if (const MCSymbolRefExpr *Ref = dyn_cast<MCSymbolRefExpr>(Expr)) {
+      if (Ref->getKind() == MCSymbolRefExpr::VK_WEAKREF)
+        return false;
+    }
+  }
 
   if (Used)
     return true;
@@ -832,8 +838,6 @@ bool ELFObjectWriter::isInSymtab(const MCAssembler &Asm,
   if (Renamed)
     return false;
 
-  const MCSymbol &Symbol = Data.getSymbol();
-
   if (Symbol.getName() == "_GLOBAL_OFFSET_TABLE_")
     return true;
 
index 5f6a889769dffba60df45ad95349a9b0d5b17bb6..d07ef9e698ec1f4cf44ee7a97d247be89015e66a 100644 (file)
@@ -99,9 +99,8 @@ void MCELFStreamer::ChangeSection(const MCSection *Section,
 
 void MCELFStreamer::EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {
   getAssembler().getOrCreateSymbolData(*Symbol);
-  MCSymbolData &AliasSD = getAssembler().getOrCreateSymbolData(*Alias);
-  AliasSD.setFlags(AliasSD.getFlags() | ELF_Other_Weakref);
-  const MCExpr *Value = MCSymbolRefExpr::Create(Symbol, getContext());
+  const MCExpr *Value = MCSymbolRefExpr::Create(
+      Symbol, MCSymbolRefExpr::VK_WEAKREF, getContext());
   Alias->setVariableValue(Value);
 }
 
index 9a4c41699e104a4e57440fd3cd301c47d87c1364..673913f0b826c142750c8d241b924fc3bd78cd31 100644 (file)
@@ -180,6 +180,7 @@ StringRef MCSymbolRefExpr::getVariantKindName(VariantKind Kind) {
   case VK_DTPOFF: return "DTPOFF";
   case VK_TLVP: return "TLVP";
   case VK_SECREL: return "SECREL32";
+  case VK_WEAKREF: return "WEAKREF";
   case VK_ARM_NONE: return "none";
   case VK_ARM_TARGET1: return "target1";
   case VK_ARM_TARGET2: return "target2";
@@ -630,17 +631,31 @@ bool MCExpr::EvaluateAsRelocatableImpl(MCValue &Res,
   case SymbolRef: {
     const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(this);
     const MCSymbol &Sym = SRE->getSymbol();
+    const MCAsmInfo &MCAsmInfo = SRE->getMCAsmInfo();
 
     // Evaluate recursively if this is a variable.
-    if (Sym.isVariable() && SRE->getKind() == MCSymbolRefExpr::VK_None) {
-      bool Ret = Sym.getVariableValue()->EvaluateAsRelocatableImpl(Res, Asm,
-                                                                   Layout,
-                                                                   Addrs,
-                                                                   true);
-      // If we failed to simplify this to a constant, let the target
-      // handle it.
-      if (Ret && !Res.getSymA() && !Res.getSymB())
-        return true;
+    if (Sym.isVariable()) {
+      if (Sym.getVariableValue()->EvaluateAsRelocatableImpl(Res, Asm, Layout,
+                                                            Addrs, true)) {
+        const MCSymbolRefExpr *A = Res.getSymA();
+        const MCSymbolRefExpr *B = Res.getSymB();
+
+        if (MCAsmInfo.hasSubsectionsViaSymbols()) {
+          // FIXME: This is small hack. Given
+          // a = b + 4
+          // .long a
+          // the OS X assembler will completely drop the 4. We should probably
+          // include it in the relocation or produce an error if that is not
+          // possible.
+          if (!A && !B)
+            return true;
+        } else {
+          bool IsSymbol = A && A->getSymbol().isDefined();
+          bool IsWeakRef = SRE->getKind() == MCSymbolRefExpr::VK_WEAKREF;
+          if (!IsSymbol && !IsWeakRef)
+            return true;
+        }
+      }
     }
 
     Res = MCValue::get(SRE, 0, 0);
diff --git a/lib/MC/MCFixup.cpp b/lib/MC/MCFixup.cpp
new file mode 100644 (file)
index 0000000..d0a4fed
--- /dev/null
@@ -0,0 +1,36 @@
+//===- MCFixup.cpp - Assembly Fixup Implementation ------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/MC/MCFixup.h"
+using namespace llvm;
+
+static MCSymbolRefExpr::VariantKind getAccessVariant(const MCExpr *Expr) {
+  switch (Expr->getKind()) {
+  case MCExpr::Unary:
+  case MCExpr::Target:
+    llvm_unreachable("unsupported");
+
+  case MCExpr::Constant:
+    return MCSymbolRefExpr::VK_None;
+
+  case MCExpr::SymbolRef: {
+    const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(Expr);
+    return SRE->getKind();
+  }
+  case MCExpr::Binary: {
+    const MCBinaryExpr *ABE = cast<MCBinaryExpr>(Expr);
+    assert(getAccessVariant(ABE->getRHS()) == MCSymbolRefExpr::VK_None);
+    return getAccessVariant(ABE->getLHS());
+  }
+  }
+}
+
+MCSymbolRefExpr::VariantKind MCFixup::getAccessVariant() const {
+  return ::getAccessVariant(getValue());
+}
index 25efd67c69b80aae2cd8bf22916c4cba5770556a..d829394e09e7a628164d845719a963adbf5c2d42 100644 (file)
@@ -71,17 +71,16 @@ const MCSymbol *ARMELFObjectWriter::ExplicitRelSym(const MCAssembler &Asm,
   bool InNormalSection = true;
   unsigned RelocType = 0;
   RelocType = GetRelocTypeInner(Target, Fixup, IsPCRel);
+  assert(!Target.getSymB() ||
+         Target.getSymB()->getKind() == MCSymbolRefExpr::VK_None);
 
   DEBUG(
-      const MCSymbolRefExpr::VariantKind Kind = Target.getSymA()->getKind();
-      MCSymbolRefExpr::VariantKind Kind2;
-      Kind2 = Target.getSymB() ?  Target.getSymB()->getKind() :
-        MCSymbolRefExpr::VK_None;
+      MCSymbolRefExpr::VariantKind Kind = Fixup.getAccessVariant();
       dbgs() << "considering symbol "
         << Section.getSectionName() << "/"
         << Symbol.getName() << "/"
         << " Rel:" << (unsigned)RelocType
-        << " Kind: " << (int)Kind << "/" << (int)Kind2
+        << " Kind: " << (int)Kind
         << " Tmp:"
         << Symbol.isAbsolute() << "/" << Symbol.isDefined() << "/"
         << Symbol.isVariable() << "/" << Symbol.isTemporary()
@@ -152,8 +151,7 @@ unsigned ARMELFObjectWriter::GetRelocType(const MCValue &Target,
 unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target,
                                                const MCFixup &Fixup,
                                                bool IsPCRel) const  {
-  MCSymbolRefExpr::VariantKind Modifier = Target.isAbsolute() ?
-    MCSymbolRefExpr::VK_None : Target.getSymA()->getKind();
+  MCSymbolRefExpr::VariantKind Modifier = Fixup.getAccessVariant();
 
   unsigned Type = 0;
   if (IsPCRel) {
index 54de70eff71a75a0117c9b06543f5ac7302923c5..1e3d4b71aefc708fbb0e20fc171653fcd17d6ae7 100644 (file)
@@ -9,6 +9,7 @@
 
 #include "MCTargetDesc/PPCMCTargetDesc.h"
 #include "MCTargetDesc/PPCFixupKinds.h"
+#include "MCTargetDesc/PPCMCExpr.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/MC/MCELFObjectWriter.h"
 #include "llvm/MC/MCExpr.h"
@@ -49,12 +50,37 @@ PPCELFObjectWriter::PPCELFObjectWriter(bool Is64Bit, uint8_t OSABI)
 PPCELFObjectWriter::~PPCELFObjectWriter() {
 }
 
+static MCSymbolRefExpr::VariantKind getAccessVariant(const MCFixup &Fixup) {
+  const MCExpr *Expr = Fixup.getValue();
+
+  if (Expr->getKind() != MCExpr::Target)
+    return Fixup.getAccessVariant();
+
+  switch (cast<PPCMCExpr>(Expr)->getKind()) {
+  case PPCMCExpr::VK_PPC_None:
+    return MCSymbolRefExpr::VK_None;
+  case PPCMCExpr::VK_PPC_LO:
+    return MCSymbolRefExpr::VK_PPC_LO;
+  case PPCMCExpr::VK_PPC_HI:
+    return MCSymbolRefExpr::VK_PPC_HI;
+  case PPCMCExpr::VK_PPC_HA:
+    return MCSymbolRefExpr::VK_PPC_HA;
+  case PPCMCExpr::VK_PPC_HIGHERA:
+    return MCSymbolRefExpr::VK_PPC_HIGHERA;
+  case PPCMCExpr::VK_PPC_HIGHER:
+    return MCSymbolRefExpr::VK_PPC_HIGHER;
+  case PPCMCExpr::VK_PPC_HIGHEST:
+    return MCSymbolRefExpr::VK_PPC_HIGHEST;
+  case PPCMCExpr::VK_PPC_HIGHESTA:
+    return MCSymbolRefExpr::VK_PPC_HIGHESTA;
+  }
+}
+
 unsigned PPCELFObjectWriter::getRelocTypeInner(const MCValue &Target,
                                                const MCFixup &Fixup,
                                                bool IsPCRel) const
 {
-  MCSymbolRefExpr::VariantKind Modifier = Target.isAbsolute() ?
-    MCSymbolRefExpr::VK_None : Target.getSymA()->getKind();
+  MCSymbolRefExpr::VariantKind Modifier = getAccessVariant(Fixup);
 
   // determine the type of the relocation
   unsigned Type;
@@ -368,8 +394,7 @@ const MCSymbol *PPCELFObjectWriter::ExplicitRelSym(const MCAssembler &Asm,
                                                    const MCFixup &Fixup,
                                                    bool IsPCRel) const {
   assert(Target.getSymA() && "SymA cannot be 0");
-  MCSymbolRefExpr::VariantKind Modifier = Target.isAbsolute() ?
-    MCSymbolRefExpr::VK_None : Target.getSymA()->getKind();
+  MCSymbolRefExpr::VariantKind Modifier = Fixup.getAccessVariant();
 
   bool EmitThisSym;
   switch (Modifier) {
index 217a51cea49db520753e900fe7531c7a28105398..ddc1379e3401974e354675ddaaf66b2737eca3f3 100644 (file)
@@ -88,9 +88,7 @@ unsigned SystemZObjectWriter::GetRelocType(const MCValue &Target,
                                            bool IsPCRel,
                                            bool IsRelocWithSymbol,
                                            int64_t Addend) const {
-  MCSymbolRefExpr::VariantKind Modifier = (Target.isAbsolute() ?
-                                           MCSymbolRefExpr::VK_None :
-                                           Target.getSymA()->getKind());
+  MCSymbolRefExpr::VariantKind Modifier = Fixup.getAccessVariant();
   unsigned Kind = Fixup.getKind();
   switch (Modifier) {
   case MCSymbolRefExpr::VK_None:
index 7118a6f6dc527752afd38cabf5b4f80bbd8aa9ab..618b0e6e1ed63723466728a5abf7dee6f21489ab 100644 (file)
@@ -46,8 +46,7 @@ unsigned X86ELFObjectWriter::GetRelocType(const MCValue &Target,
                                           int64_t Addend) const {
   // determine the type of the relocation
 
-  MCSymbolRefExpr::VariantKind Modifier = Target.isAbsolute() ?
-    MCSymbolRefExpr::VK_None : Target.getSymA()->getKind();
+  MCSymbolRefExpr::VariantKind Modifier = Fixup.getAccessVariant();
   unsigned Type;
   if (getEMachine() == ELF::EM_X86_64) {
     if (IsPCRel) {
index 6bac5913b6ee3ecc3aff129703448b4f4027e392..d2ee6afda36c38e51c19478ddd85920e7c10c5d9 100644 (file)
@@ -25,6 +25,9 @@ bar:
        .word   foo-bar
        .byte   foo-bar
 
+       zed = foo +2
+       call zed@PLT
+
 // CHECK:        Section {
 // CHECK:          Name: .rela.text
 // CHECK:          Relocations [
@@ -49,6 +52,7 @@ bar:
 // CHECK-NEXT:       0x85 R_X86_64_TPOFF64 baz 0x0
 // CHECK-NEXT:       0x8D R_X86_64_PC16 foo 0x8D
 // CHECK-NEXT:       0x8F R_X86_64_PC8 foo 0x8F
+// CHECK-NEXT:       0x91 R_X86_64_PLT32 foo 0xFFFFFFFFFFFFFFFE
 // CHECK-NEXT:     ]
 // CHECK-NEXT:   }
 
diff --git a/test/MC/X86/reloc-undef-global.s b/test/MC/X86/reloc-undef-global.s
new file mode 100644 (file)
index 0000000..a4854d4
--- /dev/null
@@ -0,0 +1,20 @@
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o - | llvm-readobj -r | FileCheck --check-prefix=ELF %s
+// RUN: llvm-mc -filetype=obj -triple x86_64-apple-darwin %s -o - | llvm-readobj -r | FileCheck --check-prefix=MACHO %s
+
+
+bar = foo + 4
+       .globl bar
+       .long bar
+
+// ELF:      Relocations [
+// ELF-NEXT:   Section (2) .rela.text {
+// ELF-NEXT:     0x0 R_X86_64_32 foo 0x4
+// ELF-NEXT:   }
+// ELF-NEXT: ]
+
+
+// MACHO: Relocations [
+// MACHO:   Section __text {
+// MACHO:     0x0 0 2 1 X86_64_RELOC_UNSIGNED 0 bar
+// MACHO:   }
+// MACHO: ]