Fix pr19645.

The fix itself is fairly simple: move getAccessVariant to MCValue so that we
replace the old weak expression evaluation with the far more general
EvaluateAsRelocatable.

This then requires that EvaluateAsRelocatable stop when it finds a non
trivial reference kind. And that in turn requires the ELF writer to look
harder for weak references.

Last but not least, this found a case where we were being bug by bug
compatible with gas and accepting an invalid input. I reported pr19647
to track it.

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

diff --git a/include/llvm/MC/MCFixup.h b/include/llvm/MC/MCFixup.h
index e6d675f..98a1419 100644 (file)
--- a/include/llvm/MC/MCFixup.h
+++ b/include/llvm/MC/MCFixup.h
@@ -88,8 +88,6 @@ public:
 
   MCFixupKind getKind() const { return MCFixupKind(Kind); }
 
-  MCSymbolRefExpr::VariantKind getAccessVariant() const;
-
   uint32_t getOffset() const { return Offset; }
   void setOffset(uint32_t Value) { Offset = Value; }
 

diff --git a/include/llvm/MC/MCValue.h b/include/llvm/MC/MCValue.h
index 21a1703..dd86979 100644 (file)
--- a/include/llvm/MC/MCValue.h
+++ b/include/llvm/MC/MCValue.h
@@ -14,14 +14,13 @@
 #ifndef LLVM_MC_MCVALUE_H
 #define LLVM_MC_MCVALUE_H
 
