Add --quiet option to llvm-gsymutil to suppress output of warnings.

[simon.giesecke]

Rebase patch on main

[simon.giesecke]

Fix cmdline.test

[clayborg]

Contents looks good, just need a test to ensure that warnings are quieted. There should be a test for each kind of warning, so it should be easy to run that same command with the --quiet option and verify that the warning does _not_ appear.

[simon.giesecke]

In D102829#2771754, @clayborg wrote:

Contents looks good, just need a test to ensure that warnings are quieted. There should be a test for each kind of warning, so it should be easy to run that same command with the --quiet option and verify that the warning does _not_ appear.

Makes sense. However, I didn't find any tests for these warnings in llvm/test/tools/llvm-gsymutil.

There are test cases in https://github.com/llvm/llvm-project/blob/1d01fc100bb5bef5f5eaf92520b2e52f64ee1d6e/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp, however they don't check the warnings either. It seems that a test case like TestDWARFTextRanges would trigger a warning, though I am not completely sure which one. Should these test cases be transferred into llvm-lit tests as well?

[simon.giesecke]

@clayborg I am not sure what it means you now accepted the revision? I am still not sure what to do about the tests.

One more thing is that I suppress everything that's called a "warning" here. I tested it, and on our release/optimized binaries (built with gcc) we get lots of "errors" such as:

00:10:57.048 error: DIE has a start address whose LowPC is between the line table Row[9006] with address 0x00000000033aaf17 and the next one.
00:10:57.048 
00:10:57.048 0x0ca2fa30: DW_TAG_subprogram
00:10:57.048               DW_AT_abstract_origin	(0x0ca2fa0b "_ZN5boost15program_options20invalid_option_valueC4ERKSbIwSt11char_traitsIwESaIwEE")
00:10:57.048               DW_AT_MIPS_linkage_name	("_ZN5boost15program_options20invalid_option_valueC2ERKSbIwSt11char_traitsIwESaIwEE")
00:10:57.048               DW_AT_object_pointer	(0x0ca2fa49)
00:10:57.048               DW_AT_ranges	(0x009bbe70
00:10:57.048                  [0x000000000ae3a740, 0x000000000ae3ab29)
00:10:57.048                  [0x00000000033aaf20, 0x00000000033aaff5))
00:10:57.048               DW_AT_frame_base	(DW_OP_call_frame_cfa)
00:10:57.048               DW_AT_GNU_all_call_sites	(0x01)
00:10:57.048               DW_AT_sibling	(0x0ca3288d)
00:10:57.048 error: DIE has a start address whose LowPC is between the line table Row[9368] with address 0x00000000033ab1fb and the next one.

I am not sure if this was just accidentally called "error" instead of "warning", and we should suppress that as well with --quiet, or if this is rather a more severe issue, which should not be silenced. I guess in that case, we would probably update the compiler version to fix that (assuming it is fixed in a more recent gcc version).

[clayborg]

In D102829#2781201, @simon.giesecke wrote:

@clayborg I am not sure what it means you now accepted the revision? I am still not sure what to do about the tests.

I will need to add tests for these warnings since they don't already exist. These kinds of tests require a deep knowledge of making DWARF that contains each of these warnings, so I won't have you do this.

One more thing is that I suppress everything that's called a "warning" here. I tested it, and on our release/optimized binaries (built with gcc) we get lots of "errors" such as:

00:10:57.048 error: DIE has a start address whose LowPC is between the line table Row[9006] with address 0x00000000033aaf17 and the next one.
00:10:57.048 
00:10:57.048 0x0ca2fa30: DW_TAG_subprogram
00:10:57.048               DW_AT_abstract_origin	(0x0ca2fa0b "_ZN5boost15program_options20invalid_option_valueC4ERKSbIwSt11char_traitsIwESaIwEE")
00:10:57.048               DW_AT_MIPS_linkage_name	("_ZN5boost15program_options20invalid_option_valueC2ERKSbIwSt11char_traitsIwESaIwEE")
00:10:57.048               DW_AT_object_pointer	(0x0ca2fa49)
00:10:57.048               DW_AT_ranges	(0x009bbe70
00:10:57.048                  [0x000000000ae3a740, 0x000000000ae3ab29)
00:10:57.048                  [0x00000000033aaf20, 0x00000000033aaff5))
00:10:57.048               DW_AT_frame_base	(DW_OP_call_frame_cfa)
00:10:57.048               DW_AT_GNU_all_call_sites	(0x01)
00:10:57.048               DW_AT_sibling	(0x0ca3288d)
00:10:57.048 error: DIE has a start address whose LowPC is between the line table Row[9368] with address 0x00000000033ab1fb and the next one.

