Skip to content

Commit af456df

Browse files
authored
[BOLT] Refactor tracking internals of BinaryFunction. NFCI (#167074)
In addition to tracking offsets inside a `BinaryFunction` that are referenced by data relocations, we need to track those relocations too. Plus, we will need to map symbols referenced by such relocations back to the containing function. This change introduces `BinaryFunction::InternalRefDataRelocations` to track the aforementioned relocations and expands `BinaryContext::SymbolToFunctionMap` to include local/temp symbols involved in relocation processing. There is no functional change introduced that should affect the output. Future PRs will use the new tracking capabilities.
1 parent 7734276 commit af456df

File tree

4 files changed

+44
-3
lines changed

4 files changed

+44
-3
lines changed

bolt/include/bolt/Core/BinaryContext.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ class BinaryContext {
497497
///
498498
/// As we fold identical functions, multiple symbols can point
499499
/// to the same BinaryFunction.
500-
std::unordered_map<const MCSymbol *, BinaryFunction *> SymbolToFunctionMap;
500+
DenseMap<const MCSymbol *, BinaryFunction *> SymbolToFunctionMap;
501501

502502
/// A mutex that is used to control parallel accesses to SymbolToFunctionMap
503503
mutable llvm::sys::RWMutex SymbolToFunctionMapMutex;

bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,14 @@ class BinaryFunction {
281281
/// goto labels.
282282
std::set<uint64_t> ExternallyReferencedOffsets;
283283

284+
/// Relocations from data sections targeting internals of this function, i.e.
285+
/// some code not at an entry point. These include, but are not limited to,
286+
/// jump table relocations and computed goto tables.
287+
///
288+
/// Since relocations can be removed/deallocated, we store relocation offsets
289+
/// instead of pointers.
290+
DenseSet<uint64_t> InternalRefDataRelocations;
291+
284292
/// Offsets of indirect branches with unknown destinations.
285293
std::set<uint64_t> UnknownIndirectBranchOffsets;
286294

@@ -640,6 +648,20 @@ class BinaryFunction {
640648
Islands->CodeOffsets.emplace(Offset);
641649
}
642650

651+
/// Register a relocation from data section referencing code at a non-zero
652+
/// offset in this function.
653+
void registerInternalRefDataRelocation(uint64_t FuncOffset,
654+
uint64_t RelOffset) {
655+
assert(FuncOffset != 0 && "Relocation should reference function internals");
656+
registerReferencedOffset(FuncOffset);
657+
InternalRefDataRelocations.insert(RelOffset);
658+
const MCSymbol *ReferencedSymbol =
659+
getOrCreateLocalLabel(getAddress() + FuncOffset);
660+
661+
// Track the symbol mapping since it's used in relocation handling.
662+
BC.setSymbolToFunctionMap(ReferencedSymbol, this);
663+
}
664+
643665
/// Register an internal offset in a function referenced from outside.
644666
void registerReferencedOffset(uint64_t Offset) {
645667
ExternallyReferencedOffsets.emplace(Offset);
@@ -1299,6 +1321,12 @@ class BinaryFunction {
12991321
void addRelocation(uint64_t Address, MCSymbol *Symbol, uint32_t RelType,
13001322
uint64_t Addend, uint64_t Value);
13011323

1324+
/// Return locations (offsets) of data section relocations targeting internals
1325+
/// of this functions.
1326+
const DenseSet<uint64_t> &getInternalRefDataRelocations() const {
1327+
return InternalRefDataRelocations;
1328+
}
1329+
13021330
/// Return the name of the section this function originated from.
13031331
std::optional<StringRef> getOriginSectionName() const {
13041332
if (!OriginSection)

bolt/lib/Core/BinaryContext.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,17 @@ void BinaryContext::foldFunction(BinaryFunction &ChildBF,
15201520
}
15211521
ChildBF.getSymbols().clear();
15221522

1523+
// Reset function mapping for local symbols.
1524+
for (uint64_t RelOffset : ChildBF.getInternalRefDataRelocations()) {
1525+
const Relocation *Rel = getRelocationAt(RelOffset);
1526+
if (!Rel || !Rel->Symbol)
1527+
continue;
1528+
1529+
WriteSymbolMapLock.lock();
1530+
SymbolToFunctionMap[Rel->Symbol] = nullptr;
1531+
WriteSymbolMapLock.unlock();
1532+
}
1533+
15231534
// Move other names the child function is known under.
15241535
llvm::move(ChildBF.Aliases, std::back_inserter(ParentBF.Aliases));
15251536
ChildBF.Aliases.clear();

bolt/lib/Rewrite/RewriteInstance.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2954,8 +2954,10 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
29542954
// if-condition above) so we're handling a relocation from a function
29552955
// to itself. RISC-V uses such relocations for branches, for example.
29562956
// These should not be registered as externally references offsets.
2957-
if (!ContainingBF)
2958-
ReferencedBF->registerReferencedOffset(RefFunctionOffset);
2957+
if (!ContainingBF && !ReferencedBF->isInConstantIsland(Address)) {
2958+
ReferencedBF->registerInternalRefDataRelocation(RefFunctionOffset,
2959+
Rel.getOffset());
2960+
}
29592961
}
29602962
if (opts::Verbosity > 1 &&
29612963
BinarySection(*BC, RelocatedSection).isWritable())

0 commit comments

Comments
 (0)