[llvm] Gsymutil aggregation similar to DwarfDump --verify (PR #81154)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 8 07:57:01 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-debuginfo
Author: Kevin Frei (kevinfrei)
<details>
<summary>Changes</summary>
GsymUtil, like DwarfDump --verify, spews a *lot* of data necessary to understand/diagnose issues with DWARF data. The trouble is that the kind of information necessary to make the messages useful also makes them nearly impossible to easily categorize. I put together a similar output categorizer (https://github.com/llvm/llvm-project/pull/79648) that will emit a summary of issues identified at the bottom of the (very verbose) output, enabling easier tracking of issues as they arise or are addressed.
There's a single output change, where a message "warning: Unable to retrieve DWO .debug_info section for some object files. (Remove the --quiet flag for full output)" was being dumped the first time it was encountered (in what looks like an attempt to make something easily grep-able), but rather than keep the output in the same order, that message is now a 'category' so gets emitted at the end of the output. The test 'tools/llvm-gsymutil/X86/elf-dwo.yaml' was changed to reflect this difference.
---
Patch is 58.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81154.diff
10 Files Affected:
- (modified) llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h (+4-3)
- (modified) llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h (+4-1)
- (modified) llvm/include/llvm/DebugInfo/GSYM/ObjectFileTransformer.h (+3-4)
- (added) llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h (+107)
- (modified) llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp (+162-140)
- (modified) llvm/lib/DebugInfo/GSYM/GsymCreator.cpp (+21-16)
- (modified) llvm/lib/DebugInfo/GSYM/ObjectFileTransformer.cpp (+8-7)
- (modified) llvm/test/tools/llvm-gsymutil/X86/elf-dwo.yaml (+1-1)
- (modified) llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp (+23-21)
- (modified) llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp (+73-46)
``````````diff
diff --git a/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h b/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h
index fd2433a677b8f..3abf3bacabdd8 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h
@@ -22,6 +22,7 @@ namespace gsym {
struct CUInfo;
struct FunctionInfo;
class GsymCreator;
+class OutputAggregator;
/// A class that transforms the DWARF in a DWARFContext into GSYM information
/// by populating the GsymCreator object that it is constructed with. This
@@ -52,9 +53,9 @@ class DwarfTransformer {
///
/// \returns An error indicating any fatal issues that happen when parsing
/// the DWARF, or Error::success() if all goes well.
- llvm::Error convert(uint32_t NumThreads, raw_ostream *OS);
+ llvm::Error convert(uint32_t NumThreads, OutputAggregator &OS);
- llvm::Error verify(StringRef GsymPath, raw_ostream &OS);
+ llvm::Error verify(StringRef GsymPath, OutputAggregator &OS);
private:
@@ -79,7 +80,7 @@ class DwarfTransformer {
/// information.
///
/// \param Die The DWARF debug info entry to parse.
- void handleDie(raw_ostream *Strm, CUInfo &CUI, DWARFDie Die);
+ void handleDie(OutputAggregator &Strm, CUInfo &CUI, DWARFDie Die);
DWARFContext &DICtx;
GsymCreator &Gsym;
diff --git a/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h b/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
index bb52f3f2cd56b..1e2f185b00f91 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
@@ -10,8 +10,10 @@
#define LLVM_DEBUGINFO_GSYM_GSYMCREATOR_H
#include <functional>
+#include <map>
#include <memory>
#include <mutex>
+#include <string>
#include <thread>
#include "llvm/ADT/AddressRanges.h"
@@ -28,6 +30,7 @@ namespace llvm {
namespace gsym {
class FileWriter;
+class OutputAggregator;
/// GsymCreator is used to emit GSYM data to a stand alone file or section
/// within a file.
@@ -360,7 +363,7 @@ class GsymCreator {
/// function infos, and function infos that were merged or removed.
/// \returns An error object that indicates success or failure of the
/// finalize.
- llvm::Error finalize(llvm::raw_ostream &OS);
+ llvm::Error finalize(OutputAggregator &OS);
/// Set the UUID value.
///
diff --git a/llvm/include/llvm/DebugInfo/GSYM/ObjectFileTransformer.h b/llvm/include/llvm/DebugInfo/GSYM/ObjectFileTransformer.h
index 2018e0962ca79..fe44e82e0dd2c 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/ObjectFileTransformer.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/ObjectFileTransformer.h
@@ -13,8 +13,6 @@
namespace llvm {
-class raw_ostream;
-
namespace object {
class ObjectFile;
}
@@ -22,6 +20,7 @@ class ObjectFile;
namespace gsym {
class GsymCreator;
+class OutputAggregator;
class ObjectFileTransformer {
public:
@@ -40,8 +39,8 @@ class ObjectFileTransformer {
///
/// \returns An error indicating any fatal issues that happen when parsing
/// the DWARF, or Error::success() if all goes well.
- static llvm::Error convert(const object::ObjectFile &Obj, raw_ostream *Log,
- GsymCreator &Gsym);
+ static llvm::Error convert(const object::ObjectFile &Obj,
+ OutputAggregator &Output, GsymCreator &Gsym);
};
} // namespace gsym
diff --git a/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h b/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
new file mode 100644
index 0000000000000..086d5f217fda6
--- /dev/null
+++ b/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
@@ -0,0 +1,107 @@
+//===- DwarfTransformer.h ---------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFO_GSYM_OUTPUTAGGREGATOR_H
+#define LLVM_DEBUGINFO_GSYM_OUTPUTAGGREGATOR_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/DebugInfo/GSYM/ExtractRanges.h"
+
+#include <map>
+#include <string>
+
+namespace llvm {
+
+class raw_ostream;
+
+namespace gsym {
+
+class OutputAggregator {
+protected:
+ // A std::map is preferable over an llvm::StringMap for presenting results
+ // in a predictable order.
+ std::map<std::string, unsigned> Aggregation;
+ raw_ostream *Out;
+ bool IncludeDetail;
+
+public:
+ OutputAggregator(raw_ostream *out, bool includeDetail = true)
+ : Out(out), IncludeDetail(includeDetail) {}
+
+ // Do I want a "detail level" thing? I think so, actually...
+ void ShowDetail(bool showDetail) { IncludeDetail = showDetail; }
+ bool IsShowingDetail() const { return IncludeDetail; }
+
+ size_t GetNumCategories() const { return Aggregation.size(); }
+
+ void Report(StringRef s, std::function<void(raw_ostream &o)> detailCallback) {
+ Aggregation[std::string(s)]++;
+ if (IncludeDetail && Out != nullptr)
+ detailCallback(*Out);
+ }
+
+ void Report(StringRef s, std::function<void()> detailCallback) {
+ Aggregation[std::string(s)]++;
+ if (IncludeDetail)
+ detailCallback();
+ }
+
+ void EnumerateResults(
+ std::function<void(StringRef, unsigned)> handleCounts) const {
+ for (auto &&[name, count] : Aggregation) {
+ handleCounts(name, count);
+ }
+ }
+
+ raw_ostream *GetOS() const { return Out; }
+
+ // You can just use the stream, and if it's null, nothing happens.
+ // Don't do a lot of stuff like this, but it's convenient for silly stuff.
+ // It doesn't work with things that have custom insertion operators, though.
+ template <typename T> OutputAggregator &operator<<(T &&value) {
+ if (Out != nullptr)
+ *Out << value;
+ return *this;
+ }
+
+ // For multi-threaded usage, we can collect stuff in another aggregator,
+ // then merge it in here
+ void Merge(const OutputAggregator &other) {
+ for (auto &&[name, count] : other.Aggregation) {
+ auto it = Aggregation.find(name);
+ if (it == Aggregation.end())
+ Aggregation.emplace(name, count);
+ else
+ it->second += count;
+ }
+ }
+};
+
+class StringAggregator : public OutputAggregator {
+private:
+ std::string storage;
+ raw_string_ostream StrStream;
+
+public:
+ StringAggregator(bool includeDetail = true)
+ : OutputAggregator(&StrStream, includeDetail), StrStream(storage) {}
+ friend OutputAggregator &operator<<(OutputAggregator &agg,
+ StringAggregator &sa);
+};
+
+inline OutputAggregator &operator<<(OutputAggregator &agg,
+ StringAggregator &sa) {
+ agg.Merge(sa);
+ sa.StrStream.flush();
+ return agg << sa.storage;
+}
+
+} // namespace gsym
+} // namespace llvm
+
+#endif // LLVM_DEBUGINFO_GSYM_OUTPUTAGGREGATOR_H
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 6268c7845a142..c2f9de6579f7a 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -21,6 +21,8 @@
#include "llvm/DebugInfo/GSYM/GsymCreator.h"
#include "llvm/DebugInfo/GSYM/GsymReader.h"
#include "llvm/DebugInfo/GSYM/InlineInfo.h"
+#include "llvm/DebugInfo/GSYM/OutputAggregator.h"
+
#include <optional>
using namespace llvm;
@@ -215,9 +217,9 @@ ConvertDWARFRanges(const DWARFAddressRangesVector &DwarfRanges) {
return Ranges;
}
-static void parseInlineInfo(GsymCreator &Gsym, raw_ostream *Log, CUInfo &CUI,
- DWARFDie Die, uint32_t Depth, FunctionInfo &FI,
- InlineInfo &Parent,
+static void parseInlineInfo(GsymCreator &Gsym, OutputAggregator &Out,
+ CUInfo &CUI, DWARFDie Die, uint32_t Depth,
+ FunctionInfo &FI, InlineInfo &Parent,
const AddressRanges &AllParentRanges,
bool &WarnIfEmpty) {
if (!hasInlineInfo(Die, Depth))
@@ -250,14 +252,18 @@ static void parseInlineInfo(GsymCreator &Gsym, raw_ostream *Log, CUInfo &CUI,
// parsing for.
if (AllParentRanges.contains(InlineRange)) {
WarnIfEmpty = false;
- } else if (Log) {
- *Log << "error: inlined function DIE at "
- << HEX32(Die.getOffset()) << " has a range ["
- << HEX64(InlineRange.start()) << " - "
- << HEX64(InlineRange.end()) << ") that isn't contained in "
- << "any parent address ranges, this inline range will be "
- "removed.\n";
- }
+ } else
+ Out.Report("Function DIE has uncontained address range",
+ [&](raw_ostream &OS) {
+ OS << "error: inlined function DIE at "
+ << HEX32(Die.getOffset()) << " has a range ["
+ << HEX64(InlineRange.start()) << " - "
+ << HEX64(InlineRange.end())
+ << ") that isn't contained in "
+ << "any parent address ranges, this inline range "
+ "will be "
+ "removed.\n";
+ });
}
}
}
@@ -281,26 +287,30 @@ static void parseInlineInfo(GsymCreator &Gsym, raw_ostream *Log, CUInfo &CUI,
II.CallLine = dwarf::toUnsigned(Die.find(dwarf::DW_AT_call_line), 0);
// parse all children and append to parent
for (DWARFDie ChildDie : Die.children())
- parseInlineInfo(Gsym, Log, CUI, ChildDie, Depth + 1, FI, II,
+ parseInlineInfo(Gsym, Out, CUI, ChildDie, Depth + 1, FI, II,
AllInlineRanges, WarnIfEmpty);
Parent.Children.emplace_back(std::move(II));
- } else if (Log) {
- *Log << "error: inlined function DIE at " << HEX32(Die.getOffset())
- << " has an invalid file index " << DwarfFileIdx
- << " in its DW_AT_call_file attribute, this inline entry and all "
- << "children will be removed.\n";
- }
+ } else
+ Out.Report(
+ "Inlined function die has invlaid file index in DW_AT_call_file",
+ [&](raw_ostream &OS) {
+ OS << "error: inlined function DIE at " << HEX32(Die.getOffset())
+ << " has an invalid file index " << DwarfFileIdx
+ << " in its DW_AT_call_file attribute, this inline entry and "
+ "all "
+ << "children will be removed.\n";
+ });
return;
}
if (Tag == dwarf::DW_TAG_subprogram || Tag == dwarf::DW_TAG_lexical_block) {
// skip this Die and just recurse down
for (DWARFDie ChildDie : Die.children())
- parseInlineInfo(Gsym, Log, CUI, ChildDie, Depth + 1, FI, Parent,
+ parseInlineInfo(Gsym, Out, CUI, ChildDie, Depth + 1, FI, Parent,
AllParentRanges, WarnIfEmpty);
}
}
-static void convertFunctionLineTable(raw_ostream *Log, CUInfo &CUI,
+static void convertFunctionLineTable(OutputAggregator &Out, CUInfo &CUI,
DWARFDie Die, GsymCreator &Gsym,
FunctionInfo &FI) {
std::vector<uint32_t> RowVector;
@@ -319,15 +329,15 @@ static void convertFunctionLineTable(raw_ostream *Log, CUInfo &CUI,
if (FilePath.empty()) {
// If we had a DW_AT_decl_file, but got no file then we need to emit a
// warning.
- if (Log) {
+ Out.Report("Invalid file index in DW_AT_decl_file", [&](raw_ostream &OS) {
const uint64_t DwarfFileIdx = dwarf::toUnsigned(
Die.findRecursively(dwarf::DW_AT_decl_file), UINT32_MAX);
- *Log << "error: function DIE at " << HEX32(Die.getOffset())
- << " has an invalid file index " << DwarfFileIdx
- << " in its DW_AT_decl_file attribute, unable to create a single "
- << "line entry from the DW_AT_decl_file/DW_AT_decl_line "
- << "attributes.\n";
- }
+ OS << "error: function DIE at " << HEX32(Die.getOffset())
+ << " has an invalid file index " << DwarfFileIdx
+ << " in its DW_AT_decl_file attribute, unable to create a single "
+ << "line entry from the DW_AT_decl_file/DW_AT_decl_line "
+ << "attributes.\n";
+ });
return;
}
if (auto Line =
@@ -347,14 +357,15 @@ static void convertFunctionLineTable(raw_ostream *Log, CUInfo &CUI,
std::optional<uint32_t> OptFileIdx =
CUI.DWARFToGSYMFileIndex(Gsym, Row.File);
if (!OptFileIdx) {
- if (Log) {
- *Log << "error: function DIE at " << HEX32(Die.getOffset()) << " has "
- << "a line entry with invalid DWARF file index, this entry will "
- << "be removed:\n";
- Row.dumpTableHeader(*Log, /*Indent=*/0);
- Row.dump(*Log);
- *Log << "\n";
- }
+ Out.Report(
+ "Invalid file index in DWARF line table", [&](raw_ostream &OS) {
+ OS << "error: function DIE at " << HEX32(Die.getOffset()) << " has "
+ << "a line entry with invalid DWARF file index, this entry will "
+ << "be removed:\n";
+ Row.dumpTableHeader(OS, /*Indent=*/0);
+ Row.dump(OS);
+ OS << "\n";
+ });
continue;
}
const uint32_t FileIdx = OptFileIdx.value();
@@ -367,12 +378,15 @@ static void convertFunctionLineTable(raw_ostream *Log, CUInfo &CUI,
// an error, but not worth stopping the creation of the GSYM.
if (!FI.Range.contains(RowAddress)) {
if (RowAddress < FI.Range.start()) {
- if (Log) {
- *Log << "error: DIE has a start address whose LowPC is between the "
- "line table Row[" << RowIndex << "] with address "
- << HEX64(RowAddress) << " and the next one.\n";
- Die.dump(*Log, 0, DIDumpOptions::getForSingleDIE());
- }
+ Out.Report("Start address lies between valid Row table entries",
+ [&](raw_ostream &OS) {
+ OS << "error: DIE has a start address whose LowPC is "
+ "between the "
+ "line table Row["
+ << RowIndex << "] with address " << HEX64(RowAddress)
+ << " and the next one.\n";
+ Die.dump(OS, 0, DIDumpOptions::getForSingleDIE());
+ });
RowAddress = FI.Range.start();
} else {
continue;
@@ -388,20 +402,21 @@ static void convertFunctionLineTable(raw_ostream *Log, CUInfo &CUI,
// line entry we already have in our "function_info.Lines". If
// so break out after printing a warning.
auto FirstLE = FI.OptLineTable->first();
- if (FirstLE && *FirstLE == LE) {
- if (Log && !Gsym.isQuiet()) {
- *Log << "warning: duplicate line table detected for DIE:\n";
- Die.dump(*Log, 0, DIDumpOptions::getForSingleDIE());
- }
- } else {
- if (Log) {
- *Log << "error: line table has addresses that do not "
- << "monotonically increase:\n";
- for (uint32_t RowIndex2 : RowVector)
- CUI.LineTable->Rows[RowIndex2].dump(*Log);
- Die.dump(*Log, 0, DIDumpOptions::getForSingleDIE());
- }
- }
+ if (FirstLE && *FirstLE == LE)
+ // if (Log && !Gsym.isQuiet()) { TODO <-- This looks weird
+ Out.Report("Duplicate line table detected", [&](raw_ostream &OS) {
+ OS << "warning: duplicate line table detected for DIE:\n";
+ Die.dump(OS, 0, DIDumpOptions::getForSingleDIE());
+ });
+ else
+ Out.Report("Non-monotonically increasing addresses",
+ [&](raw_ostream &OS) {
+ OS << "error: line table has addresses that do not "
+ << "monotonically increase:\n";
+ for (uint32_t RowIndex2 : RowVector)
+ CUI.LineTable->Rows[RowIndex2].dump(OS);
+ Die.dump(OS, 0, DIDumpOptions::getForSingleDIE());
+ });
break;
}
@@ -429,7 +444,8 @@ static void convertFunctionLineTable(raw_ostream *Log, CUInfo &CUI,
FI.OptLineTable = std::nullopt;
}
-void DwarfTransformer::handleDie(raw_ostream *OS, CUInfo &CUI, DWARFDie Die) {
+void DwarfTransformer::handleDie(OutputAggregator &Out, CUInfo &CUI,
+ DWARFDie Die) {
switch (Die.getTag()) {
case dwarf::DW_TAG_subprogram: {
Expected<DWARFAddressRangesVector> RangesOrError = Die.getAddressRanges();
@@ -442,11 +458,11 @@ void DwarfTransformer::handleDie(raw_ostream *OS, CUInfo &CUI, DWARFDie Die) {
break;
auto NameIndex = getQualifiedNameIndex(Die, CUI.Language, Gsym);
if (!NameIndex) {
- if (OS) {
- *OS << "error: function at " << HEX64(Die.getOffset())
- << " has no name\n ";
- Die.dump(*OS, 0, DIDumpOptions::getForSingleDIE());
- }
+ Out.Report("Function has no name", [&](raw_ostream &OS) {
+ OS << "error: function at " << HEX64(Die.getOffset())
+ << " has no name\n ";
+ Die.dump(OS, 0, DIDumpOptions::getForSingleDIE());
+ });
break;
}
// All ranges for the subprogram DIE in case it has multiple. We need to
@@ -472,8 +488,8 @@ void DwarfTransformer::handleDie(raw_ostream *OS, CUInfo &CUI, DWARFDie Die) {
// Many linkers can't remove DWARF and might set the LowPC to zero. Since
// high PC can be an offset from the low PC in more recent DWARF versions
- // we need to watch for a zero'ed low pc which we do using
- // ValidTextRanges below.
+ // we need to watch for a zero'ed low pc which we do using ValidTextRanges
+ // below.
if (!Gsym.IsValidTextAddress(Range.LowPC)) {
// We expect zero and -1 to be invalid addresses in DWARF depending
// on the linker of the DWARF. This indicates a function was stripped
@@ -482,13 +498,15 @@ void DwarfTransformer::handleDie(raw_ostream *OS, CUInfo &CUI, DWARFDie Die) {
if (Range.LowPC != 0) {
if (!Gsym.isQuiet()) {
// Unexpected invalid address, emit a warning
- if (OS) {
- *OS << "warning: DIE has an address range whose start address "
- "is not in any executable sections ("
- << *Gsym.GetValidTextRanges()
- << ") and will not be processed:\n";
- Die.dump(*OS, 0, DIDumpOptions::getForSingleDIE());
- }
+ Out.Report("Address range starts outside executable section",
+ [&](raw_ostream &OS) {
+ OS << "warning: DIE has an address range whose "
+ "start address "
+ "is not in any executable sections ("
+ << *Gsym.GetValidTextRanges()
+ << ") and will not be processed:\n";
+ Die.dump(OS, 0, DIDumpOptions::getForSingleDIE());
+ });
}
}
break;
@@ -498,14 +516,14 @@ void DwarfTransformer::handleDie(raw_ostream *OS, CUInfo &CUI, DWARFDie Die) {
FI.Range = {Range.LowPC, Range.HighPC};
FI.Name = *NameIndex;
if (CUI.LineTable)
- convertFunctionLineTable(OS, CUI, Die, Gsym, FI);
+ convertFunctionLineTable(Out, CUI, Die, Gsym, FI);
if (hasInlineInfo(Die, 0)) {
FI.Inline = InlineInfo();
FI.Inline->Name = *NameIndex;
FI.Inline->Ranges.insert(FI.Range);
bool WarnIfEmpty = true;
- parseInlineInfo(Gsym, OS, CUI, Die, 0, FI, *FI.Inline,
+ parseInlineInfo(Gsym, Out, CUI, Die, 0, FI, *FI.Inline,
AllSubprogramRanges, WarnIfEmpty);
// Make sure we at least got some valid inline info other than just
// the top level function. If we didn't then remove the inline info
@@ -517,11 +535,14 @@ void DwarfTransformer::handleDie(raw_ostream *OS, CUInfo &CUI, DWARFDie Die) {
// information object, we will know if we got anything valid from the
// debug info.
if...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/81154
More information about the llvm-commits
mailing list