Clean up the GSym error aggregation code, and pass the aggregator by reference

[kevinfrei]

There was a problem with llvm-gsymutils error aggregation code not properly collecting aggregate errors. The was that the output aggregator collecting errors from other threads wasn't being passed by reference, so it was merging them into a copy of the app-wide output aggregator.

While I was at it, I added a better comment above the "Merge" code and made it a bit more efficient, after learning more details about emplace vs. insert or operator[] on std::map's.

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Kevin Frei (kevinfrei)

Changes

There was a problem with llvm-gsymutils error aggregation code not properly collecting aggregate errors. The was that the output aggregator collecting errors from other threads wasn't being passed by reference, so it was merging them into a copy of the app-wide output aggregator.

While I was at it, I added a better comment above the "Merge" code and made it a bit more efficient, after learning more details about emplace vs. insert or operator[] on std::map's.

---

Full diff: https://github.com/llvm/llvm-project/pull/89688.diff

2 Files Affected:

• (modified) llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h (+4-5)
• (modified) llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp (+1-1)

diff --git a/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h b/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
index 8deea3bff1ed76..35ef0a8bc89085 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/OutputAggregator.h
@@ -57,13 +57,12 @@ class OutputAggregator {
   }
 
   // For multi-threaded usage, we can collect stuff in another aggregator,
-  // then merge it in here
+  // then merge it in here. Note that this is *not* thread safe. It is up to
+  // the caller to ensure that this is only called from one thread at a time.
   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
+      auto [it, inserted] = Aggregation.emplace(name, count);
+      if (!inserted)
         it->second += count;
     }
   }
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index ff6b560d11726b..601686fdd3dd51 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -612,7 +612,7 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
       DWARFDie Die = getDie(*CU);
       if (Die) {
         CUInfo CUI(DICtx, dyn_cast<DWARFCompileUnit>(CU.get()));
-        pool.async([this, CUI, &LogMutex, Out, Die]() mutable {
+        pool.async([this, CUI, &LogMutex, &Out, Die]() mutable {
           std::string storage;
           raw_string_ostream StrStream(storage);
           OutputAggregator ThreadOut(Out.GetOS() ? &StrStream : nullptr);

[kevinfrei]

@jeffreytan81 could you take a look? @clayborg is out for the week (and this is very simple)