I am not sure if this was just accidentally called "error" instead of "warning", and we should suppress that as well with --quiet, or if this is rather a more severe issue, which should not be silenced. I guess in that case, we would probably update the compiler version to fix that (assuming it is fixed in a more recent gcc version).

This is an valid error. The compiler or linker has messed up and the DWARF is broken and this really should remain an error. Normally when you have a function, you should have a line entry for the first address in function. So if you have a function named "A" whose address range is [0x1000-0x2000) you would expect an line table entry for address 0x1000. In this error case we have a line table that looks like:

0x0000FFF0: main.cpp:12
0x00001010: main.cpp:13

So something serious went wrong in the compiler, LTO, or linker where they didn't modify the DWARF correctly. So this really should remain an error so that compiler and linker people can see this and really try to fix it.

[simon.giesecke]

In D102829#2782856, @clayborg wrote:

In D102829#2781201, @simon.giesecke wrote:

@clayborg I am not sure what it means you now accepted the revision? I am still not sure what to do about the tests.

I will need to add tests for these warnings since they don't already exist. These kinds of tests require a deep knowledge of making DWARF that contains each of these warnings, so I won't have you do this.

Ok, thanks! Then I will push my patch.

One more thing is that I suppress everything that's called a "warning" here. I tested it, and on our release/optimized binaries (built with gcc) we get lots of "errors" such as:

00:10:57.048 error: DIE has a start address whose LowPC is between the line table Row[9006] with address 0x00000000033aaf17 and the next one.
00:10:57.048 
00:10:57.048 0x0ca2fa30: DW_TAG_subprogram
00:10:57.048               DW_AT_abstract_origin	(0x0ca2fa0b "_ZN5boost15program_options20invalid_option_valueC4ERKSbIwSt11char_traitsIwESaIwEE")
00:10:57.048               DW_AT_MIPS_linkage_name	("_ZN5boost15program_options20invalid_option_valueC2ERKSbIwSt11char_traitsIwESaIwEE")
00:10:57.048               DW_AT_object_pointer	(0x0ca2fa49)
00:10:57.048               DW_AT_ranges	(0x009bbe70
00:10:57.048                  [0x000000000ae3a740, 0x000000000ae3ab29)
00:10:57.048                  [0x00000000033aaf20, 0x00000000033aaff5))
00:10:57.048               DW_AT_frame_base	(DW_OP_call_frame_cfa)
00:10:57.048               DW_AT_GNU_all_call_sites	(0x01)
00:10:57.048               DW_AT_sibling	(0x0ca3288d)
00:10:57.048 error: DIE has a start address whose LowPC is between the line table Row[9368] with address 0x00000000033ab1fb and the next one.

I am not sure if this was just accidentally called "error" instead of "warning", and we should suppress that as well with --quiet, or if this is rather a more severe issue, which should not be silenced. I guess in that case, we would probably update the compiler version to fix that (assuming it is fixed in a more recent gcc version).

This is an valid error. The compiler or linker has messed up and the DWARF is broken and this really should remain an error. Normally when you have a function, you should have a line entry for the first address in function. So if you have a function named "A" whose address range is [0x1000-0x2000) you would expect an line table entry for address 0x1000. In this error case we have a line table that looks like:

0x0000FFF0: main.cpp:12
0x00001010: main.cpp:13

So something serious went wrong in the compiler, LTO, or linker where they didn't modify the DWARF correctly. So this really should remain an error so that compiler and linker people can see this and really try to fix it.

