[lldb] Add a per-CU API to read the SDK #119022

adrian-prantl · 2024-12-06T20:10:50Z

The Swift plugin would find this useful.

llvmbot · 2024-12-06T20:11:24Z

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/119022.diff

4 Files Affected:

(modified) lldb/include/lldb/Target/Platform.h (+32)
(modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+38)
(modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h (+6)
(modified) lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp (+7)

diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index f8a2cbf0d5d049..2f954ed18bb830 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -473,6 +473,38 @@ class Platform : public PluginInterface {
                       LLVM_PRETTY_FUNCTION, GetName()));
   }
 
+  /// Search CU for the SDK paths the CUs was compiled against. In the
+  /// presence of different SDKs, we try to pick the most appropriate
+  /// one using \ref XcodeSDK::Merge.
+  ///
+  /// \param[in] unit The CU
+  ///
+  /// \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).
+  virtual llvm::Expected<std::pair<XcodeSDK, bool>>
+  GetSDKPathFromDebugInfo(CompileUnit &unit) {
+    return llvm::createStringError(
+        llvm::formatv("{0} not implemented for '{1}' platform.",
+                      LLVM_PRETTY_FUNCTION, GetName()));
+  }
+
+  /// Returns the full path of the most appropriate SDK for the
+  /// specified compile unit. This function gets this path by parsing
+  /// debug-info (see \ref `GetSDKPathFromDebugInfo`).
+  ///
+  /// \param[in] unit The CU to scan.
+  ///
+  /// \returns If successful, returns the full path to an
+  ///          Xcode SDK.
+  virtual llvm::Expected<std::string>
+  ResolveSDKPathFromDebugInfo(CompileUnit &unit) {
+    return llvm::createStringError(
+        llvm::formatv("{0} not implemented for '{1}' platform.",
+                      LLVM_PRETTY_FUNCTION, GetName()));
+  }
+
   bool IsHost() const {
     return m_is_host; // Is this the default host platform?
   }
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index ed0c614cb3576b..90324e388dbf5e 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -29,6 +29,7 @@
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueString.h"
 #include "lldb/Interpreter/Options.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
@@ -1429,3 +1430,40 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) {
 
   return path_or_err->str();
 }
