-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[BOLT] Don't emit invalid (gdb-breaking) address ranges in gdb-index #151923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BOLT] Don't emit invalid (gdb-breaking) address ranges in gdb-index #151923
Conversation
|
@llvm/pr-subscribers-bolt Author: None (itrofimow) ChangesEmpty address map ranges in gdb-index break gdb, so don't emit them Full diff: https://github.com/llvm/llvm-project/pull/151923.diff 1 Files Affected:
diff --git a/bolt/lib/Core/GDBIndex.cpp b/bolt/lib/Core/GDBIndex.cpp
index c7fb4889646b4..5406f0c4e38d2 100644
--- a/bolt/lib/Core/GDBIndex.cpp
+++ b/bolt/lib/Core/GDBIndex.cpp
@@ -99,10 +99,19 @@ void GDBIndex::updateGdbIndexSection(
Data += SymbolTableOffset - CUTypesOffset;
// Calculate the size of the new address table.
+ const auto IsValidAddressRange = [](const DebugAddressRange &Range) {
+ return Range.HighPC > Range.LowPC;
+ };
+
uint32_t NewAddressTableSize = 0;
for (const auto &CURangesPair : ARangesSectionWriter.getCUAddressRanges()) {
const SmallVector<DebugAddressRange, 2> &Ranges = CURangesPair.second;
- NewAddressTableSize += Ranges.size() * 20;
+ NewAddressTableSize +=
+ llvm::count_if(Ranges,
+ [&IsValidAddressRange](const DebugAddressRange &Range) {
+ return IsValidAddressRange(Range);
+ }) *
+ 20;
}
// Difference between old and new table (and section) sizes.
@@ -163,10 +172,15 @@ void GDBIndex::updateGdbIndexSection(
const uint32_t CUIndex = OffsetToIndexMap[CURangesPair.first];
const DebugAddressRangesVector &Ranges = CURangesPair.second;
for (const DebugAddressRange &Range : Ranges) {
- write64le(Buffer, Range.LowPC);
- write64le(Buffer + 8, Range.HighPC);
- write32le(Buffer + 16, CUIndex);
- Buffer += 20;
+ // Don't emit ranges that break gdb,
+ // https://sourceware.org/bugzilla/show_bug.cgi?id=33247.
+ // We've seen [0, 0) ranges here, for instance.
+ if (IsValidAddressRange(Range)) {
+ write64le(Buffer, Range.LowPC);
+ write64le(Buffer + 8, Range.HighPC);
+ write32le(Buffer + 16, CUIndex);
+ Buffer += 20;
+ }
}
}
|
maksfb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - thanks for the fix! Can you add a test case?
|
Thanks! So this PR is me fixing the symptoms, and I have no idea what exactly leads to BOLT generating such ranges, nor how to fix the root-cause; thus I don't know how to set up a test for this |
maksfb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. This change looks straightforward and it only affects gdb-index. Thanks for the fix!
|
Thanks! Could you please merge this PR for me? |
…lvm#151923) Empty address map ranges in gdb-index break gdb, so don't emit them
Empty address map ranges in gdb-index break gdb, so don't emit them