[lldb] Limit 8b259fe573e1 to dSYMs

[JDevlieghere]

Summary

Limit trusting the arange accelerator tables to dSYMs only, and not any debug info object file.

[JDevlieghere]

typo

[aprantl]

Should this be true for a fully linked ELF executable, too?

lldb/include/lldb/Symbol/ObjectFile.h

685

Doxygen comment?

[JDevlieghere]

In D141330#4037925, @aprantl wrote:

Should this be true for a fully linked ELF executable, too?

Sounds reasonable, but adding @labath and @DavidSpickett to confirm. This is trivial to extend later.

[JDevlieghere]

Address @aprantl's feedback

[aprantl]

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

6098

Can you also add a comment here explaining why we only trust a dSYM?

[jasonmolenda]

Yeah I'd add a comment explaining that this returns true if the DWARF dwarf_aranges accelerator table can be trusted to be complete by lldb. which it is with dsymutil created dSYM files. Otherwise lldb will confirm that every CU mentioned in the debug_info is included in dwarf_aranges, and for any that are not, it will scan the debug_info for that compile unit looking to make sure there aren't any subprograms that were omitted. Anyway, LGTM.

[labath]

In D141330#4037932, @JDevlieghere wrote:

In D141330#4037925, @aprantl wrote:

Should this be true for a fully linked ELF executable, too?

Sounds reasonable, but adding @labath and @DavidSpickett to confirm. This is trivial to extend later.

It sounds like this is depends on whoever produced the file, not on the file format.

[jasonmolenda]

In D141330#4043397, @labath wrote:

In D141330#4037932, @JDevlieghere wrote:

In D141330#4037925, @aprantl wrote:

Should this be true for a fully linked ELF executable, too?

Sounds reasonable, but adding @labath and @DavidSpickett to confirm. This is trivial to extend later.

It sounds like this is depends on whoever produced the file, not on the file format.

It's not even the producer (compiler/assembler) - it's the linker (or whatever links the dwarf) that needs to generate the table, and we can't detect that from the debug info.

We need (1) a guarantee that every compile unit which generated code in the debug_info has an entry in debug_aranges, (2) a guarantee that all of the address ranges that come from the CU are described, and (3) a guarantee that the ranges in debug_aranges are unique, that no address will map to multiple compile units. The description in the dwarf v5 doc includes,

By scanning the table, a debugger can quickly decide which compilation unit to look in to find the debugging information for an object that has a given address.

If the range of addresses covered by the text and/or data of a compilation unit is not contiguous, then there may be multiple address range descriptors for that compilation unit.

(I dislike it when the standard says "there MAY be multiple address range descriptors" -- does this mean if I have a noncontiguous CUs A and B interleaved in the final binary, the debug_aranges can overlap?)

There's no guarantee for (1) if a debug_aranges table is present, but maybe we can simply assume that any producer producing debug_aranges has done so comprehensively. I couldn't imagine why it wouldn't be that way.

The point of this patch, of course, is to skip the verification of (1) - that every CU that generated functions in the final binary has a debug_aranges entry. It turns out we have compile units in debug_info that don't emit any functions, and lldb would see those as missing from debug_aranges and iterate over the DIEs looking for subprograms.

Maybe lldb should simply trust that if debug_aranges exists, all of 1-3 are true, and revisit it when/if we get bug reports of some random toolchain in the world violating that assumption.

[labath]

Just to clarify: I was responding to the question about the ELF file usage, not questioning the overall approach. Given that MachO has a special code for dsym files, I think it makes sense to use it.

In D141330#4043807, @jasonmolenda wrote:

By scanning the table, a debugger can quickly decide which compilation unit to look in to find the debugging information for an object that has a given address.

If the range of addresses covered by the text and/or data of a compilation unit is not contiguous, then there may be multiple address range descriptors for that compilation unit.

(I dislike it when the standard says "there MAY be multiple address range descriptors" -- does this mean if I have a noncontiguous CUs A and B interleaved in the final binary, the debug_aranges can overlap?)

I see what you mean, but I wouldn't really read it that way, and I hope nobody produces files like that.

There's no guarantee for (1) if a debug_aranges table is present, but maybe we can simply assume that any producer producing debug_aranges has done so comprehensively. I couldn't imagine why it wouldn't be that way.