Yeah, makes sense that this is a real issue. I can reproduce such errors with a RelWithDebInfo build of ninja-build using gcc 8.3.1, 9.3.1 and 10.2.1 (from CentOS 7 devtoolset-* packages), with or without LTO, using both gold and lld (I didn't try all combinations), with varying number of errors. e.g.

$ CC=/opt/rh/devtoolset-10/root/bin/gcc CXX=/opt/rh/devtoolset-10/root/bin/g++ cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -B build-gcc10 -G Ninja
[...]
$ cmake --build build-gcc10/
[...]
$ llvm-gsymutil --quiet --convert build-gcc10/ninja
Input file: build-gcc10/ninja
Output file (x86_64): build-gcc10/ninja.gsym
error: DIE has a start address whose LowPC is between the line table Row[2448] with address 0x0000000000404aea and the next one.

0x0001fea8: DW_TAG_subprogram
              DW_AT_abstract_origin	(0x00016646 "_ZNSt10_HashtableI11StringPieceSt4pairIKS0_PN8BuildLog8LogEntryEESaIS6_ENSt8__detail10_Select1stESt8equal_toIS0_ESt4hashIS0_ENS8_18_Mod_range_hashingENS8_20_Default_ranged_hashENS8_20_Prime_rehash_policyENS8_17_Hashtable_traitsILb1ELb0ELb1EEEE10_M_emplaceIJS6_EEES1_INS8_14_Node_iteratorIS6_Lb0ELb1EEEbESt17integral_constantIbLb1EEDpOT_")
              DW_AT_ranges	(0x00001e00
                 [0x0000000000407a60, 0x0000000000407d61)
                 [0x0000000000404af6, 0x0000000000404b22))
              DW_AT_frame_base	(DW_OP_call_frame_cfa)
              DW_AT_GNU_all_call_sites	(true)
              DW_AT_sibling	(0x00021223)
error: DIE has a start address whose LowPC is between the line table Row[701] with address 0x0000000000405460 and the next one.

0x0015068e: DW_TAG_subprogram
              DW_AT_specification	(0x0014dd01 "_ZN13DiskInterface8MakeDirsERKSs")
              DW_AT_object_pointer	(0x001506a1)
              DW_AT_ranges	(0x00022500
                 [0x0000000000414e30, 0x0000000000415018)
                 [0x000000000040546c, 0x0000000000405486))
              DW_AT_frame_base	(DW_OP_call_frame_cfa)
              DW_AT_GNU_all_call_sites	(true)
              DW_AT_sibling	(0x00151687)
error: DIE has a start address whose LowPC is between the line table Row[2462] with address 0x000000000040553a and the next one.

[...]

llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h

class GsymCreator {
   StringSet<> StringStorage;
   DenseMap<llvm::gsym::FileEntry, uint32_t> FileEntryToIndex;
   std::vector<llvm::gsym::FileEntry> Files;
   std::vector<uint8_t> UUID;
   Optional<AddressRanges> ValidTextRanges;
   AddressRanges Ranges;
   llvm::Optional<uint64_t> BaseAddress;
   bool Finalized = false;
+  bool Quiet;
 
 public:
-
-  GsymCreator();
+  GsymCreator(bool Quiet = false);
 
   /// Save a GSYM file to a stand alone file.
   ///
   /// \param Path The file path to save the GSYM file to.
   /// \param ByteOrder The endianness to use when saving the file.
   /// \returns An error object that indicates success or failure of the save.
   llvm::Error save(StringRef Path, llvm::support::endianness ByteOrder) const;
 
public:
   /// and allows symbolication without needing to find the original base
   /// address in the original object file.
   ///
   /// \param  Addr The address to use as the base address of the GSYM file
   ///              when it is saved to disk.
   void setBaseAddress(uint64_t Addr) {
     BaseAddress = Addr;
   }
+
+  /// Whether the transformation should be quiet, i.e. not output warnings.
+  bool isQuiet() const { return Quiet; }
 };
 
 } // namespace gsym
 } // namespace llvm
 
 #endif // LLVM_DEBUGINFO_GSYM_GSYMCREATOR_H

llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp

if (RowIndex != RowVector[0] && Row.Address < PrevRow.Address) {
       // We have seen full duplicate line tables for functions in some
       // DWARF files. Watch for those here by checking the the last
       // row was the function's end address (HighPC) and that the
       // current line table entry's address is the same as the first
       // 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 (!Gsym.isQuiet()) {
           Log << "warning: duplicate line table detected for DIE:\n";
           Die.dump(Log, 0, DIDumpOptions::getForSingleDIE());
+        }
       } else {
         // Print out (ignore if os == nulls as this is expensive)
         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());
