Explicitly pass a module to register Xcode SDKs with to ParseXcodeSDK. Was: Add an API that allows iterating over a debug map's OSO's Modules

[aprantl]

Summary

Based on Pavel's feedback, here is a different approach, where the "master" debug map module is passed in explicitly to ParseXcodeSDK(). This allows updating both the source remapping dictionary of the OSO (needed for source listings) and the debug map (needed to remap Clang compiler options in swift-lldb).

For Swift LLDB (but potentially also for module support in Clang-land) I need a way to accumulate the path remappings produced by Module::RegisterXcodeSDK(). Unfortunately there is no way to go from an executable's Module to the auxiliary modules that represent the individual OSO object files of a SymbolFileDWARFDebugMap.

Here is a proposed API that would allow me to do this. Does this look reasonable?

[labath]

My understanding is that these OSO modules are supposed to be a hidden implementation detail of SymbolFileDWARF(DebugMap) and that they should not leak out the main module. For example, we make sure to always rewrite any addresses that come out of the oso modules to be in terms of the main module, and ensure that parsed dwarf compile units are assigned to the CompileUnits belonging to the main module, etc.

In that world, the right thing to do would be to ensure that SymbolFileDWARF(DebugMap) accumulates the appropriate sdk information in the main module.

[aprantl]

Completely different implementation :-)

[labath]

Well... this is closer, but we should get rid of the Module argument. It's not consistent with the other SymbolFile functions, and also not necessary...

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

782–791

I don't think you need to pass thread the Module argument everywhere. The module you want should be available as comp_unit.GetModule(). SymbolFileDWARFDebugMap::ParseCompileUnitAtIndex ensures that the CUs are created pointing to the "main" module.

[aprantl]

Thanks, Pavel that was really helpful! Using cu->GetModule() indeed works, and is of course much cleaner.

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

else
     return eLanguageTypeUnknown;
 }
 
 XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
   if (!dwarf_cu)
     return {};
-  ModuleSP module_sp = m_objfile_sp->GetModule();
-  if (!module_sp)
-    return {};
   const DWARFBaseDIE cu_die = dwarf_cu->GetNonSkeletonUnit().GetUnitDIEOnly();
   if (!cu_die)
     return {};
   const char *sdk = cu_die.GetAttributeValueAsString(DW_AT_APPLE_sdk, nullptr);
   if (!sdk)
     return {};
   const char *sysroot =
       cu_die.GetAttributeValueAsString(DW_AT_LLVM_sysroot, "");
+  // Register the sysroot path remapping with the module belonging to
+  // the CU as well as the one belonging to the symbol file. The two
+  // would be different if this is an OSO object and module is the
+  // corresponding debug map, in which case both should be updated.
+  ModuleSP module_sp = comp_unit.GetModule();
+  if (module_sp)
     module_sp->RegisterXcodeSDK(sdk, sysroot);
+
+  ModuleSP local_module_sp = m_objfile_sp->GetModule();
+  if (local_module_sp && local_module_sp != module_sp)
+    local_module_sp->RegisterXcodeSDK(sdk, sysroot);
+
   return {sdk};
 }
 
 size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, "SymbolFileDWARF::ParseFunctions");
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

public:
   ~SymbolFileDWARFDebugMap() override;
 
   uint32_t CalculateAbilities() override;
   void InitializeObject() override;
 
   // Compile Unit function calls
   lldb::LanguageType
   ParseLanguage(lldb_private::CompileUnit &comp_unit) override;
-
   lldb_private::XcodeSDK
   ParseXcodeSDK(lldb_private::CompileUnit &comp_unit) override;
-
   size_t ParseFunctions(lldb_private::CompileUnit &comp_unit) override;
-
   bool ParseLineTable(lldb_private::CompileUnit &comp_unit) override;
-
   bool ParseDebugMacros(lldb_private::CompileUnit &comp_unit) override;
 
   bool ForEachExternalModule(
       lldb_private::CompileUnit &, llvm::DenseSet<lldb_private::SymbolFile *> &,
       llvm::function_ref<bool(lldb_private::Module &)>) override;
 
   bool ParseSupportFiles(lldb_private::CompileUnit &comp_unit,
                          lldb_private::FileSpecList &support_files) override;

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

 SymbolFileDWARFDebugMap::ParseLanguage(CompileUnit &comp_unit) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit);
   if (oso_dwarf)
     return oso_dwarf->ParseLanguage(comp_unit);
   return eLanguageTypeUnknown;
 }
 
-XcodeSDK
-SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) {
+XcodeSDK SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit);
   if (oso_dwarf)
     return oso_dwarf->ParseXcodeSDK(comp_unit);
   return {};
 }
 
 size_t SymbolFileDWARFDebugMap::ParseFunctions(CompileUnit &comp_unit) {

lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

)";
   auto triple = "x86_64-apple-macosx";
   YAMLModuleTester t(yamldata, triple);
   auto dwarf_unit_sp = t.GetDwarfUnit();
   auto *dwarf_cu = llvm::cast<DWARFCompileUnit>(dwarf_unit_sp.get());
   ASSERT_TRUE((bool)dwarf_cu);
   SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
   CompUnitSP comp_unit = sym_file.GetCompileUnitAtIndex(0);
   ASSERT_TRUE((bool)comp_unit.get());
+  ModuleSP module = t.GetModule();
+  ASSERT_EQ(module->GetSourceMappingList().GetSize(), 0u);
   XcodeSDK sdk = sym_file.ParseXcodeSDK(*comp_unit);
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
+  ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
 #endif