Modify llvm-gsymutil lookups to handle overlapping ranges correctly. (#…

…72350)

llvm-gsymutil allows address ranges to overlap. There was a bug where if
we had debug info for a function with a range like [0x100-0x200) and a
symbol at the same start address yet with a larger range like
[0x100-0x300), we would randomly get either only information from the
first or second entry. This could cause lookups to fail due to the way
the binary search worked.

This patch makes sure that when lookups happen we find the first address
table entry that can match an address, and also ensures that we always
select the first FunctionInfo that could match. FunctionInfo entries are
sorted such that the most debug info rich entries come first. And if we
have two ranges that have the same start address, the smaller range
comes first and the larger one comes next. This patch also adds the
ability to iterate over all function infos with the same start address
to always find a range that contains the address.

Added a unit test to test this functionality that failed prior to this
fix and now succeeds.

Also fix an issue when dumping an entire GSYM file that has duplicate address entries where it used to always print out the binary search match for the FunctionInfo, not the actual data for the address index.

llvm/include/llvm/DebugInfo/GSYM/GsymReader.h

@@ -106,6 +106,15 @@ class GsymReader {
   /// address.
   llvm::Expected<FunctionInfo> getFunctionInfo(uint64_t Addr) const;
 
+  /// Get the full function info given an address index.
+  ///
+  /// \param AddrIdx A address index for an address in the address table.
+  ///
+  /// \returns An expected FunctionInfo that contains the function info object
+  /// or an error object that indicates reason for failing get the function
+  /// info object.
+  llvm::Expected<FunctionInfo> getFunctionInfoAtIndex(uint64_t AddrIdx) const;
+
   /// Lookup an address in the a GSYM.
   ///
   /// Lookup just the information needed for a specific address \a Addr. This
@@ -266,6 +275,19 @@ class GsymReader {
       return std::nullopt;
     if (Iter == End || AddrOffset < *Iter)
       --Iter;
+
+    // GSYM files have sorted function infos with the most information (line
+    // table and/or inline info) first in the array of function infos, so
+    // always backup as much as possible as long as the address offset is the
+    // same as the previous entry.
+    while (Iter != Begin) {
+      auto Prev = Iter - 1;
+      if (*Prev == *Iter)
+        Iter = Prev;
+      else
+        break;
+    }
+
     return std::distance(Begin, Iter);
   }
 
@@ -303,6 +325,38 @@ class GsymReader {
   /// \returns An optional GSYM data offset for the offset of the FunctionInfo
   /// that needs to be decoded.
   std::optional<uint64_t> getAddressInfoOffset(size_t Index) const;
+
+  /// Given an address, find the correct function info data and function
+  /// address.
+  ///
+  /// Binary search the address table and find the matching address info
+  /// and make sure that the function info contains the address. GSYM allows
+  /// functions to overlap, and the most debug info is contained in the first
+  /// entries due to the sorting when GSYM files are created. We can have
+  /// multiple function info that start at the same address only if their
+  /// address range doesn't match. So find the first entry that matches \a Addr
+  /// and iterate forward until we find one that contains the address.
+  ///
+  /// \param[in] Addr A virtual address that matches the original object file
+  /// to lookup.
+  ///
+  /// \param[out] FuncStartAddr A virtual address that is the base address of
+  /// the function that is used for decoding the FunctionInfo.
+  ///
+  /// \returns An valid data extractor on success, or an error if we fail to
+  /// find the address in a function info or corrrectly decode the data
+  llvm::Expected<llvm::DataExtractor>
+  getFunctionInfoDataForAddress(uint64_t Addr, uint64_t &FuncStartAddr) const;
+
+  /// Get the function data and address given an address index.
+  ///
+  /// \param AddrIdx A address index from the address table.
+  ///
+  /// \returns An expected FunctionInfo that contains the function info object
+  /// or an error object that indicates reason for failing to lookup the
+  /// address.
+  llvm::Expected<llvm::DataExtractor>
+  getFunctionInfoDataAtIndex(uint64_t AddrIdx, uint64_t &FuncStartAddr) const;
 };
 
 } // namespace gsym

