Fix a case where multiple symbols with zero size would cause duplicate entries in gsym files.

[clayborg]

Summary

Symbol tables can have symbols with no size in mach-o files that were failing to get combined into a single entry. This resulted in many duplicate entries for the same address and made gsym files larger.

[wallace]

makes sense

[wallace]

btw, did you consider changing Range::intersects function so that it checks the case of empty ranges?

[clayborg]

In D105068#2845890, @wallace wrote:

btw, did you consider changing Range::intersects function so that it checks the case of empty ranges?

Yes, but I would not consider any empty address range to be able to intersect with anything else, so I opted not to modify it.

llvm/lib/DebugInfo/GSYM/GsymCreator.cpp

llvm::Error GsymCreator::finalize(llvm::raw_ostream &OS) {
   //
   // Note that in case of (b), we cannot include Y in the result because then
   // we wouldn't find any function for range (end of Y, end of X)
   // with binary search
   auto NumBefore = Funcs.size();
   Funcs.erase(
       removeIfBinary(Funcs.begin(), Funcs.end(),
                      [&](const auto &Prev, const auto &Curr) {
-                       if (Prev.Range.intersects(Curr.Range)) {
-                         // Overlapping address ranges.
-                         if (Prev.Range == Curr.Range) {
+                       // Empty ranges won't intersect, but we still need to
+                       // catch the case where we have multiple symbols at the
+                       // same address and coalesce them.
+                       const bool ranges_equal = Prev.Range == Curr.Range;
+                       if (ranges_equal || Prev.Range.intersects(Curr.Range)) {
+                         // Overlapping ranges or empty identical ranges.
+                         if (ranges_equal) {
                            // Same address range. Check if one is from debug
                            // info and the other is from a symbol table. If
                            // so, then keep the one with debug info. Our
                            // sorting guarantees that entries with matching
                            // address ranges that have debug info are last in
                            // the sort.
                            if (Prev == Curr) {
                              // FunctionInfo entries match exactly (range,

llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp

TEST(GSYMTest, TestDWARFDeadStripAddr8) {
   // Test that the only function that made it was the "main" function.
   EXPECT_EQ(GR->getNumAddresses(), 1u);
   auto ExpFI = GR->getFunctionInfo(0x1000);
   ASSERT_THAT_EXPECTED(ExpFI, Succeeded());
   ASSERT_EQ(ExpFI->Range, AddressRange(0x1000, 0x2000));
   StringRef MethodName = GR->getString(ExpFI->Name);
   EXPECT_EQ(MethodName, "main");
 }
+
+TEST(GSYMTest, TestGsymCreatorMultipleSymbolsWithNoSize) {
+  // Multiple symbols at the same address with zero size were being emitted
+  // instead of being combined into a single entry. This function tests to make
+  // sure we only get one symbol.
+  uint8_t UUID[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
+  GsymCreator GC;
+  GC.setUUID(UUID);
+  constexpr uint64_t BaseAddr = 0x1000;
+  constexpr uint8_t AddrOffSize = 1;
+  const uint32_t Func1Name = GC.insertString("foo");
+  const uint32_t Func2Name = GC.insertString("bar");
+  GC.addFunctionInfo(FunctionInfo(BaseAddr, 0, Func1Name));
+  GC.addFunctionInfo(FunctionInfo(BaseAddr, 0, Func2Name));
+  Error Err = GC.finalize(llvm::nulls());
+  ASSERT_FALSE(Err);
+  TestEncodeDecode(GC, llvm::support::little, GSYM_VERSION, AddrOffSize,
+                   BaseAddr,
+                   1, // NumAddresses
+                   ArrayRef<uint8_t>(UUID));
+  TestEncodeDecode(GC, llvm::support::big, GSYM_VERSION, AddrOffSize, BaseAddr,
+                   1, // NumAddresses
+                   ArrayRef<uint8_t>(UUID));
+}