Skip to content
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

Gsymutil aggregation similar to DwarfDump --verify #81154

Merged
merged 4 commits into from
Feb 13, 2024

Conversation

kevinfrei
Copy link
Contributor

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 (#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.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 8, 2024

@llvm/pr-subscribers-debuginfo

Author: Kevin Frei (kevinfrei)

Changes

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 (#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 --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]

Copy link

github-actions bot commented Feb 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@kevinfrei
Copy link
Contributor Author

@clayborg can you take a look at this please :)

llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h Outdated Show resolved Hide resolved
llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h Outdated Show resolved Hide resolved
llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h Outdated Show resolved Hide resolved
llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h Outdated Show resolved Hide resolved
llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h Outdated Show resolved Hide resolved
llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h Outdated Show resolved Hide resolved
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp Outdated Show resolved Hide resolved
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp Outdated Show resolved Hide resolved
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp Outdated Show resolved Hide resolved
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp Outdated Show resolved Hide resolved
@kevinfrei
Copy link
Contributor Author

Testing changes now. Updates coming.

Copy link
Collaborator

@clayborg clayborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the test suite runs fine:

./bin/llvm-lit -sv llvm/test/tools/llvm-gsymutil

And the DebugInfoGSYMTests unit test binary runs fine, this all LGTM now.

@kevinfrei
Copy link
Contributor Author

As long as the test suite runs fine:

./bin/llvm-lit -sv llvm/test/tools/llvm-gsymutil

And the DebugInfoGSYMTests unit test binary runs fine, this all LGTM now.

Yup. Everything is clean (I had to make a few changes to the unit test binary that you'll see in the diff). I've been running full check-llvm because I'm lazy and didn't want to accidentally miss a test.

@clayborg clayborg merged commit 3bdc4c7 into llvm:main Feb 13, 2024
4 checks passed
@kevinfrei kevinfrei deleted the gsym-summary branch February 13, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants