[llvm-gsymutil] Don't warn about non-increasing line tables with merged functions

[alx32]

When converting a dSYM to gSYM, if the dSYM contains merged functions, it is expected that the dSYM contain line tables with non-monotonically increasing addresses.

This isn't an issue and we shouldn't emit an error or warning in such cases.

With this change we don't emit such messages when converting a dSYM to gSYM with merged functions enabled.

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: None (alx32)

Changes

When converting a dSYM to gSYM, if the dSYM contains merged functions, it is expected that the dSYM contain line tables with non-monotonically increasing addresses.

This isn't an issue and we shouldn't emit an error or warning in such cases.

With this change we don't emit such messages when converting a dSYM to gSYM with merged functions enabled.

---

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

5 Files Affected:

• (modified) llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h (+4-1)
• (modified) llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp (+1-1)
• (modified) llvm/lib/DebugInfo/GSYM/GsymCreator.cpp (+1-1)
• (modified) llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-merged-callsites-dsym.yaml (+5-2)
• (modified) llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp (+1)

diff --git a/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h b/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
index 0d098da96dd278..d6d7757f5a5e38 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
@@ -147,7 +147,7 @@ class GsymCreator {
   bool IsSegment = false;
   bool Finalized = false;
   bool Quiet;
-
+  bool UseMergedFuncs = false;
 
   /// Get the first function start address.
   ///
@@ -486,6 +486,9 @@ class GsymCreator {
   /// encode.
   llvm::Expected<std::unique_ptr<GsymCreator>>
   createSegment(uint64_t SegmentSize, size_t &FuncIdx) const;
+
+  bool getUseMergedFunctions() const { return UseMergedFuncs; }
+  void setUseMergedFunctions(bool Enable) { UseMergedFuncs = Enable; }
 };
 
 } // namespace gsym
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 568af5ee8e3ae0..84894a406cacea 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -405,7 +405,7 @@ static void convertFunctionLineTable(OutputAggregator &Out, CUInfo &CUI,
           OS << "warning: duplicate line table detected for DIE:\n";
           Die.dump(OS, 0, DIDumpOptions::getForSingleDIE());
         });
-      else
+      else if (!Gsym.getUseMergedFunctions())
         Out.Report("Non-monotonically increasing addresses",
                    [&](raw_ostream &OS) {
                      OS << "error: line table has addresses that do not "
diff --git a/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp b/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp
index 14078f5aaf9a46..9c52924ef6cd78 100644
--- a/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp
+++ b/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp
@@ -22,7 +22,7 @@ using namespace llvm;
 using namespace gsym;
 
 GsymCreator::GsymCreator(bool Quiet)
-    : StrTab(StringTableBuilder::ELF), Quiet(Quiet) {
+    : StrTab(StringTableBuilder::ELF), Quiet(Quiet), UseMergedFuncs(false) {
   insertFile(StringRef());
 }
 
diff --git a/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-merged-callsites-dsym.yaml b/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-merged-callsites-dsym.yaml
index 4cecc79c72b4b3..5d1a6f2e6d1af4 100644
--- a/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-merged-callsites-dsym.yaml
+++ b/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-merged-callsites-dsym.yaml
@@ -3,13 +3,16 @@
 # RUN: split-file %s %t
 # RUN: yaml2obj %t/merged_callsites.dSYM.yaml -o %t/merged_callsites.dSYM
 
-# RUN: llvm-gsymutil --convert=%t/merged_callsites.dSYM --merged-functions --callsites-yaml-file=%t/callsites.yaml -o %t/call_sites_dSYM.gsym
-# RUN: llvm-gsymutil --convert=%t/merged_callsites.dSYM --merged-functions --dwarf-callsites -o %t/dwarf_call_sites_dSYM.gsym
+# RUN: llvm-gsymutil --convert=%t/merged_callsites.dSYM --merged-functions --callsites-yaml-file=%t/callsites.yaml -o %t/call_sites_dSYM.gsym | FileCheck --check-prefix=CHECK-CONVERT %s
+# RUN: llvm-gsymutil --convert=%t/merged_callsites.dSYM --merged-functions --dwarf-callsites -o %t/dwarf_call_sites_dSYM.gsym | FileCheck --check-prefix=CHECK-CONVERT %s
 
 # Dump the GSYM file and check the output for callsite information
 # RUN: llvm-gsymutil %t/call_sites_dSYM.gsym | FileCheck --check-prefix=CHECK-MERGED-CALLSITES %s
 # RUN: llvm-gsymutil %t/dwarf_call_sites_dSYM.gsym | FileCheck --check-prefix=CHECK-MERGED-CALLSITES %s
 
+# CHECK-CONVERT-NOT: line table has addresses that do not monotonically increase
+# CHECK-CONVERT-NOT: Non-monotonically increasing addresses occurred
+
 # CHECK-MERGED-CALLSITES:      FunctionInfo @ 0x[[#%x,FUNC4_1:]]: [0x[[#%x,FUNC4_1_START:]] - 0x[[#%x,FUNC4_1_END:]]) "function4_copy1"
 # CHECK-MERGED-CALLSITES:      ++ Merged FunctionInfos[0]:
 # CHECK-MERGED-CALLSITES-NEXT:     [0x[[#%x,FUNC4_2_START:]] - 0x[[#%x,FUNC4_2_END:]]) "function4_copy2"
diff --git a/llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp b/llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
index 654da68bb69600..5af35cf2d9c8cb 100644
--- a/llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
+++ b/llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
@@ -331,6 +331,7 @@ static llvm::Error handleObjectFile(ObjectFile &Obj, const std::string &OutFile,
       NumThreads > 0 ? NumThreads : std::thread::hardware_concurrency();
 
   GsymCreator Gsym(Quiet);
+  Gsym.setUseMergedFunctions(UseMergedFunctions);
 
   // See if we can figure out the base address for a given object file, and if
   // we can, then set the base address to use to this value. This will ease

[ellishg]

We really should have brackets around these if/else blocks

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

[ellishg]

Can we document this variable?

[kyulee-com]

It seems you're matching get/set pair, but this is just about checking a status. I prefer isSomething() for the check. Maybe isUsingMergedFunctions(). For the set part, you can do enableUseMergedFunctions(bool) or something.

[alx32]

Looking again, the best way to go about this would be to use the DW_AT_LLVM_stmt_sequence attribute and avoiding the error via that methodology. I'll submit that as a separate PR.