Skip to content

Commit

Permalink
[BOLT][DWARF] Fix race conditions for debug fission in non-determinis…
Browse files Browse the repository at this point in the history
…tic mode

Summary: Adding mutexes to avoid runtime race conditions.

(cherry picked from FBD33439854)
  • Loading branch information
ayermolo authored and maksfb committed Jan 5, 2022
1 parent bc9032c commit e579f5c
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 12 deletions.
4 changes: 4 additions & 0 deletions bolt/include/bolt/Core/DebugData.h
Expand Up @@ -259,6 +259,8 @@ class DebugAddrWriter {
std::unordered_map<uint64_t, AddressForDWOCU> AddressMaps;
/// Maps DWOID to offset within .debug_addr section.
std::unordered_map<uint64_t, uint64_t> DWOIdToOffsetMap;
/// Mutex used for parallel processing of debug info.
std::mutex WriterMutex;
};

using DebugStrBufferVector = SmallVector<char, 16>;
Expand All @@ -278,6 +280,8 @@ class DebugStrWriter {
bool isInitialized() const { return !StrBuffer->empty(); }

private:
/// Mutex used for parallel processing of debug info.
std::mutex WriterMutex;
/// Initializes Buffer and Stream.
void initialize();
/// Creats internal data stractures.
Expand Down
5 changes: 4 additions & 1 deletion bolt/include/bolt/Rewrite/DWARFRewriter.h
Expand Up @@ -69,9 +69,10 @@ class DWARFRewriter {
std::mutex LocListDebugInfoPatchesMutex;

/// Update debug info for all DIEs in \p Unit.
void updateUnitDebugInfo(uint64_t CUIndex, DWARFUnit &Unit,
void updateUnitDebugInfo(DWARFUnit &Unit,
DebugInfoBinaryPatcher &DebugInfoPatcher,
DebugAbbrevWriter &AbbrevWriter,
DebugLocWriter &DebugLocWriter,
Optional<uint64_t> RangesBase = None);

/// Patches the binary for an object's address ranges to be updated.
Expand Down Expand Up @@ -179,6 +180,7 @@ class DWARFRewriter {
/// Helper function for creating and returning per-DWO patchers/writers.
template <class T, class Patcher>
Patcher *getBinaryDWOPatcherHelper(T &BinaryPatchers, uint64_t DwoId) {
std::lock_guard<std::mutex> Lock(DebugInfoPatcherMutex);
auto Iter = BinaryPatchers.find(DwoId);
if (Iter == BinaryPatchers.end()) {
// Using make_pair instead of {} to work around bug in older version of
Expand Down Expand Up @@ -211,6 +213,7 @@ class DWARFRewriter {
/// Creates abbrev writer for DWO unit with \p DWOId.
DebugAbbrevWriter *createBinaryDWOAbbrevWriter(DWARFContext &Context,
uint64_t DWOId) {
std::lock_guard<std::mutex> Lock(DebugInfoPatcherMutex);
auto &Entry = BinaryDWOAbbrevWriters[DWOId];
Entry = std::make_unique<DebugAbbrevWriter>(Context, DWOId);
return Entry.get();
Expand Down
3 changes: 3 additions & 0 deletions bolt/lib/Core/DebugData.cpp
Expand Up @@ -181,6 +181,7 @@ void DebugAddrWriter::AddressForDWOCU::dump() {
}
uint32_t DebugAddrWriter::getIndexFromAddress(uint64_t Address,
uint64_t DWOId) {
std::lock_guard<std::mutex> Lock(WriterMutex);
if (!AddressMaps.count(DWOId))
AddressMaps[DWOId] = AddressForDWOCU();

Expand All @@ -199,6 +200,7 @@ uint32_t DebugAddrWriter::getIndexFromAddress(uint64_t Address,
// update AddressToIndex and IndexToAddress
void DebugAddrWriter::addIndexAddress(uint64_t Address, uint32_t Index,
uint64_t DWOId) {
std::lock_guard<std::mutex> Lock(WriterMutex);
AddressForDWOCU &Map = AddressMaps[DWOId];
auto Entry = Map.find(Address);
if (Entry != Map.end()) {
Expand Down Expand Up @@ -666,6 +668,7 @@ void DebugStrWriter::initialize() {
}

uint32_t DebugStrWriter::addString(StringRef Str) {
std::lock_guard<std::mutex> Lock(WriterMutex);
if (StrBuffer->empty())
initialize();
auto Offset = StrBuffer->size();
Expand Down
39 changes: 28 additions & 11 deletions bolt/lib/Rewrite/DWARFRewriter.cpp
Expand Up @@ -223,6 +223,7 @@ void DWARFRewriter::updateDebugInfo() {
// specified.
std::unordered_map<std::string, uint32_t> NameToIndexMap;
std::unordered_map<uint64_t, std::string> DWOIdToName;
std::mutex AccessMutex;

auto updateDWONameCompDir = [&](DWARFUnit &Unit) -> void {
const DWARFDie &DIE = Unit.getUnitDIE();
Expand All @@ -231,7 +232,13 @@ void DWARFRewriter::updateDebugInfo() {
(void)AttrInfoVal;
assert(AttrInfoVal && "Skeleton CU doesn't have dwo_name.");

std::string ObjectName = getDWOName(Unit, &NameToIndexMap, DWOIdToName);
std::string ObjectName = "";

{
std::lock_guard<std::mutex> Lock(AccessMutex);
ObjectName = getDWOName(Unit, &NameToIndexMap, DWOIdToName);
}

uint32_t NewOffset = StrWriter->addString(ObjectName.c_str());
DebugInfoPatcher->addLE32Patch(AttrInfoVal->Offset, NewOffset,
AttrInfoVal->Size);
Expand All @@ -256,6 +263,7 @@ void DWARFRewriter::updateDebugInfo() {
if (DWOId)
SplitCU = BC.getDWOCU(*DWOId);

DebugLocWriter *DebugLocWriter = nullptr;
// Skipping CUs that failed to load.
if (SplitCU) {
updateDWONameCompDir(*Unit);
Expand All @@ -264,8 +272,14 @@ void DWARFRewriter::updateDebugInfo() {
// have same DWO ID.
assert(LocListWritersByCU.count(*DWOId) == 0 &&
"LocList writer for DWO unit already exists.");
LocListWritersByCU[*DWOId] =
std::make_unique<DebugLoclistWriter>(&BC, *DWOId);
{
std::lock_guard<std::mutex> Lock(AccessMutex);
DebugLocWriter =
LocListWritersByCU
.insert(
{*DWOId, std::make_unique<DebugLoclistWriter>(&BC, *DWOId)})
.first->second.get();
}
DebugInfoBinaryPatcher *DwoDebugInfoPatcher =
llvm::cast<DebugInfoBinaryPatcher>(
getBinaryDWODebugInfoPatcher(*DWOId));
Expand All @@ -278,16 +292,20 @@ void DWARFRewriter::updateDebugInfo() {
DwoDebugInfoPatcher->addUnitBaseOffsetLabel((*SplitCU)->getOffset());
DebugAbbrevWriter *DWOAbbrevWriter =
createBinaryDWOAbbrevWriter((*SplitCU)->getContext(), *DWOId);
updateUnitDebugInfo(*DWOId, *(*SplitCU), *DwoDebugInfoPatcher,
*DWOAbbrevWriter);
updateUnitDebugInfo(*(*SplitCU), *DwoDebugInfoPatcher, *DWOAbbrevWriter,
*DebugLocWriter);
DwoDebugInfoPatcher->clearDestinationLabels();
if (!DwoDebugInfoPatcher->getWasRangBasedUsed())
RangesBase = None;
}

{
std::lock_guard<std::mutex> Lock(AccessMutex);
DebugLocWriter = LocListWritersByCU[CUIndex].get();
}
DebugInfoPatcher->addUnitBaseOffsetLabel(Unit->getOffset());
updateUnitDebugInfo(CUIndex, *Unit, *DebugInfoPatcher, *AbbrevWriter,
RangesBase);
updateUnitDebugInfo(*Unit, *DebugInfoPatcher, *AbbrevWriter,
*DebugLocWriter, RangesBase);
};

if (opts::NoThreads || opts::DeterministicDebugInfo) {
Expand Down Expand Up @@ -318,13 +336,12 @@ void DWARFRewriter::updateDebugInfo() {
}

void DWARFRewriter::updateUnitDebugInfo(
uint64_t CUIndex, DWARFUnit &Unit, DebugInfoBinaryPatcher &DebugInfoPatcher,
DebugAbbrevWriter &AbbrevWriter, Optional<uint64_t> RangesBase) {
DWARFUnit &Unit, DebugInfoBinaryPatcher &DebugInfoPatcher,
DebugAbbrevWriter &AbbrevWriter, DebugLocWriter &DebugLocWriter,
Optional<uint64_t> RangesBase) {
// Cache debug ranges so that the offset for identical ranges could be reused.
std::map<DebugAddressRangesVector, uint64_t> CachedRanges;

auto &DebugLocWriter = *LocListWritersByCU[CUIndex].get();

uint64_t DIEOffset = Unit.getOffset() + Unit.getHeaderSize();
uint64_t NextCUOffset = Unit.getNextUnitOffset();
DWARFDebugInfoEntry Die;
Expand Down

0 comments on commit e579f5c

Please sign in to comment.