for (const DWARFAddressRange &Range : Ranges) {
       // 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
         // and the debug info wasn't able to be stripped from the DWARF. If
         // the LowPC isn't zero or -1, then we should emit an error.
         if (Range.LowPC != 0) {
-          // Unexpected invalid address, emit an error
+          if (!Gsym.isQuiet()) {
+            // Unexpected invalid address, emit a warning
             Log << "warning: DIE has an address range whose start address is "
-              "not in any executable sections (" <<
-              *Gsym.GetValidTextRanges() << ") and will not be processed:\n";
+                   "not in any executable sections ("
+                << *Gsym.GetValidTextRanges()
+                << ") and will not be processed:\n";
             Die.dump(Log, 0, DIDumpOptions::getForSingleDIE());
           }
+        }
         break;
       }
 
       FunctionInfo FI;
       FI.setStartAddress(Range.LowPC);
       FI.setEndAddress(Range.HighPC);
       FI.Name = *NameIndex;
       if (CUI.LineTable) {

llvm/lib/DebugInfo/GSYM/GsymCreator.cpp

 #include <algorithm>
 #include <cassert>
 #include <functional>
 #include <vector>
 
 using namespace llvm;
 using namespace gsym;
 
-GsymCreator::GsymCreator() : StrTab(StringTableBuilder::ELF) {
+GsymCreator::GsymCreator(bool Quiet)
+    : StrTab(StringTableBuilder::ELF), Quiet(Quiet) {
   insertFile(StringRef());
 }
 
 uint32_t GsymCreator::insertFile(StringRef Path, llvm::sys::path::Style Style) {
   llvm::StringRef directory = llvm::sys::path::parent_path(Path, Style);
   llvm::StringRef filename = llvm::sys::path::filename(Path, Style);
   // We must insert the strings first, then call the FileEntry constructor.
   // If we inline the insertString() function call into the constructor, the
Funcs.erase(
                              return true;
                            } else {
                              if (!Prev.hasRichInfo() && Curr.hasRichInfo()) {
                                // Same address range, one with no debug info
                                // (symbol) and the next with debug info. Keep
                                // the latter.
                                return true;
                              } else {
+                               if (!Quiet) {
                                  OS << "warning: same address range contains "
                                        "different debug "
                                     << "info. Removing:\n"
                                     << Prev << "\nIn favor of this one:\n"
                                     << Curr << "\n";
+                               }
                                return true;
                              }
                            }
                          } else {
-                           // print warnings about overlaps
+                           if (!Quiet) { // print warnings about overlaps
                              OS << "warning: function ranges overlap:\n"
                                 << Prev << "\n"
                                 << Curr << "\n";
                            }
+                         }
                        } else if (Prev.Range.size() == 0 &&
                                   Curr.Range.contains(Prev.Range.Start)) {
+                         if (!Quiet) {
                            OS << "warning: removing symbol:\n"
                               << Prev << "\nKeeping:\n"
                               << Curr << "\n";
+                         }
                          return true;
                        }
 
                        return false;
                      }),
       Funcs.end());
 
   // If our last function info entry doesn't have a size and if we have valid

llvm/test/tools/llvm-gsymutil/cmdline.test

 RUN: llvm-gsymutil -h 2>&1 | FileCheck --check-prefix=HELP %s
 RUN: llvm-gsymutil --help 2>&1 | FileCheck --check-prefix=HELP %s
 HELP: OVERVIEW: A tool for dumping, searching and creating GSYM files.
 HELP: USAGE: llvm-gsymutil{{[^ ]*}} [options] <input GSYM files>
 HELP: OPTIONS:
 HELP: Conversion Options:
 HELP: --arch=<arch>
 HELP: --convert=<path>
 HELP: --num-threads=<n>
 HELP: --out-file=<path>
+HELP: --quiet
 HELP: --verify
 HELP: Generic Options:
 HELP: --help
 HELP: --version
 HELP: Lookup Options:
 HELP: --address=<addr>
 HELP: --addresses-from-stdin
 HELP: Options:
 HELP: --verbose
 
 RUN: llvm-gsymutil --version 2>&1 | FileCheck --check-prefix=VERSION %s
 VERSION: {{ version }}
 
 RUN: not llvm-gsymutil --addresses-from-stdin --address 0x12345678 | FileCheck --check-prefix=INCOMPATIBLE %s
 RUN: not llvm-gsymutil --addresses-from-stdin llvm-gsymutil | FileCheck --check-prefix=INCOMPATIBLE %s
 INCOMPATIBLE: error: no input files or addresses can be specified when using the --addresses-from-stdin option.

llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp

 
 static opt<unsigned>
     NumThreads("num-threads",
                desc("Specify the maximum number (n) of simultaneous threads "
                     "to use when converting files to GSYM.\nDefaults to the "
                     "number of cores on the current machine."),
                cl::value_desc("n"), cat(ConversionOptions));
 
+static opt<bool>
+    Quiet("quiet", desc("Do not output warnings about the debug information"),
+          cat(ConversionOptions));
+
 static list<uint64_t> LookupAddresses("address",
                                       desc("Lookup an address in a GSYM file"),
                                       cl::value_desc("addr"),
                                       cat(LookupOptions));
 
 static opt<bool> LookupAddressesFromStdin(
     "addresses-from-stdin",
     desc("Lookup addresses in a GSYM file that are read from stdin\nEach input "

 }
 
 static llvm::Error handleObjectFile(ObjectFile &Obj,
                                     const std::string &OutFile) {
   auto ThreadCount =
       NumThreads > 0 ? NumThreads : std::thread::hardware_concurrency();
   auto &OS = outs();
 
-  GsymCreator Gsym;
+  GsymCreator Gsym(Quiet);
 
   // 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
   // symbolication since clients can slide the GSYM lookup addresses by using
   // the load bias of the shared library.
   if (auto ImageBaseAddr = getImageBaseAddress(Obj))
     Gsym.setBaseAddress(*ImageBaseAddr);