llvm/lib/DebugInfo/GSYM/GsymReader.cpp

@@ -253,49 +253,94 @@ GsymReader::getAddressIndex(const uint64_t Addr) const {
 
 }
 
-llvm::Expected<FunctionInfo> GsymReader::getFunctionInfo(uint64_t Addr) const {
-  Expected<uint64_t> AddressIndex = getAddressIndex(Addr);
-  if (!AddressIndex)
-    return AddressIndex.takeError();
-  // Address info offsets size should have been checked in parse().
-  assert(*AddressIndex < AddrInfoOffsets.size());
-  auto AddrInfoOffset = AddrInfoOffsets[*AddressIndex];
-  assert(
-      (Endian == llvm::endianness::big || Endian == llvm::endianness::little) &&
-      "Endian must be either big or little");
-  DataExtractor Data(MemBuffer->getBuffer().substr(AddrInfoOffset),
-                     Endian == llvm::endianness::little, 4);
-  if (std::optional<uint64_t> OptAddr = getAddress(*AddressIndex)) {
-    auto ExpectedFI = FunctionInfo::decode(Data, *OptAddr);
-    if (ExpectedFI) {
-      if (ExpectedFI->Range.contains(Addr) || ExpectedFI->Range.size() == 0)
-        return ExpectedFI;
-      return createStringError(std::errc::invalid_argument,
-                                "address 0x%" PRIx64 " is not in GSYM", Addr);
+llvm::Expected<DataExtractor>
+GsymReader::getFunctionInfoDataForAddress(uint64_t Addr,
+                                          uint64_t &FuncStartAddr) const {
+  Expected<uint64_t> ExpectedAddrIdx = getAddressIndex(Addr);
+  if (!ExpectedAddrIdx)
+    return ExpectedAddrIdx.takeError();
+  const uint64_t FirstAddrIdx = *ExpectedAddrIdx;
+  // The AddrIdx is the first index of the function info entries that match
+  // \a Addr. We need to iterate over all function info objects that start with
+  // the same address until we find a range that contains \a Addr.
+  std::optional<uint64_t> FirstFuncStartAddr;
+  const size_t NumAddresses = getNumAddresses();
+  for (uint64_t AddrIdx = FirstAddrIdx; AddrIdx < NumAddresses; ++AddrIdx) {
+    auto ExpextedData = getFunctionInfoDataAtIndex(AddrIdx, FuncStartAddr);
+    // If there was an error, return the error.
+    if (!ExpextedData)
+      return ExpextedData;
+
+    // Remember the first function start address if it hasn't already been set.
+    // If it is already valid, check to see if it matches the first function
+    // start address and only continue if it matches.
+    if (FirstFuncStartAddr.has_value()) {
+      if (*FirstFuncStartAddr != FuncStartAddr)
+        break; // Done with consecutive function entries with same address.
+    } else {
+      FirstFuncStartAddr = FuncStartAddr;
     }
+    // Make sure the current function address ranges contains \a Addr.
+    // Some symbols on Darwin don't have valid sizes, so if we run into a
+    // symbol with zero size, then we have found a match for our address.
+
+    // The first thing the encoding of a FunctionInfo object is the function
+    // size.
+    uint64_t Offset = 0;
+    uint32_t FuncSize = ExpextedData->getU32(&Offset);
+    if (FuncSize == 0 ||
+        AddressRange(FuncStartAddr, FuncStartAddr + FuncSize).contains(Addr))
+      return ExpextedData;
   }
   return createStringError(std::errc::invalid_argument,
-                           "failed to extract address[%" PRIu64 "]",
-                           *AddressIndex);
+                           "address 0x%" PRIx64 " is not in GSYM", Addr);
+}
+
+llvm::Expected<DataExtractor>
+GsymReader::getFunctionInfoDataAtIndex(uint64_t AddrIdx,
+                                       uint64_t &FuncStartAddr) const {
+  if (AddrIdx >= getNumAddresses())
+    return createStringError(std::errc::invalid_argument,
+                             "invalid address index %" PRIu64, AddrIdx);
+  const uint32_t AddrInfoOffset = AddrInfoOffsets[AddrIdx];
+  assert((Endian == endianness::big || Endian == endianness::little) &&
+         "Endian must be either big or little");
+  StringRef Bytes = MemBuffer->getBuffer().substr(AddrInfoOffset);
+  if (Bytes.empty())
+    return createStringError(std::errc::invalid_argument,
+                             "invalid address info offset 0x%" PRIx32,
+                             AddrInfoOffset);
+  std::optional<uint64_t> OptFuncStartAddr = getAddress(AddrIdx);
+  if (!OptFuncStartAddr)
+    return createStringError(std::errc::invalid_argument,
+                             "failed to extract address[%" PRIu64 "]", AddrIdx);
+  FuncStartAddr = *OptFuncStartAddr;
+  return DataExtractor(Bytes, Endian == llvm::endianness::little, 4);
+}
+
+llvm::Expected<FunctionInfo> GsymReader::getFunctionInfo(uint64_t Addr) const {
+  uint64_t FuncStartAddr = 0;
+  if (auto ExpectedData = getFunctionInfoDataForAddress(Addr, FuncStartAddr))
+    return FunctionInfo::decode(*ExpectedData, FuncStartAddr);
+  else
+    return ExpectedData.takeError();
+}
+
+llvm::Expected<FunctionInfo>
+GsymReader::getFunctionInfoAtIndex(uint64_t Idx) const {
+  uint64_t FuncStartAddr = 0;
+  if (auto ExpectedData = getFunctionInfoDataAtIndex(Idx, FuncStartAddr))
+    return FunctionInfo::decode(*ExpectedData, FuncStartAddr);
+  else
+    return ExpectedData.takeError();
 }
 
 llvm::Expected<LookupResult> GsymReader::lookup(uint64_t Addr) const {
-  Expected<uint64_t> AddressIndex = getAddressIndex(Addr);
-  if (!AddressIndex)
-    return AddressIndex.takeError();
-  // Address info offsets size should have been checked in parse().
-  assert(*AddressIndex < AddrInfoOffsets.size());
-  auto AddrInfoOffset = AddrInfoOffsets[*AddressIndex];
-  assert(
-      (Endian == llvm::endianness::big || Endian == llvm::endianness::little) &&
-      "Endian must be either big or little");
-  DataExtractor Data(MemBuffer->getBuffer().substr(AddrInfoOffset),
-                     Endian == llvm::endianness::little, 4);
-  if (std::optional<uint64_t> OptAddr = getAddress(*AddressIndex))
-    return FunctionInfo::lookup(Data, *this, *OptAddr, Addr);
-  return createStringError(std::errc::invalid_argument,
-                           "failed to extract address[%" PRIu64 "]",
-                           *AddressIndex);
+  uint64_t FuncStartAddr = 0;
+  if (auto ExpectedData = getFunctionInfoDataForAddress(Addr, FuncStartAddr))
+    return FunctionInfo::lookup(*ExpectedData, *this, FuncStartAddr, Addr);
+  else
+    return ExpectedData.takeError();
 }
 
 void GsymReader::dump(raw_ostream &OS) {
@@ -346,7 +391,7 @@ void GsymReader::dump(raw_ostream &OS) {
 
   for (uint32_t I = 0; I < Header.NumAddresses; ++I) {
     OS << "FunctionInfo @ " << HEX32(AddrInfoOffsets[I]) << ": ";
-    if (auto FI = getFunctionInfo(*getAddress(I)))
+    if (auto FI = getFunctionInfoAtIndex(I))
       dump(OS, *FI);
     else
       logAllUnhandledErrors(FI.takeError(), OS, "FunctionInfo:");