[patch] Fix pr24486
Rafael Espíndola via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 20 18:33:55 PDT 2015
The attached patch fixes pr24486.
It extends the work done in r233995 so that now getFragment (in
addition to getSection) also works for variable symbols.
With that the existing logic to decide if a-b can be computed works
and the expression evaluation can avoid expanding variables as
aggressively. That in turn lets the relocation code see the original
variable.
I think the testcase change to COFF is OK. Both relocations mean
exactly the same. In fact, gas produces relocations with the .text
symbol like we do for ELF. We should probably just implement that for
COFF some day.
Cheers,
Rafael
-------------- next part --------------
diff --git a/include/llvm/MC/MCExpr.h b/include/llvm/MC/MCExpr.h
index b3a6073..76da86c 100644
--- a/include/llvm/MC/MCExpr.h
+++ b/include/llvm/MC/MCExpr.h
@@ -20,6 +20,7 @@ class MCAsmLayout;
class MCAssembler;
class MCContext;
class MCFixup;
+class MCFragment;
class MCSection;
class MCStreamer;
class MCSymbol;
@@ -115,7 +116,7 @@ public:
/// currently defined as the absolute section for constants, or
/// otherwise the section associated with the first defined symbol in the
/// expression.
- MCSection *findAssociatedSection() const;
+ MCFragment *findAssociatedFragment() const;
/// @}
};
@@ -556,7 +557,7 @@ public:
const MCAsmLayout *Layout,
const MCFixup *Fixup) const = 0;
virtual void visitUsedExpr(MCStreamer& Streamer) const = 0;
- virtual MCSection *findAssociatedSection() const = 0;
+ virtual MCFragment *findAssociatedFragment() const = 0;
virtual void fixELFSymbolsInTLSFixups(MCAssembler &) const = 0;
diff --git a/include/llvm/MC/MCMachObjectWriter.h b/include/llvm/MC/MCMachObjectWriter.h
index 175d73e..c89b859 100644
--- a/include/llvm/MC/MCMachObjectWriter.h
+++ b/include/llvm/MC/MCMachObjectWriter.h
@@ -246,6 +246,11 @@ public:
const MCAsmLayout &Layout) override;
bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
+ const MCSymbol &A,
+ const MCSymbol &B,
+ bool InSet) const override;
+
+ bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
const MCSymbol &SymA,
const MCFragment &FB, bool InSet,
bool IsPCRel) const override;
diff --git a/include/llvm/MC/MCObjectWriter.h b/include/llvm/MC/MCObjectWriter.h
index 2211673..7ab912c 100644
--- a/include/llvm/MC/MCObjectWriter.h
+++ b/include/llvm/MC/MCObjectWriter.h
@@ -92,6 +92,11 @@ public:
bool InSet) const;
virtual bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
+ const MCSymbol &A,
+ const MCSymbol &B,
+ bool InSet) const;
+
+ virtual bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
const MCSymbol &SymA,
const MCFragment &FB,
bool InSet,
diff --git a/include/llvm/MC/MCSymbol.h b/include/llvm/MC/MCSymbol.h
index b2910df..e73fc77 100644
--- a/include/llvm/MC/MCSymbol.h
+++ b/include/llvm/MC/MCSymbol.h
@@ -56,19 +56,17 @@ protected:
SymContentsCommon,
};
- // Special sentinal value for the absolute pseudo section.
- //
- // FIXME: Use a PointerInt wrapper for this?
- static MCSection *AbsolutePseudoSection;
+ // Special sentinal value for the absolute pseudo fragment.
+ static MCFragment *AbsolutePseudoFragment;
/// If a symbol has a Fragment, the section is implied, so we only need
/// one pointer.
+ /// The special AbsolutePseudoFragment value is for absolute symbols.
+ /// If this is a variable symbol, this caches the variable value's fragment.
/// FIXME: We might be able to simplify this by having the asm streamer create
/// dummy fragments.
/// If this is a section, then it gives the symbol is defined in. This is null
- /// for undefined symbols, and the special AbsolutePseudoSection value for
- /// absolute symbols. If this is a variable symbol, this caches the variable
- /// value's section.
+ /// for undefined symbols.
///
/// If this is a fragment, then it gives the fragment this symbol's value is
/// relative to, if any.
@@ -180,14 +178,12 @@ private:
MCSymbol(const MCSymbol &) = delete;
void operator=(const MCSymbol &) = delete;
MCSection *getSectionPtr() const {
- if (MCFragment *F = getFragment())
+ if (MCFragment *F = getFragment()) {
+ assert(F != AbsolutePseudoFragment);
return F->getParent();
+ }
const auto &SectionOrFragment = SectionOrFragmentAndHasName.getPointer();
- assert(!SectionOrFragment.is<MCFragment *>() && "Section or null expected");
- MCSection *Section = SectionOrFragment.dyn_cast<MCSection *>();
- if (Section || !isVariable())
- return Section;
- return Section = getVariableValue()->findAssociatedSection();
+ return SectionOrFragment.dyn_cast<MCSection *>();
}
/// \brief Get a reference to the name field. Requires that we have a name
@@ -248,7 +244,7 @@ public:
/// isDefined - Check if this symbol is defined (i.e., it has an address).
///
/// Defined symbols are either absolute or in some section.
- bool isDefined() const { return getSectionPtr() != nullptr; }
+ bool isDefined() const { return isAbsolute() || getSectionPtr() != nullptr; }
/// isInSection - Check if this symbol is defined in some section (i.e., it
/// is defined but not absolute).
@@ -258,7 +254,7 @@ public:
bool isUndefined() const { return !isDefined(); }
/// isAbsolute - Check if this is an absolute symbol.
- bool isAbsolute() const { return getSectionPtr() == AbsolutePseudoSection; }
+ bool isAbsolute() const { return getFragment() == AbsolutePseudoFragment; }
/// Get the section associated with a defined, non-absolute symbol.
MCSection &getSection() const {
@@ -380,7 +376,13 @@ public:
}
MCFragment *getFragment() const {
- return SectionOrFragmentAndHasName.getPointer().dyn_cast<MCFragment *>();
+ const auto &SectionOrFragment = SectionOrFragmentAndHasName.getPointer();
+ MCFragment *Fragment = SectionOrFragment.dyn_cast<MCFragment *>();
+ if (Fragment || !isVariable())
+ return Fragment;
+ Fragment = getVariableValue()->findAssociatedFragment();
+ SectionOrFragmentAndHasName.setPointer(Fragment);
+ return Fragment;
}
void setFragment(MCFragment *Value) const {
SectionOrFragmentAndHasName.setPointer(Value);
diff --git a/lib/MC/ELFObjectWriter.cpp b/lib/MC/ELFObjectWriter.cpp
index e925bc2..3b8321e 100644
--- a/lib/MC/ELFObjectWriter.cpp
+++ b/lib/MC/ELFObjectWriter.cpp
@@ -447,7 +447,7 @@ void ELFObjectWriter::writeSymbol(SymbolTableWriter &Writer,
uint32_t StringIndex, ELFSymbolData &MSD,
const MCAsmLayout &Layout) {
const auto &Symbol = cast<MCSymbolELF>(*MSD.Symbol);
- assert((!Symbol.getFragment() ||
+ assert((!Symbol.isInSection() ||
(Symbol.getFragment()->getParent() == &Symbol.getSection())) &&
"The symbol's section doesn't match the fragment's symbol");
const MCSymbolELF *Base =
diff --git a/lib/MC/MCAssembler.cpp b/lib/MC/MCAssembler.cpp
index 09a27fe..2e712f4 100644
--- a/lib/MC/MCAssembler.cpp
+++ b/lib/MC/MCAssembler.cpp
@@ -404,7 +404,7 @@ const MCSymbol *MCAssembler::getAtom(const MCSymbol &S) const {
return &S;
// Absolute and undefined symbols have no defining atom.
- if (!S.getFragment())
+ if (!S.isInSection())
return nullptr;
// Non-linker visible symbols in sections which can't be atomized have no
diff --git a/lib/MC/MCExpr.cpp b/lib/MC/MCExpr.cpp
index a30ceec..4d76b7d 100644
--- a/lib/MC/MCExpr.cpp
+++ b/lib/MC/MCExpr.cpp
@@ -477,7 +477,8 @@ static void AttemptToFoldSymbolOffsetDifference(
if (!Asm->getWriter().isSymbolRefDifferenceFullyResolved(*Asm, A, B, InSet))
return;
- if (SA.getFragment() == SB.getFragment()) {
+ if (SA.getFragment() == SB.getFragment() && !SA.isVariable() &&
+ !SB.isVariable()) {
Addend += (SA.getOffset() - SB.getOffset());
// Pointers to Thumb symbols need to have their low-bit set to allow
@@ -616,9 +617,7 @@ static bool canExpand(const MCSymbol &Sym, const MCAssembler *Asm, bool InSet) {
if (InSet)
return true;
- if (!Asm)
- return false;
- return !Asm->getWriter().isWeak(Sym);
+ return !Sym.isInSection();
}
bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
@@ -765,45 +764,41 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
llvm_unreachable("Invalid assembly expression kind!");
}
-MCSection *MCExpr::findAssociatedSection() const {
+MCFragment *MCExpr::findAssociatedFragment() const {
switch (getKind()) {
case Target:
// We never look through target specific expressions.
- return cast<MCTargetExpr>(this)->findAssociatedSection();
+ return cast<MCTargetExpr>(this)->findAssociatedFragment();
case Constant:
- return MCSymbol::AbsolutePseudoSection;
+ return MCSymbol::AbsolutePseudoFragment;
case SymbolRef: {
const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(this);
const MCSymbol &Sym = SRE->getSymbol();
-
- if (Sym.isDefined())
- return &Sym.getSection();
-
- return nullptr;
+ return Sym.getFragment();
}
case Unary:
- return cast<MCUnaryExpr>(this)->getSubExpr()->findAssociatedSection();
+ return cast<MCUnaryExpr>(this)->getSubExpr()->findAssociatedFragment();
case Binary: {
const MCBinaryExpr *BE = cast<MCBinaryExpr>(this);
- MCSection *LHS_S = BE->getLHS()->findAssociatedSection();
- MCSection *RHS_S = BE->getRHS()->findAssociatedSection();
+ MCFragment *LHS_F = BE->getLHS()->findAssociatedFragment();
+ MCFragment *RHS_F = BE->getRHS()->findAssociatedFragment();
- // If either section is absolute, return the other.
- if (LHS_S == MCSymbol::AbsolutePseudoSection)
- return RHS_S;
- if (RHS_S == MCSymbol::AbsolutePseudoSection)
- return LHS_S;
+ // If either is absolute, return the other.
+ if (LHS_F == MCSymbol::AbsolutePseudoFragment)
+ return RHS_F;
+ if (RHS_F == MCSymbol::AbsolutePseudoFragment)
+ return LHS_F;
// Not always correct, but probably the best we can do without more context.
if (BE->getOpcode() == MCBinaryExpr::Sub)
- return MCSymbol::AbsolutePseudoSection;
+ return MCSymbol::AbsolutePseudoFragment;
// Otherwise, return the first non-null section.
- return LHS_S ? LHS_S : RHS_S;
+ return LHS_F ? LHS_F : RHS_F;
}
}
diff --git a/lib/MC/MCMachOStreamer.cpp b/lib/MC/MCMachOStreamer.cpp
index 33045d0..6643531 100644
--- a/lib/MC/MCMachOStreamer.cpp
+++ b/lib/MC/MCMachOStreamer.cpp
@@ -462,7 +462,8 @@ void MCMachOStreamer::FinishImpl() {
// defining symbols.
DenseMap<const MCFragment *, const MCSymbol *> DefiningSymbolMap;
for (const MCSymbol &Symbol : getAssembler().symbols()) {
- if (getAssembler().isSymbolLinkerVisible(Symbol) && Symbol.getFragment()) {
+ if (getAssembler().isSymbolLinkerVisible(Symbol) && Symbol.isInSection() &&
+ !Symbol.isVariable()) {
// An atom defining symbol should never be internal to a fragment.
assert(Symbol.getOffset() == 0 &&
"Invalid offset in atom defining symbol!");
diff --git a/lib/MC/MCObjectWriter.cpp b/lib/MC/MCObjectWriter.cpp
index 3479034..e84f74a 100644
--- a/lib/MC/MCObjectWriter.cpp
+++ b/lib/MC/MCObjectWriter.cpp
@@ -33,8 +33,14 @@ bool MCObjectWriter::isSymbolRefDifferenceFullyResolved(
if (!SA.getFragment() || !SB.getFragment())
return false;
- return isSymbolRefDifferenceFullyResolvedImpl(Asm, SA, *SB.getFragment(),
- InSet, false);
+ return isSymbolRefDifferenceFullyResolvedImpl(Asm, SA, SB, InSet);
+}
+
+bool MCObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(
+ const MCAssembler &Asm, const MCSymbol &A, const MCSymbol &B,
+ bool InSet) const {
+ return isSymbolRefDifferenceFullyResolvedImpl(Asm, A, *B.getFragment(), InSet,
+ false);
}
bool MCObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(
diff --git a/lib/MC/MCSymbol.cpp b/lib/MC/MCSymbol.cpp
index 125380a..d9e58ce 100644
--- a/lib/MC/MCSymbol.cpp
+++ b/lib/MC/MCSymbol.cpp
@@ -17,7 +17,8 @@
using namespace llvm;
// Sentinel value for the absolute pseudo section.
-MCSection *MCSymbol::AbsolutePseudoSection = reinterpret_cast<MCSection *>(1);
+MCFragment *MCSymbol::AbsolutePseudoFragment =
+ reinterpret_cast<MCFragment *>(4);
void *MCSymbol::operator new(size_t s, const StringMapEntry<bool> *Name,
MCContext &Ctx) {
diff --git a/lib/MC/MachObjectWriter.cpp b/lib/MC/MachObjectWriter.cpp
index 8ce6127..d09d8b1 100644
--- a/lib/MC/MachObjectWriter.cpp
+++ b/lib/MC/MachObjectWriter.cpp
@@ -628,6 +628,18 @@ void MachObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
}
bool MachObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(
+ const MCAssembler &Asm, const MCSymbol &A, const MCSymbol &B,
+ bool InSet) const {
+ // FIXME: We don't handle things like
+ // foo = .
+ // creating atoms.
+ if (A.isVariable() || B.isVariable())
+ return false;
+ return MCObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(Asm, A, B,
+ InSet);
+}
+
+bool MachObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(
const MCAssembler &Asm, const MCSymbol &SymA, const MCFragment &FB,
bool InSet, bool IsPCRel) const {
if (InSet)
diff --git a/test/MC/COFF/alias.s b/test/MC/COFF/alias.s
index 2293d43..369bbe8 100644
--- a/test/MC/COFF/alias.s
+++ b/test/MC/COFF/alias.s
@@ -21,9 +21,9 @@ weak_aliased_to_external = external2
.long weak_aliased_to_external
// CHECK: Relocations [
-// CHECK: 0x0 IMAGE_REL_I386_DIR32 local1
+// CHECK: 0x0 IMAGE_REL_I386_DIR32 external_aliased_to_local
// CHECK: 0x4 IMAGE_REL_I386_DIR32 external1
-// CHECK: 0x8 IMAGE_REL_I386_DIR32 local2
+// CHECK: 0x8 IMAGE_REL_I386_DIR32 global_aliased_to_local
// CHECK: 0xC IMAGE_REL_I386_DIR32 external2
// CHECK: ]
// CHECK: Symbols [
diff --git a/test/MC/ELF/relocation.s b/test/MC/ELF/relocation.s
index 34f1a40..0fec767 100644
--- a/test/MC/ELF/relocation.s
+++ b/test/MC/ELF/relocation.s
@@ -55,6 +55,11 @@ bar:
.quad pr23272_2 - pr23272
.quad pr23272_3 - pr23272
+ .global pr24486
+pr24486:
+ pr24486_alias = pr24486
+ .long pr24486_alias
+
.code16
call pr23771
@@ -94,6 +99,7 @@ bar:
// CHECK-NEXT: 0xD4 R_X86_64_SIZE32 blah 0xFFFFFFFFFFFFFFE0
// CHECK-NEXT: 0xD8 R_X86_64_GOTPCREL foo 0x0
// CHECK-NEXT: 0xDC R_X86_64_PLT32 foo 0x0
-// CHECK-NEXT: 0xF1 R_X86_64_PC16 pr23771 0xFFFFFFFFFFFFFFFE
+// CHECK-NEXT: 0xF0 R_X86_64_32 .text 0xF0
+// CHECK-NEXT: 0xF5 R_X86_64_PC16 pr23771 0xFFFFFFFFFFFFFFFE
// CHECK-NEXT: ]
// CHECK-NEXT: }
More information about the llvm-commits
mailing list