From 930ca9843331c2cdce2a49517f661d60bfe61204 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Sat, 3 May 2014 19:57:04 +0000 Subject: [PATCH] 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 --- include/llvm/MC/MCFixup.h | 2 - include/llvm/MC/MCValue.h | 5 ++- lib/MC/CMakeLists.txt | 1 - lib/MC/ELFObjectWriter.cpp | 23 +++++++++- lib/MC/MCExpr.cpp | 3 +- lib/MC/MCFixup.cpp | 42 ------------------- lib/MC/MCValue.cpp | 18 ++++++++ .../ARM/MCTargetDesc/ARMELFObjectWriter.cpp | 2 +- .../MCTargetDesc/PPCELFObjectWriter.cpp | 7 ++-- .../MCTargetDesc/SystemZMCObjectWriter.cpp | 2 +- .../X86/MCTargetDesc/X86ELFObjectWriter.cpp | 2 +- test/MC/ELF/relocation-386.s | 3 ++ test/MC/ELF/relocation.s | 3 +- 13 files changed, 55 insertions(+), 58 deletions(-) delete mode 100644 lib/MC/MCFixup.cpp diff --git a/include/llvm/MC/MCFixup.h b/include/llvm/MC/MCFixup.h index e6d675fba36..98a1419a193 100644 --- 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 21a17038a14..dd86979690c 100644 --- 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 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 7b69c7a9e36..6a384c1a8e1 100644 --- 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 3bce1211920..0a54627f8cd 100644 --- 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(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 60b14ad53ee..ff5009f0a20 100644 --- 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(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 index e41e67350c5..00000000000 --- 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(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(Expr); - return SRE->getKind(); - } - case MCExpr::Binary: { - const MCBinaryExpr *ABE = cast(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 21d9e2102e2..9dfc56ef23d 100644 --- 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 a6f7258e651..1c842633f43 100644 --- 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 d19f6a0214d..cd3b4f45359 100644 --- 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(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 54c6987ca2f..c6a18165889 100644 --- 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 c4a1af86157..3fdec874cba 100644 --- 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 4ddfd00b120..ba12df0d3b6 100644 --- 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 318d60fdc82..c0e6007dc4e 100644 --- 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