+
+llvm::Expected<std::pair<XcodeSDK, bool>>
+PlatformDarwin::GetSDKPathFromDebugInfo(CompileUnit &unit) {
+  ModuleSP module_sp = unit.CalculateSymbolContextModule();
+  if (!module_sp)
+    return llvm::createStringError("compile unit has no module");
+  SymbolFile *sym_file = module_sp->GetSymbolFile();
+  if (!sym_file)
+    return llvm::createStringError(
+        llvm::formatv("No symbol file available for module '{0}'",
+                      module_sp->GetFileSpec().GetFilename()));
+
+  const bool found_mismatch = false;
+  return std::pair{sym_file->ParseXcodeSDK(unit), found_mismatch};
+}
+
+llvm::Expected<std::string>
+PlatformDarwin::ResolveSDKPathFromDebugInfo(CompileUnit &unit) {
+  auto sdk_or_err = GetSDKPathFromDebugInfo(unit);
+  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();
+}
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index 66a26d2f496776..157709424893c2 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -130,6 +130,12 @@ class PlatformDarwin : public PlatformPOSIX {
   llvm::Expected<std::string>
   ResolveSDKPathFromDebugInfo(Module &module) override;
 
+  llvm::Expected<std::pair<XcodeSDK, bool>>
+  GetSDKPathFromDebugInfo(CompileUnit &unit) override;
+
+  llvm::Expected<std::string>
+  ResolveSDKPathFromDebugInfo(CompileUnit &unit) override;
+
 protected:
   static const char *GetCompatibleArch(ArchSpec::Core core, size_t idx);
 
diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
index f1998a92f19b05..fc008ca7011e44 100644
--- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -268,6 +268,13 @@ TEST_P(SDKPathParsingMultiparamTests, TestSDKPathFromDebugInfo) {
   EXPECT_EQ(found_mismatch, expect_mismatch);
   EXPECT_EQ(sdk.IsAppleInternalSDK(), expect_internal_sdk);
   EXPECT_NE(sdk.GetString().find(expect_sdk_path_pattern), std::string::npos);
+
+  {
+    auto sdk_or_err =
+        platform_sp->GetSDKPathFromDebugInfo(*dwarf_cu->GetLLDBCompUnit());
+    ASSERT_TRUE(static_cast<bool>(sdk_or_err));
+    EXPECT_EQ(sdk.IsAppleInternalSDK(), expect_internal_sdk);
+  }
 }
 
 SDKPathParsingTestData sdkPathParsingTestCases[] = {

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

medismailben · 2024-12-06T20:51:41Z

lldb/include/lldb/Target/Platform.h

+  ///          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).
+  virtual llvm::Expected<std::pair<XcodeSDK, bool>>


Not very related to this change but it looks like this base Platform class which is supposed to be generic has some methods specific to the certain platforms ...

This could be part of @bulbazord Platform refactor but maybe we should have a SDK base class that'd be returned here instead of the the Darwin platform specific XcodeSDK return type.

This is needed by the Swift plugin.

JDevlieghere

LGTM.

lldb/include/lldb/Target/Platform.h

Co-authored-by: Jonas Devlieghere <[email protected]>

github-actions · 2024-12-06T22:16:33Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 1d95825d4d168a17a4f27401dec3f2977a59a70e 4facb5fc17280c27af99b014e621f7a573afb929 --extensions h,cpp -- lldb/include/lldb/Target/Platform.h lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

View the diff from clang-format here.

diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index a702abb540..d6864798ea 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -477,7 +477,7 @@ public:
   ///
   /// \param[in] unit The CU
   ///
-  /// \returns A parsed XcodeSDK object if successful, an Error otherwise. 
+  /// \returns A parsed XcodeSDK object if successful, an Error otherwise.
   virtual llvm::Expected<XcodeSDK> GetSDKPathFromDebugInfo(CompileUnit &unit) {
     return llvm::createStringError(
         llvm::formatv("{0} not implemented for '{1}' platform.",

clayborg · 2024-12-06T23:16:33Z

lldb/include/lldb/Target/Platform.h

+  /// \param[in] unit The CU
+  ///
+  /// \returns A parsed XcodeSDK object if successful, an Error otherwise. 
+  virtual llvm::Expected<XcodeSDK> GetSDKPathFromDebugInfo(CompileUnit &unit) {


Can we abstract a base class for SDK and have XcodeSDK become a plug-in?

I'd volunteer to do that work if we had another platform that also has a concept of an SDK. Otherwise it'd be dangerous that I design the base class to be way too close to Xcode's SDK properties.

adrian-prantl requested a review from JDevlieghere as a code owner December 6, 2024 20:10

adrian-prantl requested review from Michael137 and removed request for JDevlieghere December 6, 2024 20:10

llvmbot added the lldb label Dec 6, 2024

adrian-prantl requested a review from JDevlieghere December 6, 2024 20:11

medismailben self-requested a review December 6, 2024 20:47

medismailben reviewed Dec 6, 2024

View reviewed changes

[lldb] Add a per-CU API to read the SDK.

60ab35f

This is needed by the Swift plugin.

adrian-prantl force-pushed the sdk-cu-api branch from aadb454 to 60ab35f Compare December 6, 2024 21:04

JDevlieghere approved these changes Dec 6, 2024

View reviewed changes

lldb/include/lldb/Target/Platform.h Outdated Show resolved Hide resolved

lldb/include/lldb/Target/Platform.h Show resolved Hide resolved

Update lldb/include/lldb/Target/Platform.h

4facb5f

Co-authored-by: Jonas Devlieghere <[email protected]>

adrian-prantl merged commit 01d8e0f into llvm:main Dec 6, 2024
4 of 6 checks passed

clayborg reviewed Dec 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb] Add a per-CU API to read the SDK #119022

[lldb] Add a per-CU API to read the SDK #119022

Uh oh!

adrian-prantl commented Dec 6, 2024 •

edited

Loading

Uh oh!

llvmbot commented Dec 6, 2024

Uh oh!

Uh oh!

medismailben Dec 6, 2024

Uh oh!

JDevlieghere left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 6, 2024

Uh oh!

clayborg Dec 6, 2024

Uh oh!

adrian-prantl Dec 6, 2024

Uh oh!

Uh oh!

[lldb] Add a per-CU API to read the SDK #119022

[lldb] Add a per-CU API to read the SDK #119022

Uh oh!

Conversation

adrian-prantl commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 6, 2024

Uh oh!

Uh oh!

medismailben Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 6, 2024

Uh oh!

clayborg Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

adrian-prantl Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adrian-prantl commented Dec 6, 2024 •

edited

Loading