Skip to content

Commit e3a20f5

Browse files
espindolaespindola
committed
Fix pr24486.
This 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 even if a or b are variables. Given that, the expression evaluation can avoid expanding variables as aggressively and that in turn lets the relocation code see the original variable. In order for this to work with the asm streamer, there is now a dummy fragment per section. It is used to assign a section to a symbol when no other fragment exists. This patch is a joint work by Maxim Ostapenko andy myself. llvm-svn: 249303
1 parent ee4e08b commit e3a20f5

30 files changed

+153
-113
lines changed

‎llvm/include/llvm/MC/MCAssembler.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ class MCFragment : public ilist_node<MCFragment> {
5252
FT_Dwarf,
5353
FT_DwarfFrame,
5454
FT_LEB,
55-
FT_SafeSEH
55+
FT_SafeSEH,
56+
FT_Dummy
5657
};
5758

5859
private:
@@ -136,9 +137,19 @@ class MCFragment : public ilist_node<MCFragment> {
136137
/// and only some fragments have a meaningful implementation.
137138
void setBundlePadding(uint8_t N) { BundlePadding = N; }
138139

140+
/// \brief Return true if given frgment has FT_Dummy type.
141+
bool isDummy() const { return Kind == FT_Dummy; }
142+
139143
void dump();
140144
};
141145

146+
class MCDummyFragment : public MCFragment {
147+
public:
148+
explicit MCDummyFragment(MCSection *Sec)
149+
: MCFragment(FT_Dummy, false, 0, Sec){};
150+
static bool classof(const MCFragment *F) { return F->getKind() == FT_Dummy; }
151+
};
152+
142153
/// Interface implemented by fragments that contain encoded instructions and/or
143154
/// data.
144155
///

‎llvm/include/llvm/MC/MCExpr.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class MCAsmLayout;
2020
class MCAssembler;
2121
class MCContext;
2222
class MCFixup;
23+
class MCFragment;
2324
class MCSection;
2425
class MCStreamer;
2526
class MCSymbol;
@@ -115,7 +116,7 @@ class MCExpr {
115116
/// currently defined as the absolute section for constants, or
116117
/// otherwise the section associated with the first defined symbol in the
117118
/// expression.
118-
MCSection *findAssociatedSection() const;
119+
MCFragment *findAssociatedFragment() const;
119120

120121
/// @}
121122
};
@@ -556,7 +557,7 @@ class MCTargetExpr : public MCExpr {
556557
const MCAsmLayout *Layout,
557558
const MCFixup *Fixup) const = 0;
558559
virtual void visitUsedExpr(MCStreamer& Streamer) const = 0;
559-
virtual MCSection *findAssociatedSection() const = 0;
560+
virtual MCFragment *findAssociatedFragment() const = 0;
560561

561562
virtual void fixELFSymbolsInTLSFixups(MCAssembler &) const = 0;
562563

‎llvm/include/llvm/MC/MCMachObjectWriter.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,11 @@ class MachObjectWriter : public MCObjectWriter {
247247
void executePostLayoutBinding(MCAssembler &Asm,
248248
const MCAsmLayout &Layout) override;
249249

250+
bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
251+
const MCSymbol &A,
252+
const MCSymbol &B,
253+
bool InSet) const override;
254+
250255
bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
251256
const MCSymbol &SymA,
252257
const MCFragment &FB, bool InSet,

‎llvm/include/llvm/MC/MCObjectWriter.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ class MCObjectWriter {
9292
const MCSymbolRefExpr *B,
9393
bool InSet) const;
9494