+#include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/DataTypes.h"
 #include <cassert>
 
 namespace llvm {
 class MCAsmInfo;
-class MCSymbol;
-class MCSymbolRefExpr;
 class raw_ostream;
 
 /// MCValue - This represents an "assembler immediate".  In its most
@@ -61,6 +60,8 @@ public:
   /// dump - Print the value to stderr.
   void dump() const;
 
+  MCSymbolRefExpr::VariantKind getAccessVariant() const;
+
   static MCValue get(const MCSymbolRefExpr *SymA,
                      const MCSymbolRefExpr *SymB = nullptr,
                      int64_t Val = 0, uint32_t RefKind = 0) {

diff --git a/lib/MC/CMakeLists.txt b/lib/MC/CMakeLists.txt
index 7b69c7a..6a384c1 100644 (file)
--- a/lib/MC/CMakeLists.txt
+++ b/lib/MC/CMakeLists.txt
@@ -16,7 +16,6 @@ add_llvm_library(LLVMMC
   MCELF.cpp
   MCELFObjectTargetWriter.cpp
   MCELFStreamer.cpp
-  MCFixup.cpp
   MCFunction.cpp
   MCExpr.cpp
   MCExternalSymbolizer.cpp

diff --git a/lib/MC/ELFObjectWriter.cpp b/lib/MC/ELFObjectWriter.cpp
index 3bce121..0a54627 100644 (file)
--- a/lib/MC/ELFObjectWriter.cpp
+++ b/lib/MC/ELFObjectWriter.cpp
@@ -787,6 +787,25 @@ bool ELFObjectWriter::shouldRelocateWithSymbol(const MCAssembler &Asm,
   return false;
 }
 
+static const MCSymbol *getWeakRef(const MCSymbolRefExpr &Ref) {
+  const MCSymbol &Sym = Ref.getSymbol();
+
+  if (Ref.getKind() == MCSymbolRefExpr::VK_WEAKREF)
+    return &Sym;
+
+  if (!Sym.isVariable())
+    return nullptr;
+
+  const MCExpr *Expr = Sym.getVariableValue();
+  const auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr);
+  if (!Inner)
+    return nullptr;
+
+  if (Inner->getKind() == MCSymbolRefExpr::VK_WEAKREF)
+    return &Inner->getSymbol();
+  return nullptr;
+}
+
 void ELFObjectWriter::RecordRelocation(const MCAssembler &Asm,
                                        const MCAsmLayout &Layout,
                                        const MCFragment *Fragment,
@@ -872,8 +891,8 @@ void ELFObjectWriter::RecordRelocation(const MCAssembler &Asm,
     if (const MCSymbol *R = Renames.lookup(SymA))
       SymA = R;
 
-    if (RefA->getKind() == MCSymbolRefExpr::VK_WEAKREF)
-      WeakrefUsedInReloc.insert(SymA);
+    if (const MCSymbol *WeakRef = getWeakRef(*RefA))
+      WeakrefUsedInReloc.insert(WeakRef);
     else
       UsedInReloc.insert(SymA);
   }

diff --git a/lib/MC/MCExpr.cpp b/lib/MC/MCExpr.cpp
index 60b14ad..ff5009f 100644 (file)
--- a/lib/MC/MCExpr.cpp
+++ b/lib/MC/MCExpr.cpp
@@ -658,12 +658,11 @@ bool MCExpr::EvaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
 
   case SymbolRef: {
     const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(this);
-    bool IsWeakRef = SRE->getKind() == MCSymbolRefExpr::VK_WEAKREF;
     const MCSymbol &Sym = SRE->getSymbol();
     const MCAsmInfo &MCAsmInfo = SRE->getMCAsmInfo();
 
     // Evaluate recursively if this is a variable.
-    if (Sym.isVariable() && !IsWeakRef) {
+    if (Sym.isVariable() && SRE->getKind() == MCSymbolRefExpr::VK_None) {
       if (Sym.getVariableValue()->EvaluateAsRelocatableImpl(
               Res, Asm, Layout, Addrs, true, ForceVarExpansion)) {
         const MCSymbolRefExpr *A = Res.getSymA();

diff --git a/lib/MC/MCFixup.cpp b/lib/MC/MCFixup.cpp
deleted file mode 100644 (file)
index e41e673..0000000
--- a/lib/MC/MCFixup.cpp
+++ /dev/null
@@ -1,42 +0,0 @@
-//===- 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: {
-    assert(getAccessVariant(cast<MCUnaryExpr>(Expr)->getSubExpr()) ==
-           MCSymbolRefExpr::VK_None);
-    return MCSymbolRefExpr::VK_None;
-  }
-
-  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());
-  }
-  }
-  llvm_unreachable("unknown MCExpr kind");
-}
-
-MCSymbolRefExpr::VariantKind MCFixup::getAccessVariant() const {
-  return ::getAccessVariant(getValue());
-}

diff --git a/lib/MC/MCValue.cpp b/lib/MC/MCValue.cpp
index 21d9e21..9dfc56e 100644 (file)
--- a/lib/MC/MCValue.cpp
+++ b/lib/MC/MCValue.cpp
@@ -10,6 +10,7 @@
 #include "llvm/MC/MCValue.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
@@ -41,3 +42,20 @@ void MCValue::dump() const {
   print(dbgs(), nullptr);
 }
 #endif
+
+MCSymbolRefExpr::VariantKind MCValue::getAccessVariant() const {
+  const MCSymbolRefExpr *B = getSymB();
+  if (B) {
+    if (B->getKind() != MCSymbolRefExpr::VK_None)
+      llvm_unreachable("unsupported");
+  }
+
+  const MCSymbolRefExpr *A = getSymA();
+  if (!A)
+    return MCSymbolRefExpr::VK_None;
+
+  MCSymbolRefExpr::VariantKind Kind = A->getKind();
+  if (Kind == MCSymbolRefExpr::VK_WEAKREF)
+    return MCSymbolRefExpr::VK_None;
+  return Kind;
+}

diff --git a/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp b/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
index a6f7258..1c84263 100644 (file)
--- a/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
+++ b/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
@@ -74,7 +74,7 @@ unsigned ARMELFObjectWriter::GetRelocType(const MCValue &Target,
 unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target,
                                                const MCFixup &Fixup,
                                                bool IsPCRel) const  {
-  MCSymbolRefExpr::VariantKind Modifier = Fixup.getAccessVariant();
+  MCSymbolRefExpr::VariantKind Modifier = Target.getAccessVariant();
 
   unsigned Type = 0;
   if (IsPCRel) {

diff --git a/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp b/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
index d19f6a0..cd3b4f4 100644 (file)
--- a/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
+++ b/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
@@ -41,11 +41,12 @@ PPCELFObjectWriter::PPCELFObjectWriter(bool Is64Bit, uint8_t OSABI)
 PPCELFObjectWriter::~PPCELFObjectWriter() {
 }
 
-static MCSymbolRefExpr::VariantKind getAccessVariant(const MCFixup &Fixup) {
+static MCSymbolRefExpr::VariantKind getAccessVariant(const MCValue &Target,
+                                                     const MCFixup &Fixup) {
   const MCExpr *Expr = Fixup.getValue();
 
   if (Expr->getKind() != MCExpr::Target)
-    return Fixup.getAccessVariant();
+    return Target.getAccessVariant();
 
   switch (cast<PPCMCExpr>(Expr)->getKind()) {
   case PPCMCExpr::VK_PPC_None:
@@ -72,7 +73,7 @@ unsigned PPCELFObjectWriter::getRelocTypeInner(const MCValue &Target,
                                                const MCFixup &Fixup,
                                                bool IsPCRel) const
 {
-  MCSymbolRefExpr::VariantKind Modifier = getAccessVariant(Fixup);
+  MCSymbolRefExpr::VariantKind Modifier = getAccessVariant(Target, Fixup);
 
   // determine the type of the relocation
   unsigned Type;

diff --git a/lib/Target/SystemZ/MCTargetDesc/SystemZMCObjectWriter.cpp b/lib/Target/SystemZ/MCTargetDesc/SystemZMCObjectWriter.cpp
index 54c6987..c6a1816 100644 (file)
--- a/lib/Target/SystemZ/MCTargetDesc/SystemZMCObjectWriter.cpp
+++ b/lib/Target/SystemZ/MCTargetDesc/SystemZMCObjectWriter.cpp
@@ -82,7 +82,7 @@ static unsigned getPLTReloc(unsigned Kind) {
 unsigned SystemZObjectWriter::GetRelocType(const MCValue &Target,
                                            const MCFixup &Fixup,
                                            bool IsPCRel) const {
-  MCSymbolRefExpr::VariantKind Modifier = Fixup.getAccessVariant();
+  MCSymbolRefExpr::VariantKind Modifier = Target.getAccessVariant();
   unsigned Kind = Fixup.getKind();
   switch (Modifier) {
   case MCSymbolRefExpr::VK_None:

diff --git a/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp b/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp
index c4a1af8..3fdec87 100644 (file)
--- a/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp
+++ b/lib/Target/X86/MCTargetDesc/X86ELFObjectWriter.cpp
@@ -43,7 +43,7 @@ unsigned X86ELFObjectWriter::GetRelocType(const MCValue &Target,
                                           bool IsPCRel) const {
   // determine the type of the relocation
 
-  MCSymbolRefExpr::VariantKind Modifier = Fixup.getAccessVariant();
+  MCSymbolRefExpr::VariantKind Modifier = Target.getAccessVariant();
   unsigned Type;
   if (getEMachine() == ELF::EM_X86_64) {
     if (IsPCRel) {

diff --git a/test/MC/ELF/relocation-386.s b/test/MC/ELF/relocation-386.s
index 4ddfd00..ba12df0 100644 (file)
--- a/test/MC/ELF/relocation-386.s
+++ b/test/MC/ELF/relocation-386.s
@@ -62,6 +62,7 @@
 // CHECK-NEXT:     0x9E         R_386_PC16       und_symbol 0x0
 // Relocation 28 (und_symbol-bar2) is of type R_386_PC8
 // CHECK-NEXT:     0xA0         R_386_PC8        und_symbol 0x0
+// CHECK-NEXT:     0xA3         R_386_GOTOFF     und_symbol 0x0
 // CHECK-NEXT:   }
 // CHECK-NEXT: ]
 
@@ -127,6 +128,8 @@ bar2:
         .word und_symbol-bar2
         .byte und_symbol-bar2
 
+        leal 1 + und_symbol@GOTOFF, %edi
+
         .section        zedsec,"awT",@progbits
 zed:
         .long 0

diff --git a/test/MC/ELF/relocation.s b/test/MC/ELF/relocation.s
index 318d60f..c0e6007 100644 (file)
--- a/test/MC/ELF/relocation.s
+++ b/test/MC/ELF/relocation.s
@@ -25,6 +25,7 @@ bar:
        .word   foo-bar
        .byte   foo-bar
 
+        # this should probably be an error...
        zed = foo +2
        call zed@PLT
 
@@ -57,7 +58,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:       0x91 R_X86_64_PLT32 zed 0xFFFFFFFFFFFFFFFC
 // CHECK-NEXT:       0x98 R_X86_64_PC32 foo 0xFFFFFFFFFFFFFFFB
 // CHECK-NEXT:       0x9D R_X86_64_GOTPC32 _GLOBAL_OFFSET_TABLE_ 0x1
 // CHECK-NEXT:       0xA3 R_X86_64_GOTPC64 _GLOBAL_OFFSET_TABLE_ 0x2