[lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

[Michael137]

Summary

When we build the Clang module compilation command (e.g., when
a user requests import of a module via expression @import Foundation),
LLDB will try to determine which SDK directory to use as the sysroot.
However, it currently does so by simply enumerating the SDKs directory
and picking the last one that's appropriate for module compilation
(see PlatformDarwin::GetSDKDirectoryForModules). That means if we have
multiple platform SDKs installed (e.g., a public and internal one), we
may pick the wrong one by chance.

On Darwin platforms we emit the SDK path that a object
file was compiled against into DWARF (using DW_AT_LLVM_sysroot
and DW_AT_APPLE_sdk). For Swift debugging, we already parse the SDK
path from debug-info if we can.

This patch mimicks the Swift behaviour for non-Swift languages. I.e.,
if we can get the SDK path from debug-info, do so. Otherwise, fall back
to the old heuristic.

rdar://110407148

[Michael137]

lldb/include/lldb/Target/Platform.h

492 ↗

(On Diff #543159)

I'm open to suggestions on where to put this API. Platform seems a bit too generic. But the intention is to share it with the Swift plugin. I could put it into PlatformDarwin.

[Michael137]

• Remove redundant headers

[Michael137]

lldb/include/lldb/Target/Platform.h

492 ↗

(On Diff #543159)

See how I intend to use it from the Swift plugin here

[aprantl]

lldb/include/lldb/Target/Platform.h

479 ↗

(On Diff #543160)

These flags really only make sense in the context of an XcodeSDK, so why not just return an XcodeSDK or XcodeSDK::Info object here? Otherwise we'll probably introduce subtle bugs due to a lossy translation between the flags.

[Michael137]

lldb/include/lldb/Target/Platform.h

479 ↗

(On Diff #543160)

Yup I think that'd be better. That'll also make it easier to use from the Swift plugin

[Michael137]

• Move into PlatformDarwin
• Return XcodeSDK from GetSDKPathFromDebugInfo so it's easier to re-use from Swift plugin
• Introduce ResolveSDKPathFromDebugInfo to be used from PlatformDarwin
• Adjust tests

[Michael137]

• Remove redundant header

[Michael137]

• Remove more headers

[Michael137]

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

1363

Only remaining question is whether we want to limit this to just Objective-C and Swift. Going through each compile unit for C++ seems like a lot of work for something that we won't use

@aprantl

[Michael137]

lldb/include/lldb/Target/Platform.h

479 ↗

(On Diff #543160)

Actually on second look, the XcodeSDK and XcodeSDK::Info objects represent information about a single (possibly parsed) SDK path. Whereas what the intention here was is to let the caller know whether we encountered a public/internal SDK while scanning all the CUs. Since we only return a single XcodeSDK (not all the ones we looked at) in my opinion it isn't quite right to store that information in it.

This is all really only used to print a Swift health. Maybe we could instead just log this to LLDBLog::Types? Then we don't need to worry about returning any of this information. @aprantl

[Michael137]

• Update unit-tests
• Expand function docs

[Michael137]

• Parameterize tests
• Return bool to indicate SDK mismatch

[Michael137]

• Fix test

[Michael137]

• EXPECT -> ASSERT

[aprantl]

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

1363

The work to detect the language and to get the SDK attribute is almost the same, both are stored in the top-level CU DIE.

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h

public:
 
   /// Return the toolchain directory the current LLDB instance is located in.
   static FileSpec GetCurrentToolchainDirectory();
 
   /// Return the command line tools directory the current LLDB instance is
   /// located in.
   static FileSpec GetCurrentCommandLineToolsDirectory();
 
+  /// Search each CU associated with the specified 'module' for
+  /// the SDK paths the CUs were compiled against. In the presence
+  /// of different SDKs, we try to pick the most appropriate one
+  /// using \ref XcodeSDK::Merge.
+  ///
+  /// \param[in] module Module whose debug-info CUs to parse for
+  ///                   which SDK they were compiled against.
+  ///
+  /// \returns If successful, returns a pair of a parsed XcodeSDK
+  ///          object and a boolean that is 'true' if we encountered
+  ///          a conflicting combination of SDKs when parsing the CUs
+  ///          (e.g., a public and internal SDK).
+  static llvm::Expected<std::pair<XcodeSDK, bool>>
+  GetSDKPathFromDebugInfo(Module &module);
+
+  /// Returns the full path of the most appropriate SDK for the
+  /// specified 'module'. This function gets this path by parsing
+  /// debug-info (see \ref `GetSDKPathFromDebugInfo`).
+  ///
+  /// \param[in] module Module whose debug-info to parse for
+  ///                   which SDK it was compiled against.
+  ///
+  /// \returns If successful, returns the full path to an
+  ///          Xcode SDK.
+  static llvm::Expected<std::string>
+  ResolveSDKPathFromDebugInfo(Module &module);
+
 protected:
   static const char *GetCompatibleArch(ArchSpec::Core core, size_t idx);
 
   struct CrashInfoAnnotations {
     uint64_t version;          // unsigned long
     uint64_t message;          // char *
     uint64_t signature_string; // char *
     uint64_t backtrace;        // char *

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/ProcessInfo.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VersionTuple.h"
 
 #if defined(__APPLE__)
 #include <TargetConditionals.h>
 #endif
 
case XcodeSDK::Type::unknown:
       }
       return;
     }
     minimum_version_option << version.getAsString();
     options.emplace_back(std::string(minimum_version_option.GetString()));
   }
 
   FileSpec sysroot_spec;
-  // Scope for mutex locker below
-  {
+
+  if (target) {
+    if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
+      auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
+      if (path_or_err) {
+        sysroot_spec = FileSpec(*path_or_err);
+      } else {
+        LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host),
+                       path_or_err.takeError(),
+                       "Failed to resolve SDK path: {0}");
+      }
+    }
+  }
+
+  if (!FileSystem::Instance().IsDirectory(sysroot_spec.GetPath())) {
     std::lock_guard<std::mutex> guard(m_mutex);
     sysroot_spec = GetSDKDirectoryForModules(sdk_type);
   }
 
   if (FileSystem::Instance().IsDirectory(sysroot_spec.GetPath())) {
     options.push_back("-isysroot");
     options.push_back(sysroot_spec.GetPath());
   }
#elif TARGET_OS_TV
   return llvm::Triple::TvOS;
 #elif TARGET_OS_BRIDGE
   return llvm::Triple::BridgeOS;
 #else
 #error "LLDB being compiled for an unrecognized Darwin OS"
 #endif
 #endif // __APPLE__
 }
+
+llvm::Expected<std::pair<XcodeSDK, bool>>
+PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) {
+  SymbolFile *sym_file = module.GetSymbolFile();
+  if (!sym_file)
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        llvm::formatv("No symbol file available for module '{0}'",
+                      module.GetFileSpec().GetFilename().AsCString("")));
+
+  bool found_public_sdk = false;
+  bool found_internal_sdk = false;
+  XcodeSDK merged_sdk;
+  for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) {
+    if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) {
+      auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp);
+      bool is_internal_sdk = cu_sdk.IsAppleInternalSDK();
+      found_public_sdk |= !is_internal_sdk;
+      found_internal_sdk |= is_internal_sdk;
+
+      merged_sdk.Merge(cu_sdk);
+    }
+  }
+
+  const bool found_mismatch = found_internal_sdk && found_public_sdk;
+
+  return std::pair{std::move(merged_sdk), found_mismatch};
+}
+
+llvm::Expected<std::string>
+PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) {
+  auto sdk_or_err = GetSDKPathFromDebugInfo(module);
+  if (!sdk_or_err)
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        llvm::formatv("Failed to parse SDK path from debug-info: {0}",
+                      llvm::toString(sdk_or_err.takeError())));
+
+  auto [sdk, _] = std::move(*sdk_or_err);
+
+  auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
+  if (!path_or_err)
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        llvm::formatv("Error while searching for SDK (XcodeSDK '{0}'): {1}",
+                      sdk.GetString(),
+                      llvm::toString(path_or_err.takeError())));
+
+  return path_or_err->str();
+}

lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

 //===-- XcodeSDKModuleTests.cpp -------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
 #include "Plugins/SymbolFile/DWARF/DWARFCompileUnit.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 #ifdef __APPLE__
 namespace {
 class XcodeSDKModuleTests : public testing::Test {
   SubsystemRAII<HostInfoBase, PlatformMacOSX> subsystems;
 };
-} // namespace
 
+struct SDKPathParsingTestData {
+  /// Each path will be put into a new CU's
+  /// DW_AT_LLVM_sysroot.
+  std::vector<llvm::StringRef> input_sdk_paths;
+
+  /// 'true' if we expect \ref GetSDKPathFromDebugInfo
+  /// to notify us about an SDK mismatch.
+  bool expect_mismatch;
+
+  /// 'true if the test expects the parsed SDK to
+  /// be an internal one.
+  bool expect_internal_sdk;
+
+  /// A substring that the final parsed sdk
+  /// is expected to contain.
+  llvm::StringRef expect_sdk_path_pattern;
+};
+
+struct SDKPathParsingMultiparamTests
+    : public XcodeSDKModuleTests,
+      public testing::WithParamInterface<SDKPathParsingTestData> {
+  std::vector<std::string>
+  createCompileUnits(std::vector<llvm::StringRef> const &sdk_paths) {
+    std::vector<std::string> compile_units;
+
+    for (auto sdk_path : sdk_paths) {
+      compile_units.emplace_back(llvm::formatv(
+          R"(
+        - Version:         2
+          AddrSize:        8
+          AbbrevTableID:   0
+          AbbrOffset:      0x0
+          Entries:
+            - AbbrCode:        0x00000001
+              Values:
+                - Value:       0x000000000000000C
+                - CStr:        {0}
+                - CStr:        {1}
+            - AbbrCode:        0x00000000
+            )",
+          llvm::sys::path::filename(sdk_path, llvm::sys::path::Style::posix),
+          sdk_path));
+    }
+
+    return compile_units;
+  }
+};
+} // namespace
 
 TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
   const char *yamldata = R"(
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
)";
   CompUnitSP comp_unit = sym_file.GetCompileUnitAtIndex(0);
   ASSERT_TRUE(static_cast<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);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