95+
virtual bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
96+
const MCSymbol &A,
97+
const MCSymbol &B,
98+
bool InSet) const;
99+
95100
virtual bool isSymbolRefDifferenceFullyResolvedImpl(const MCAssembler &Asm,
96101
const MCSymbol &SymA,
97102
const MCFragment &FB,

‎llvm/include/llvm/MC/MCSection.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "llvm/ADT/StringRef.h"
1919
#include "llvm/ADT/ilist.h"
2020
#include "llvm/ADT/ilist_node.h"
21+
#include "llvm/MC/MCAssembler.h"
2122
#include "llvm/MC/SectionKind.h"
2223
#include "llvm/Support/Compiler.h"
2324

@@ -92,6 +93,8 @@ class MCSection {
9293

9394
unsigned IsRegistered : 1;
9495

96+
MCDummyFragment DummyFragment;
97+
9598
FragmentListType Fragments;
9699

97100
/// Mapping from subsection number to insertion point for subsection numbers
@@ -152,6 +155,9 @@ class MCSection {
152155
return const_cast<MCSection *>(this)->getFragmentList();
153156
}
154157

158+
const MCDummyFragment &getDummyFragment() const { return DummyFragment; }
159+
MCDummyFragment &getDummyFragment() { return DummyFragment; }
160+
155161
MCSection::iterator begin();
156162
MCSection::const_iterator begin() const {
157163
return const_cast<MCSection *>(this)->begin();

‎llvm/include/llvm/MC/MCStreamer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ class MCStreamer {
358358
///
359359
/// Each emitted symbol will be tracked in the ordering table,
360360
/// so we can sort on them later.
361-
void AssignSection(MCSymbol *Symbol, MCSection *Section);
361+
void AssignFragment(MCSymbol *Symbol, MCFragment *Fragment);
362362

363363
/// \brief Emit a label for \p Symbol into the current section.
364364
///

‎llvm/include/llvm/MC/MCSymbol.h

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -56,28 +56,25 @@ class MCSymbol {
5656
SymContentsCommon,
5757
};
5858

59-
// Special sentinal value for the absolute pseudo section.
60-
//
61-
// FIXME: Use a PointerInt wrapper for this?
62-
static MCSection *AbsolutePseudoSection;
59+
// Special sentinal value for the absolute pseudo fragment.
60+
static MCFragment *AbsolutePseudoFragment;
6361

6462
/// If a symbol has a Fragment, the section is implied, so we only need
6563
/// one pointer.
64+
/// The special AbsolutePseudoFragment value is for absolute symbols.
65+
/// If this is a variable symbol, this caches the variable value's fragment.
6666
/// FIXME: We might be able to simplify this by having the asm streamer create
6767
/// dummy fragments.
6868
/// If this is a section, then it gives the symbol is defined in. This is null
69-
/// for undefined symbols, and the special AbsolutePseudoSection value for
70-
/// absolute symbols. If this is a variable symbol, this caches the variable
71-
/// value's section.
69+
/// for undefined symbols.
7270
///
7371
/// If this is a fragment, then it gives the fragment this symbol's value is
7472
/// relative to, if any.
7573
///
7674
/// For the 'HasName' integer, this is true if this symbol is named.
7775
/// A named symbol will have a pointer to the name allocated in the bytes
7876
/// immediately prior to the MCSymbol.
79-
mutable PointerIntPair<PointerUnion<MCSection *, MCFragment *>, 1>
80-
SectionOrFragmentAndHasName;
77+
mutable PointerIntPair<MCFragment *, 1> FragmentAndHasName;
8178

8279
/// IsTemporary - True if this is an assembler temporary label, which
8380
/// typically does not survive in the .o file's symbol table. Usually
@@ -155,7 +152,7 @@ class MCSymbol {
155152
Kind(Kind), IsUsedInReloc(false), SymbolContents(SymContentsUnset),
156153
CommonAlignLog2(0), Flags(0) {
157154
Offset = 0;
158-
SectionOrFragmentAndHasName.setInt(!!Name);
155+
FragmentAndHasName.setInt(!!Name);
159156
if (Name)
160157
getNameEntryPtr() = Name;
161158
}
@@ -180,19 +177,16 @@ class MCSymbol {
180177
MCSymbol(const MCSymbol &) = delete;
181178
void operator=(const MCSymbol &) = delete;
182179
MCSection *getSectionPtr(bool SetUsed = true) const {
183-
if (MCFragment *F = getFragment())
180+
if (MCFragment *F = getFragment(SetUsed)) {
181+
assert(F != AbsolutePseudoFragment);
184182
return F->getParent();
185-
const auto &SectionOrFragment = SectionOrFragmentAndHasName.getPointer();
186-
assert(!SectionOrFragment.is<MCFragment *>() && "Section or null expected");
187-
MCSection *Section = SectionOrFragment.dyn_cast<MCSection *>();
188-
if (Section || !isVariable())
189-
return Section;
190-
return Section = getVariableValue(SetUsed)->findAssociatedSection();
183+
}
184+
return nullptr;
191185
}
192186

193187
/// \brief Get a reference to the name field. Requires that we have a name
194188
const StringMapEntry<bool> *&getNameEntryPtr() {
195-
assert(SectionOrFragmentAndHasName.getInt() && "Name is required");
189+
assert(FragmentAndHasName.getInt() && "Name is required");
196190
NameEntryStorageTy *Name = reinterpret_cast<NameEntryStorageTy *>(this);
197191
return (*(Name - 1)).NameEntry;
198192
}
@@ -203,7 +197,7 @@ class MCSymbol {
203197
public:
204198
/// getName - Get the symbol name.
205199
StringRef getName() const {
206-
if (!SectionOrFragmentAndHasName.getInt())
200+
if (!FragmentAndHasName.getInt())
207201
return StringRef();
208202

209203
return getNameEntryPtr()->first();
@@ -249,7 +243,7 @@ class MCSymbol {
249243
///
250244
/// Defined symbols are either absolute or in some section.
251245
bool isDefined(bool SetUsed = true) const {
252-
return getSectionPtr(SetUsed) != nullptr;
246+
return getFragment(SetUsed) != nullptr;
253247
}
254248

255249
/// isInSection - Check if this symbol is defined in some section (i.e., it
@@ -263,7 +257,7 @@ class MCSymbol {
263257

264258
/// isAbsolute - Check if this is an absolute symbol.
265259
bool isAbsolute(bool SetUsed = true) const {
266-
return getSectionPtr(SetUsed) == AbsolutePseudoSection;
260+
return getFragment(SetUsed) == AbsolutePseudoFragment;
267261
}
268262

269263
/// Get the section associated with a defined, non-absolute symbol.
@@ -272,19 +266,14 @@ class MCSymbol {
272266
return *getSectionPtr(SetUsed);
273267
}
274268

275-
/// Mark the symbol as defined in the section \p S.
276-
void setSection(MCSection &S) {
277-
assert(!isVariable() && "Cannot set section of variable");
278-
assert(!SectionOrFragmentAndHasName.getPointer().is<MCFragment *>() &&
279-
"Section or null expected");
280-
SectionOrFragmentAndHasName.setPointer(&S);
269+
/// Mark the symbol as defined in the fragment \p F.
270+
void setFragment(MCFragment *F) const {
271+
assert(!isVariable() && "Cannot set fragment of variable");
272+
FragmentAndHasName.setPointer(F);
281273
}
282274

283275
/// Mark the symbol as undefined.
284-
void setUndefined() {
285-
SectionOrFragmentAndHasName.setPointer(
286-
PointerUnion<MCSection *, MCFragment *>());
287-
}
276+
void setUndefined() { FragmentAndHasName.setPointer(nullptr); }
288277

289278
bool isELF() const { return Kind == SymbolKindELF; }
290279

@@ -385,11 +374,13 @@ class MCSymbol {
385374
return SymbolContents == SymContentsCommon;
386375
}
387376

388-
MCFragment *getFragment() const {
389-
return SectionOrFragmentAndHasName.getPointer().dyn_cast<MCFragment *>();
390-
}
391-
void setFragment(MCFragment *Value) const {
392-
SectionOrFragmentAndHasName.setPointer(Value);
377+
MCFragment *getFragment(bool SetUsed = true) const {
378+
MCFragment *Fragment = FragmentAndHasName.getPointer();
379+
if (Fragment || !isVariable())
380+
return Fragment;
381+
Fragment = getVariableValue(SetUsed)->findAssociatedFragment();
382+
FragmentAndHasName.setPointer(Fragment);
383+
return Fragment;
393384
}
394385

395386
bool isExternal() const { return IsExternal; }

‎llvm/lib/MC/ELFObjectWriter.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -447,9 +447,6 @@ void ELFObjectWriter::writeSymbol(SymbolTableWriter &Writer,
447447
uint32_t StringIndex, ELFSymbolData &MSD,
448448
const MCAsmLayout &Layout) {
449449
const auto &Symbol = cast<MCSymbolELF>(*MSD.Symbol);
450-
assert((!Symbol.getFragment() ||
451-
(Symbol.getFragment()->getParent() == &Symbol.getSection())) &&
452-
"The symbol's section doesn't match the fragment's symbol");
453450
const MCSymbolELF *Base =
454451
cast_or_null<MCSymbolELF>(Layout.getBaseSymbol(Symbol));
455452

‎llvm/lib/MC/MCAsmStreamer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ void MCAsmStreamer::EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
575575
void MCAsmStreamer::EmitZerofill(MCSection *Section, MCSymbol *Symbol,
576576
uint64_t Size, unsigned ByteAlignment) {
577577
if (Symbol)
578-
AssignSection(Symbol, Section);
578+
AssignFragment(Symbol, &Section->getDummyFragment());
579579

580580
// Note: a .zerofill directive does not switch sections.
581581
OS << ".zerofill ";
@@ -599,7 +599,7 @@ void MCAsmStreamer::EmitZerofill(MCSection *Section, MCSymbol *Symbol,
599599
// e.g. _a.
600600
void MCAsmStreamer::EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
601601
uint64_t Size, unsigned ByteAlignment) {
602-
AssignSection(Symbol, Section);
602+
AssignFragment(Symbol, &Section->getDummyFragment());
603603

604604
assert(Symbol && "Symbol shouldn't be NULL!");
605605
// Instead of using the Section we'll just use the shortcut.

‎llvm/lib/MC/MCAssembler.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ MCFragment::MCFragment(FragmentType Kind, bool HasInstructions,
277277
: Kind(Kind), HasInstructions(HasInstructions), AlignToBundleEnd(false),
278278
BundlePadding(BundlePadding), Parent(Parent), Atom(nullptr),
279279
Offset(~UINT64_C(0)) {
280-
if (Parent)
280+
if (Parent && !isDummy())
281281
Parent->getFragmentList().push_back(this);
282282
}
283283

@@ -319,6 +319,9 @@ void MCFragment::destroy() {
319319
case FT_SafeSEH:
320320
delete cast<MCSafeSEHFragment>(this);
321321
return;
322+
case FT_Dummy:
323+
delete cast<MCDummyFragment>(this);
324+
return;
322325
}
323326
}
324327

@@ -411,7 +414,7 @@ const MCSymbol *MCAssembler::getAtom(const MCSymbol &S) const {
411414
return &S;
412415

413416
// Absolute and undefined symbols have no defining atom.
414-
if (!S.getFragment())
417+
if (!S.isInSection())
415418
return nullptr;
416419

417420
// Non-linker visible symbols in sections which can't be atomized have no
@@ -547,6 +550,8 @@ uint64_t MCAssembler::computeFragmentSize(const MCAsmLayout &Layout,
547550
return cast<MCDwarfLineAddrFragment>(F).getContents().size();
548551
case MCFragment::FT_DwarfFrame:
549552
return cast<MCDwarfCallFrameFragment>(F).getContents().size();
553+
case MCFragment::FT_Dummy:
554+
llvm_unreachable("Should not have been added");
550555
}
551556

552557
llvm_unreachable("invalid fragment kind");
@@ -780,6 +785,8 @@ static void writeFragment(const MCAssembler &Asm, const MCAsmLayout &Layout,
780785
OW->writeBytes(CF.getContents());
781786
break;
782787
}
788+
case MCFragment::FT_Dummy:
789+
llvm_unreachable("Should not have been added");
783790
}
784791

785792
assert(OW->getStream().tell() - Start == FragmentSize &&
@@ -1147,6 +1154,9 @@ void MCFragment::dump() {
11471154
case MCFragment::FT_DwarfFrame: OS << "MCDwarfCallFrameFragment"; break;
11481155
case MCFragment::FT_LEB: OS << "MCLEBFragment"; break;
11491156
case MCFragment::FT_SafeSEH: OS << "MCSafeSEHFragment"; break;
1157+
case MCFragment::FT_Dummy:
1158+
OS << "MCDummyFragment";
1159+
break;
11501160
}
11511161

11521162
OS << "<MCFragment " << (void*) this << " LayoutOrder:" << LayoutOrder
@@ -1245,6 +1255,8 @@ void MCFragment::dump() {
12451255
OS << " Sym:" << F->getSymbol();
12461256
break;
12471257
}
1258+
case MCFragment::FT_Dummy:
1259+
break;
12481260
}
12491261
OS << ">";
12501262
}

‎llvm/lib/MC/MCELFStreamer.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ void MCELFStreamer::EmitLabel(MCSymbol *S) {
111111
MCObjectStreamer::EmitLabel(Symbol);
112112

113113
const MCSectionELF &Section =
114-
static_cast<const MCSectionELF&>(Symbol->getSection());
114+
static_cast<const MCSectionELF &>(*getCurrentSectionOnly());
115115
if (Section.getFlags() & ELF::SHF_TLS)
116116
Symbol->setType(ELF::STT_TLS);
117117
}
@@ -311,11 +311,6 @@ void MCELFStreamer::EmitCommonSymbol(MCSymbol *S, uint64_t Size,
311311
Symbol->setType(ELF::STT_OBJECT);
312312

313313
if (Symbol->getBinding() == ELF::STB_LOCAL) {
314-
MCSection *Section = getAssembler().getContext().getELFSection(
315-
".bss", ELF::SHT_NOBITS, ELF::SHF_WRITE | ELF::SHF_ALLOC);
316-
317-
AssignSection(Symbol, Section);
318-
319314
struct LocalCommon L = {Symbol, Size, ByteAlignment};
320315
LocalCommons.push_back(L);
321316
} else {
@@ -630,7 +625,8 @@ void MCELFStreamer::Flush() {
630625
const MCSymbol &Symbol = *i->Symbol;
631626
uint64_t Size = i->Size;
632627
unsigned ByteAlignment = i->ByteAlignment;
633-
MCSection &Section = Symbol.getSection();
628+
MCSection &Section = *getAssembler().getContext().getELFSection(
629+
".bss", ELF::SHT_NOBITS, ELF::SHF_WRITE | ELF::SHF_ALLOC);
634630

635631
getAssembler().registerSection(Section);
636632
new MCAlignFragment(ByteAlignment, 0, 1, ByteAlignment, &Section);

0 commit comments

Comments
 (0)