We can't do that because, as I alluded to in the other comment, the aranges section can be produced by the compiler, and there's no guarantee that all CUs will be built with the same flags (so some may be missing that section). In the elf world a linker will just concatenate those sections without trying to fill in the blanks.

A normal binary includes some code from the system runtime (which is built by the OS vendor not the user). Typically, that code will not have debug info, but sometimes it does, and in that case it's pretty much guaranteed that it will be built using different compiler/flags than the user code.

lldb/include/lldb/Symbol/ObjectFile.h

public:
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
   virtual llvm::StringRef
   StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const {
     return symbol_name;
   }
 
+  /// Can we trust the address ranges accelerator associated with this object
+  /// file to be complete.
+  virtual bool CanTrustAddressRanges() { return false; }
+
   static lldb::SymbolType GetSymbolTypeFromName(
       llvm::StringRef name,
       lldb::SymbolType symbol_type_hint = lldb::eSymbolTypeUndefined);
 
   /// Loads this objfile to memory.
   ///
   /// Loads the bits needed to create an executable image to the memory. It is
   /// useful with bare-metal targets where target does not have the ability to

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h

public:
   llvm::VersionTuple GetVersion() override;
 
   llvm::VersionTuple GetMinimumOSVersion() override;
 
   llvm::VersionTuple GetSDKVersion() override;
 
   bool GetIsDynamicLinkEditor() override;
 
+  bool CanTrustAddressRanges() override;
+
   static bool ParseHeader(lldb_private::DataExtractor &data,
                           lldb::offset_t *data_offset_ptr,
                           llvm::MachO::mach_header &header);
 
   bool AllowAssemblyEmulationUnwindPlans() override;
 
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

llvm::VersionTuple ObjectFileMachO::GetSDKVersion() {
 
   return *m_sdk_versions;
 }
 
 bool ObjectFileMachO::GetIsDynamicLinkEditor() {
   return m_header.filetype == llvm::MachO::MH_DYLINKER;
 }
 
+bool ObjectFileMachO::CanTrustAddressRanges() {
+  // Dsymutil guarantees that the .debug_aranges accelerator is complete and can
+  // be trusted by LLDB.
+  return m_header.filetype == llvm::MachO::MH_DSYM;
+}
+
 bool ObjectFileMachO::AllowAssemblyEmulationUnwindPlans() {
   return m_allow_assembly_emulation_unwind_plans;
 }
 
 Section *ObjectFileMachO::GetMachHeaderSection() {
   // Find the first address of the mach header which is the first non-zero file
   // sized section whose file offset is zero. This is the base file address of
   // the mach-o file which can be subtracted from the vmaddr of the other

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp

const DWARFDebugAranges &DWARFDebugInfo::GetCompileUnitAranges() {
   std::set<dw_offset_t> cus_with_data;
   for (size_t n = 0; n < m_cu_aranges_up->GetNumRanges(); n++) {
     dw_offset_t offset = m_cu_aranges_up->OffsetAtIndex(n);
     if (offset != DW_INVALID_OFFSET)
       cus_with_data.insert(offset);
   }
 
   // Manually build arange data for everything that wasn't in .debug_aranges.
-  // Skip this step for dSYMs as we can trust dsymutil to have emitted complete
-  // aranges.
-  const bool is_dsym =
-      m_dwarf.GetObjectFile() &&
-      m_dwarf.GetObjectFile()->GetType() == ObjectFile::eTypeDebugInfo;
-  if (!is_dsym) {
+  // The .debug_aranges accelerator is not guaranteed to be complete.
+  // Tools such as dsymutil can provide stronger guarantees than required by the
+  // standard. Without that guarantee, we have to iterate over every CU in the
+  // .debug_info and make sure there's a corresponding entry in the table and if
+  // not, add one for every subprogram.
+  ObjectFile *OF = m_dwarf.GetObjectFile();
+  if (!OF || !OF->CanTrustAddressRanges()) {
     const size_t num_units = GetNumUnits();
     for (size_t idx = 0; idx < num_units; ++idx) {
       DWARFUnit *cu = GetUnitAtIndex(idx);
 
       dw_offset_t offset = cu->GetOffset();
       if (cus_with_data.find(offset) == cus_with_data.end())
         cu->BuildAddressRangeTable(m_cu_aranges_up.get());
     }