+  // Tests that parsing a CU with an invalid SDK directory name fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+    - Table:
+        - Code:            0x00000001
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_data2
+            - Attribute:       DW_AT_APPLE_sdk
+              Form:            DW_FORM_string
+  debug_info:
+    - Version:         2
+      AddrSize:        8
+      AbbrevTableID:   0
+      AbbrOffset:      0x0
+      Entries:
+        - AbbrCode:        0x00000001
+          Values:
+            - Value:       0x000000000000000C
+            - CStr:        "1abc@defgh2"
+        - AbbrCode:        0x00000000
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast<bool>(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
+  // Tests that parsing a CU without a DW_AT_APPLE_sdk fails.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+    - Table:
+        - Code:            0x00000001
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_data2
+            - Attribute:       DW_AT_LLVM_sysroot
+              Form:            DW_FORM_string
+  debug_info:
+    - Version:         2
+      AddrSize:        8
+      AbbrevTableID:   0
+      AbbrOffset:      0x0
+      Entries:
+        - AbbrCode:        0x00000001
+          Values:
+            - Value:       0x000000000000000C
+            - CStr:        "/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+        - AbbrCode:        0x00000000
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  EXPECT_FALSE(static_cast<bool>(path_or_err));
+  llvm::consumeError(path_or_err.takeError());
+}
+
+TEST_P(SDKPathParsingMultiparamTests, TestSDKPathFromDebugInfo) {
+  // Tests that we can parse the SDK path from debug-info.
+  // In the presence of multiple compile units, one of which
+  // points to an internal SDK, we should pick the internal SDK.
+
+  std::string yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+    - Table:
+        - Code:            0x00000001
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_data2
+            - Attribute:       DW_AT_APPLE_sdk
+              Form:            DW_FORM_string
+            - Attribute:       DW_AT_LLVM_sysroot
+              Form:            DW_FORM_string
+  debug_info:
+)";
+
+  auto [input_sdk_paths, expect_mismatch, expect_internal_sdk,
+        expect_sdk_path_pattern] = GetParam();
+
+  for (auto &&sdk : createCompileUnits(input_sdk_paths))
+    yamldata += std::move(sdk);
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast<DWARFCompileUnit>(dwarf_unit);
+  ASSERT_TRUE(static_cast<bool>(dwarf_cu));
+  SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), input_sdk_paths.size());
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  EXPECT_TRUE(static_cast<bool>(sdk_or_err));
+
+  auto [sdk, found_mismatch] = *sdk_or_err;
+
+  EXPECT_TRUE(found_mismatch);
+  EXPECT_TRUE(sdk.IsAppleInternalSDK());
+  EXPECT_NE(sdk.GetString().find("Internal.sdk"), std::string::npos);
+}
+
+SDKPathParsingTestData sdkPathParsingTestCases[] = {
+    /// Multiple CUs with a mix of internal and public SDKs
+    {.input_sdk_paths =
+         {"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk",
+          "/invalid/path/to/something.invalid.sdk",
+          "/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk",
+          "/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"},
+     .expect_mismatch = true,
+     .expect_internal_sdk = true,
+     .expect_sdk_path_pattern = "Internal.sdk"},
+
+    /// Single CU with a public SDK
+    {.input_sdk_paths =
+         {"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"},
+     .expect_mismatch = false,
+     .expect_internal_sdk = false,
+     .expect_sdk_path_pattern = "MacOSX10.9.sdk"},
+
+    /// Single CU with an internal SDK
+    {.input_sdk_paths =
+         {"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"},
+     .expect_mismatch = false,
+     .expect_internal_sdk = true,
+     .expect_sdk_path_pattern = "Internal.sdk"},
+
+    /// Two CUs with an internal SDK each
+    {.input_sdk_paths =
+         {"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk",
+          "/Library/Developer/CommandLineTools/SDKs/iPhoneOS12.9.Internal.sdk"},
+     .expect_mismatch = false,
+     .expect_internal_sdk = true,
+     .expect_sdk_path_pattern = "Internal.sdk"},
+
+    /// Two CUs with an internal SDK each
+    {.input_sdk_paths =
+         {"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.1.sdk",
+          "/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk"},
+     .expect_mismatch = false,
+     .expect_internal_sdk = false,
+     .expect_sdk_path_pattern = "iPhoneOS14.1.sdk"},
+};
+
+INSTANTIATE_TEST_CASE_P(SDKPathParsingTests, SDKPathParsingMultiparamTests,
+                        ::testing::ValuesIn(sdkPathParsingTestCases));
 #endif