[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

2023-07-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

So Ted, can you verify that nothing is missing in RISCV since this is already 
the way it works for x86/x86_64 and arm32/arm64? I am thinking like Jason is 
where I wonder if something isn't just hooked up right for RISCV somehow. Or if 
you can verify that nothing changes for x86/x86_64 or arm32/arm64 if this 
setting is enabled and there are no symbolication regressions in the comments 
if this does get enabled for the architectures where it is working as 
expected...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155107/new/

https://reviews.llvm.org/D155107

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

2023-07-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/include/lldb/API/SBProcess.h:415-416
+  /// (unsigned long long) 0xfc00
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+

Will there only ever be two variants of this for code and data? If we think 
there might be another address mask type later it might be a good idea to use 
an enumeration for these two functions and all functions below? I was thinking 
of something like:
```
enum AddressMaskType {
  eTypeCode = 0,
  eTypeData
};
lldb::addr_t GetAddressMask(AddressMaskType mask_type);
```

This enum could be used in all subsequent calls. I am just thinking of any 
needed extra API changes needed in the future. If we need another address mask 
type in the future we just add another enum and all functions stay the same if 
we do it as suggested above. Else we will need to add many more API functions 
if we need a new type later.



Comment at: lldb/include/lldb/API/SBProcess.h:430-434
+  /// If the low ten bits are the only valid bits for addressing,
+  /// this would be the mask returned:
+  ///
+  /// (lldb) p/x ~((1ULL << 10) - 1)
+  /// (unsigned long long) 0xfc00

Is this comment needed for the set accessors? These functions don't return 
anything



Comment at: lldb/include/lldb/API/SBProcess.h:451-453
+  lldb::addr_t FixCodeAddress(lldb::addr_t addr);
+  lldb::addr_t FixDataAddress(lldb::addr_t addr);
+  lldb::addr_t FixAnyAddress(lldb::addr_t addr);

Should the parameter be named "load_addr" to ensure people know it is a load 
address?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155905/new/

https://reviews.llvm.org/D155905

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D155198: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

2023-07-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Sorry for the late response, but I had verbally agreed to this approach when 
Felipe and I spoke about how to fix this. LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155198/new/

https://reviews.llvm.org/D155198

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

2023-07-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG94e8fefe385c: [lldb] Increase the default timeouts when 
running under ASan (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156279/new/

https://reviews.llvm.org/D156279

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
  lldb/source/Target/TargetProperties.td


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -244,10 +244,18 @@
 DefaultTrue,
 Desc<"If true, stop when the inferior exec's.">;
   def UtilityExpressionTimeout: Property<"utility-expression-timeout", 
"UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<15>,
+#endif
 Desc<"The time in seconds to wait for LLDB-internal utility expressions.">;
   def InterruptTimeout: Property<"interrupt-timeout", "UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<20>,
+#endif
 Desc<"The time in seconds to wait for an interrupt succeed in stopping the 
target.">;
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -3,7 +3,11 @@
 let Definition = "processgdbremote" in {
   def PacketTimeout: Property<"packet-timeout", "UInt64">,
 Global,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<5>,
+#endif
 Desc<"Specify the default packet timeout in seconds.">;
   def TargetDefinitionFile: Property<"target-definition-file", "FileSpec">,
 Global,
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -20,6 +20,11 @@
   endif()
 
   set(LLVM_TARGET_DEFINITIONS ${LTG_SOURCE})
+
+  if (LLVM_USE_SANITIZER MATCHES ".*Address.*")
+list(APPEND LTG_UNPARSED_ARGUMENTS -DLLDB_SANITIZED)
+  endif()
+
   tablegen(LLDB ${LTG_UNPARSED_ARGUMENTS})
 
   if(LTG_TARGET)


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -244,10 +244,18 @@
 DefaultTrue,
 Desc<"If true, stop when the inferior exec's.">;
   def UtilityExpressionTimeout: Property<"utility-expression-timeout", "UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<15>,
+#endif
 Desc<"The time in seconds to wait for LLDB-internal utility expressions.">;
   def InterruptTimeout: Property<"interrupt-timeout", "UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<20>,
+#endif
 Desc<"The time in seconds to wait for an interrupt succeed in stopping the target.">;
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -3,7 +3,11 @@
 let Definition = "processgdbremote" in {
   def PacketTimeout: Property<"packet-timeout", "UInt64">,
 Global,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<5>,
+#endif
 Desc<"Specify the default packet timeout in seconds.">;
   def TargetDefinitionFile: Property<"target-definition-file", "FileSpec">,
 Global,
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -20,6 +20,11 @@
   endif()
 
   set(LLVM_TARGET_DEFINITIONS ${LTG_SOURCE})
+
+  if (LLVM_USE_SANITIZER MATCHES ".*Address.*")
+list(APPEND LTG_UNPARSED_ARGUMENTS -DLLDB_SANITIZED)
+  endif()
+
   tablegen(LLDB ${LTG_UNPARSED_ARGUMENTS})
 
   if(LTG_TARGET)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 94e8fef - [lldb] Increase the default timeouts when running under ASan

2023-07-25 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2023-07-25T17:45:19-07:00
New Revision: 94e8fefe385cd3b6a4656bec244ddf9791056f0c

URL: 
https://github.com/llvm/llvm-project/commit/94e8fefe385cd3b6a4656bec244ddf9791056f0c
DIFF: 
https://github.com/llvm/llvm-project/commit/94e8fefe385cd3b6a4656bec244ddf9791056f0c.diff

LOG: [lldb] Increase the default timeouts when running under ASan

Increase the default timeouts when running under ASan. We had something
similar before we adopted tablegen, but the larger timeouts got lost in
the transition, possibly because tablegen's preprocessor support is very
limited. This patch passes a new define (LLDB_SANITIZED) to
lldb-tablegen on which we can base the default value.

Differential revision: https://reviews.llvm.org/D156279

Added: 


Modified: 
lldb/cmake/modules/AddLLDB.cmake
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
lldb/source/Target/TargetProperties.td

Removed: 




diff  --git a/lldb/cmake/modules/AddLLDB.cmake 
b/lldb/cmake/modules/AddLLDB.cmake
index d47a30f5e10921..328e883ddbe5a6 100644
--- a/lldb/cmake/modules/AddLLDB.cmake
+++ b/lldb/cmake/modules/AddLLDB.cmake
@@ -20,6 +20,11 @@ function(lldb_tablegen)
   endif()
 
   set(LLVM_TARGET_DEFINITIONS ${LTG_SOURCE})
+
+  if (LLVM_USE_SANITIZER MATCHES ".*Address.*")
+list(APPEND LTG_UNPARSED_ARGUMENTS -DLLDB_SANITIZED)
+  endif()
+
   tablegen(LLDB ${LTG_UNPARSED_ARGUMENTS})
 
   if(LTG_TARGET)

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
index d4c3c8b94b7ef3..520dad062e09a3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -3,7 +3,11 @@ include "../../../../include/lldb/Core/PropertiesBase.td"
 let Definition = "processgdbremote" in {
   def PacketTimeout: Property<"packet-timeout", "UInt64">,
 Global,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<5>,
+#endif
 Desc<"Specify the default packet timeout in seconds.">;
   def TargetDefinitionFile: Property<"target-definition-file", "FileSpec">,
 Global,

diff  --git a/lldb/source/Target/TargetProperties.td 
b/lldb/source/Target/TargetProperties.td
index 19ea505af65637..3bfdfa8cfaa957 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -244,10 +244,18 @@ let Definition = "process" in {
 DefaultTrue,
 Desc<"If true, stop when the inferior exec's.">;
   def UtilityExpressionTimeout: Property<"utility-expression-timeout", 
"UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<15>,
+#endif
 Desc<"The time in seconds to wait for LLDB-internal utility expressions.">;
   def InterruptTimeout: Property<"interrupt-timeout", "UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<20>,
+#endif
 Desc<"The time in seconds to wait for an interrupt succeed in stopping the 
target.">;
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

2023-07-25 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Nevermind, you answered my question in the description already!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156279/new/

https://reviews.llvm.org/D156279

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

2023-07-25 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

This LGTM, but I'm surprized you didn't have to delete any older code that 
tried to do the same thing. Did the thing I remember not survive the transition 
to tablegen or is this orthogonal?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156279/new/

https://reviews.llvm.org/D156279

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156279: [lldb] Increase the default timeouts when running under ASan

2023-07-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, bulbazord, rupprecht.
Herald added a project: All.
JDevlieghere requested review of this revision.

Increase the default timeouts when running under ASan. We had something similar 
before we adopted tablegen, but the larger timeouts got lost in the transition, 
possibly because tablegen preprocessor support is very limited. This patch 
passes a new define (`LLDB_SANITIZED`) to lldb-tablegen on which we can base 
the default value.


https://reviews.llvm.org/D156279

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
  lldb/source/Target/TargetProperties.td


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -244,10 +244,18 @@
 DefaultTrue,
 Desc<"If true, stop when the inferior exec's.">;
   def UtilityExpressionTimeout: Property<"utility-expression-timeout", 
"UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<15>,
+#endif
 Desc<"The time in seconds to wait for LLDB-internal utility expressions.">;
   def InterruptTimeout: Property<"interrupt-timeout", "UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<20>,
+#endif
 Desc<"The time in seconds to wait for an interrupt succeed in stopping the 
target.">;
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -3,7 +3,11 @@
 let Definition = "processgdbremote" in {
   def PacketTimeout: Property<"packet-timeout", "UInt64">,
 Global,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<5>,
+#endif
 Desc<"Specify the default packet timeout in seconds.">;
   def TargetDefinitionFile: Property<"target-definition-file", "FileSpec">,
 Global,
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -20,6 +20,11 @@
   endif()
 
   set(LLVM_TARGET_DEFINITIONS ${LTG_SOURCE})
+
+  if (LLVM_USE_SANITIZER MATCHES ".*Address.*")
+list(APPEND LTG_UNPARSED_ARGUMENTS -DLLDB_SANITIZED)
+  endif()
+
   tablegen(LLDB ${LTG_UNPARSED_ARGUMENTS})
 
   if(LTG_TARGET)


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -244,10 +244,18 @@
 DefaultTrue,
 Desc<"If true, stop when the inferior exec's.">;
   def UtilityExpressionTimeout: Property<"utility-expression-timeout", "UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<15>,
+#endif
 Desc<"The time in seconds to wait for LLDB-internal utility expressions.">;
   def InterruptTimeout: Property<"interrupt-timeout", "UInt64">,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<20>,
+#endif
 Desc<"The time in seconds to wait for an interrupt succeed in stopping the target.">;
   def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
 DefaultFalse,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -3,7 +3,11 @@
 let Definition = "processgdbremote" in {
   def PacketTimeout: Property<"packet-timeout", "UInt64">,
 Global,
+#ifdef LLDB_SANITIZED
+DefaultUnsignedValue<60>,
+#else
 DefaultUnsignedValue<5>,
+#endif
 Desc<"Specify the default packet timeout in seconds.">;
   def TargetDefinitionFile: Property<"target-definition-file", "FileSpec">,
 Global,
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -20,6 +20,11 @@
   endif()
 
   set(LLVM_TARGET_DEFINITIONS ${LTG_SOURCE})
+
+  if (LLVM_USE_SANITIZER MATCHES ".*Address.*")
+list(APPEND LTG_UNPARSED_ARGUMENTS -DLLDB_SANITIZED)
+  endif()
+
   tablegen(LLDB ${LTG_UNPARSED_ARGUMENTS})
 
   if(LTG_TARGET)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

2023-07-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156270/new/

https://reviews.llvm.org/D156270

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

2023-07-25 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa809720102fa: [lldb][NFCI] Change logic to find clang 
resource dir in standalone builds (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156270/new/

https://reviews.llvm.org/D156270

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -128,3 +128,16 @@
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+
+# If LLDB is building against a prebuilt Clang, then the Clang resource
+# directory that LLDB is using for its embedded Clang instance needs to point 
to
+# the resource directory of the used Clang installation.
+if (NOT TARGET clang-resource-headers)
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LLDB_EXTERNAL_CLANG_RESOURCE_DIR
+PREFIX "${Clang_DIR}/../../../")
+
+  if (NOT EXISTS ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR})
+message(FATAL_ERROR "Expected directory for clang-resource-headers not 
found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
+  endif()
+endif()
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -282,30 +282,6 @@
   endif()
 endif()
 
-
-# If LLDB is building against a prebuilt Clang, then the Clang resource
-# directory that LLDB is using for its embedded Clang instance needs to point
-# to the resource directory of the used Clang installation.
-if (NOT TARGET clang-resource-headers)
-  set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}")
-  # Iterate over the possible places where the external resource directory
-  # could be and pick the first that exists.
-  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
-"${LLVM_BUILD_LIBRARY_DIR}"
-"${LLVM_LIBRARY_DIR}")
-# Build the resource directory path by appending 'clang/'.
-set(CANDIDATE_RESOURCE_DIR 
"${CANDIDATE}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}")
-if (IS_DIRECTORY "${CANDIDATE_RESOURCE_DIR}")
-  set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR "${CANDIDATE_RESOURCE_DIR}")
-  break()
-endif()
-  endforeach()
-
-  if (NOT LLDB_EXTERNAL_CLANG_RESOURCE_DIR)
-message(FATAL_ERROR "Expected directory for clang-resource headers not 
found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
-  endif()
-endif()
-
 # Find Apple-specific libraries or frameworks that may be needed.
 if (APPLE)
   if(NOT APPLE_EMBEDDED)


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -128,3 +128,16 @@
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+
+# If LLDB is building against a prebuilt Clang, then the Clang resource
+# directory that LLDB is using for its embedded Clang instance needs to point to
+# the resource directory of the used Clang installation.
+if (NOT TARGET clang-resource-headers)
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LLDB_EXTERNAL_CLANG_RESOURCE_DIR
+PREFIX "${Clang_DIR}/../../../")
+
+  if (NOT EXISTS ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR})
+message(FATAL_ERROR "Expected directory for clang-resource-headers not found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
+  endif()
+endif()
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -282,30 +282,6 @@
   endif()
 endif()
 
-
-# If LLDB is building against a prebuilt Clang, then the Clang resource
-# directory that LLDB is using for its embedded Clang instance needs to point
-# to the resource directory of the used Clang installation.
-if (NOT TARGET clang-resource-headers)
-  set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}")
-  # Iterate over the possible places where the external resource directory
-  # could be and pick the first that exists.
-  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
-"${LLVM_BUILD_LIBRARY_DIR}"
-"${LLVM_LIBRARY_DIR}")
-# Build the resource directory path by appending 'clang/'.
-set(CANDIDATE_RESOURCE_DIR "${CANDIDATE}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}")
-if (IS_DIRECTORY 

[Lldb-commits] [lldb] a809720 - [lldb][NFCI] Change logic to find clang resource dir in standalone builds

2023-07-25 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-07-25T15:52:33-07:00
New Revision: a809720102fae8d1b5a7073f99f9dae9395c5f41

URL: 
https://github.com/llvm/llvm-project/commit/a809720102fae8d1b5a7073f99f9dae9395c5f41
DIFF: 
https://github.com/llvm/llvm-project/commit/a809720102fae8d1b5a7073f99f9dae9395c5f41.diff

LOG: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

As of 0beffb854209a41f31beb18f9631258349a99299 there is a CMake
function to actually calculate the relative path to the clang resource
directory. Currently we have some bespoke logic that looks in a few
places, but with this new function we should be able to eliminate some
complexity here.

Also, I moved the functionality from LLDBConfig to LLDBStandalone since
it is only used in standalone builds.

Differential Revision: https://reviews.llvm.org/D156270

Added: 


Modified: 
lldb/cmake/modules/LLDBConfig.cmake
lldb/cmake/modules/LLDBStandalone.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index 1393342dd5cb69..ce90ecabc6a5a5 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -282,30 +282,6 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   endif()
 endif()
 
-
-# If LLDB is building against a prebuilt Clang, then the Clang resource
-# directory that LLDB is using for its embedded Clang instance needs to point
-# to the resource directory of the used Clang installation.
-if (NOT TARGET clang-resource-headers)
-  set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}")
-  # Iterate over the possible places where the external resource directory
-  # could be and pick the first that exists.
-  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
-"${LLVM_BUILD_LIBRARY_DIR}"
-"${LLVM_LIBRARY_DIR}")
-# Build the resource directory path by appending 'clang/'.
-set(CANDIDATE_RESOURCE_DIR 
"${CANDIDATE}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}")
-if (IS_DIRECTORY "${CANDIDATE_RESOURCE_DIR}")
-  set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR "${CANDIDATE_RESOURCE_DIR}")
-  break()
-endif()
-  endforeach()
-
-  if (NOT LLDB_EXTERNAL_CLANG_RESOURCE_DIR)
-message(FATAL_ERROR "Expected directory for clang-resource headers not 
found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
-  endif()
-endif()
-
 # Find Apple-specific libraries or frameworks that may be needed.
 if (APPLE)
   if(NOT APPLE_EMBEDDED)

diff  --git a/lldb/cmake/modules/LLDBStandalone.cmake 
b/lldb/cmake/modules/LLDBStandalone.cmake
index e9bcabcb63dedb..fd16716d71419c 100644
--- a/lldb/cmake/modules/LLDBStandalone.cmake
+++ b/lldb/cmake/modules/LLDBStandalone.cmake
@@ -128,3 +128,16 @@ endif()
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+
+# If LLDB is building against a prebuilt Clang, then the Clang resource
+# directory that LLDB is using for its embedded Clang instance needs to point 
to
+# the resource directory of the used Clang installation.
+if (NOT TARGET clang-resource-headers)
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LLDB_EXTERNAL_CLANG_RESOURCE_DIR
+PREFIX "${Clang_DIR}/../../../")
+
+  if (NOT EXISTS ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR})
+message(FATAL_ERROR "Expected directory for clang-resource-headers not 
found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
+  endif()
+endif()



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

2023-07-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156270/new/

https://reviews.llvm.org/D156270

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

2023-07-25 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As of 0beffb854209a41f31beb18f9631258349a99299 
 there is 
a CMake
function to actually calculate the relative path to the clang resource
directory. Currently we have some bespoke logic that looks in a few
places, but with this new function we should be able to eliminate some
complexity here.

Also, I moved the functionality from LLDBConfig to LLDBStandalone since
it is only used in standalone builds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156270

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -128,3 +128,16 @@
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+
+# If LLDB is building against a prebuilt Clang, then the Clang resource
+# directory that LLDB is using for its embedded Clang instance needs to point 
to
+# the resource directory of the used Clang installation.
+if (NOT TARGET clang-resource-headers)
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LLDB_EXTERNAL_CLANG_RESOURCE_DIR
+PREFIX "${Clang_DIR}/../../../")
+
+  if (NOT EXISTS ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR})
+message(FATAL_ERROR "Expected directory for clang-resource-headers not 
found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
+  endif()
+endif()
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -282,30 +282,6 @@
   endif()
 endif()
 
-
-# If LLDB is building against a prebuilt Clang, then the Clang resource
-# directory that LLDB is using for its embedded Clang instance needs to point
-# to the resource directory of the used Clang installation.
-if (NOT TARGET clang-resource-headers)
-  set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}")
-  # Iterate over the possible places where the external resource directory
-  # could be and pick the first that exists.
-  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
-"${LLVM_BUILD_LIBRARY_DIR}"
-"${LLVM_LIBRARY_DIR}")
-# Build the resource directory path by appending 'clang/'.
-set(CANDIDATE_RESOURCE_DIR 
"${CANDIDATE}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}")
-if (IS_DIRECTORY "${CANDIDATE_RESOURCE_DIR}")
-  set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR "${CANDIDATE_RESOURCE_DIR}")
-  break()
-endif()
-  endforeach()
-
-  if (NOT LLDB_EXTERNAL_CLANG_RESOURCE_DIR)
-message(FATAL_ERROR "Expected directory for clang-resource headers not 
found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
-  endif()
-endif()
-
 # Find Apple-specific libraries or frameworks that may be needed.
 if (APPLE)
   if(NOT APPLE_EMBEDDED)


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -128,3 +128,16 @@
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+
+# If LLDB is building against a prebuilt Clang, then the Clang resource
+# directory that LLDB is using for its embedded Clang instance needs to point to
+# the resource directory of the used Clang installation.
+if (NOT TARGET clang-resource-headers)
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LLDB_EXTERNAL_CLANG_RESOURCE_DIR
+PREFIX "${Clang_DIR}/../../../")
+
+  if (NOT EXISTS ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR})
+message(FATAL_ERROR "Expected directory for clang-resource-headers not found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
+  endif()
+endif()
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -282,30 +282,6 @@
   endif()
 endif()
 
-
-# If LLDB is building against a prebuilt Clang, then the Clang resource
-# directory that LLDB is using for its embedded Clang instance needs to point
-# to the resource directory of the used Clang installation.
-if (NOT TARGET clang-resource-headers)
-  set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}")
-  # Iterate over the possible places where the external 

[Lldb-commits] [PATCH] D154989: [lldb-vsocde] Add a 'continued' event for programmatic continue events.

2023-07-25 Thread David Goldman via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ab73a398422: [lldb-vsocde] Add a continued 
event for programmatic continue events. (authored by ashgti, committed by 
dgoldman).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154989/new/

https://reviews.llvm.org/D154989

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -191,6 +191,28 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// Send a "continued" event to indicate the process is in the running state.
+void SendContinuedEvent() {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  if (!process.IsValid()) {
+return;
+  }
+
+  // If the focus thread is not set then we haven't reported any thread status
+  // to the client, so nothing to report.
+  if (!g_vsc.configuration_done_sent ||
+  g_vsc.focus_tid == LLDB_INVALID_THREAD_ID) {
+return;
+  }
+
+  llvm::json::Object event(CreateEventObject("continued"));
+  llvm::json::Object body;
+  body.try_emplace("threadId", (int64_t)g_vsc.focus_tid);
+  body.try_emplace("allThreadsContinued", true);
+  event.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(event)));
+}
+
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
@@ -252,7 +274,7 @@
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // We will have cleared g_vsc.focus_tid if the focus thread doesn't have
   // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
   // then set the focus thread to the first thread with a stop reason.
   if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
@@ -262,6 +284,7 @@
   // we at least let the UI know we stopped.
   if (num_threads_with_reason == 0) {
 lldb::SBThread thread = process.GetThreadAtIndex(0);
+g_vsc.focus_tid = thread.GetThreadID();
 g_vsc.SendJSON(CreateThreadStopped(thread, stop_id));
   } else {
 for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
@@ -468,6 +491,7 @@
 break;
   case lldb::eStateRunning:
 g_vsc.WillContinue();
+SendContinuedEvent();
 break;
   case lldb::eStateExited:
 // When restarting, we can get an "exited" event for the process we
@@ -766,10 +790,6 @@
   llvm::json::Object response;
   FillResponse(request, response);
   lldb::SBProcess process = g_vsc.target.GetProcess();
-  auto arguments = request.getObject("arguments");
-  // Remember the thread ID that caused the resume so we can set the
-  // "threadCausedFocus" boolean value in the "stopped" events.
-  g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -907,6 +907,8 @@
   uint64_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
   snprintf(desc_str, sizeof(desc_str), "breakpoint %" PRIu64 ".%" PRIu64,
bp_id, bp_loc_id);
+  body.try_emplace("hitBreakpointIds",
+   llvm::json::Array{llvm::json::Value(bp_id)});
   EmplaceSafeString(body, "description", desc_str);
 }
   } break;
@@ -951,6 +953,7 @@
   EmplaceSafeString(body, "description", std::string(description));
 }
   }
+  // "threadCausedFocus" is used in tests to validate breaking behavior.
   if (tid == g_vsc.focus_tid) {
 body.try_emplace("threadCausedFocus", true);
   }


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -191,6 +191,28 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// Send a "continued" event to indicate the process is in the running state.
+void SendContinuedEvent() {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  if (!process.IsValid()) {
+return;
+  }
+
+  // If the focus thread is not set then we haven't reported any thread status
+  // to the client, so nothing to report.
+  if (!g_vsc.configuration_done_sent ||
+  g_vsc.focus_tid == LLDB_INVALID_THREAD_ID) {
+return;
+  }
+
+  llvm::json::Object event(CreateEventObject("continued"));
+  llvm::json::Object body;
+  body.try_emplace("threadId", (int64_t)g_vsc.focus_tid);
+  

[Lldb-commits] [lldb] 3ab73a3 - [lldb-vsocde] Add a 'continued' event for programmatic continue events.

2023-07-25 Thread David Goldman via lldb-commits

Author: John Harrison
Date: 2023-07-25T14:46:17-04:00
New Revision: 3ab73a398422298fb5435ced0f7838074efa12e0

URL: 
https://github.com/llvm/llvm-project/commit/3ab73a398422298fb5435ced0f7838074efa12e0
DIFF: 
https://github.com/llvm/llvm-project/commit/3ab73a398422298fb5435ced0f7838074efa12e0.diff

LOG: [lldb-vsocde] Add a 'continued' event for programmatic continue events.

When the process is contiuned using an lldb command expression the thread state 
in VS Code is never informed and will be out of sync with the current state of 
the process. The new event will fire whenever the process is continued and 
keeps the debugger in sync with the dap client.

Reviewed By: wallace

Differential Revision: https://reviews.llvm.org/D154989

Added: 


Modified: 
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 8632ba929f77d9..98dbba32b033e3 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -907,6 +907,8 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread 
,
   uint64_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
   snprintf(desc_str, sizeof(desc_str), "breakpoint %" PRIu64 ".%" PRIu64,
bp_id, bp_loc_id);
+  body.try_emplace("hitBreakpointIds",
+   llvm::json::Array{llvm::json::Value(bp_id)});
   EmplaceSafeString(body, "description", desc_str);
 }
   } break;
@@ -951,6 +953,7 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread 
,
   EmplaceSafeString(body, "description", std::string(description));
 }
   }
+  // "threadCausedFocus" is used in tests to validate breaking behavior.
   if (tid == g_vsc.focus_tid) {
 body.try_emplace("threadCausedFocus", true);
   }

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 75c61a92bbf97d..72ebe4825024e3 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -191,6 +191,28 @@ void SendThreadExitedEvent(lldb::tid_t tid) {
   g_vsc.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// Send a "continued" event to indicate the process is in the running state.
+void SendContinuedEvent() {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  if (!process.IsValid()) {
+return;
+  }
+
+  // If the focus thread is not set then we haven't reported any thread status
+  // to the client, so nothing to report.
+  if (!g_vsc.configuration_done_sent ||
+  g_vsc.focus_tid == LLDB_INVALID_THREAD_ID) {
+return;
+  }
+
+  llvm::json::Object event(CreateEventObject("continued"));
+  llvm::json::Object body;
+  body.try_emplace("threadId", (int64_t)g_vsc.focus_tid);
+  body.try_emplace("allThreadsContinued", true);
+  event.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(event)));
+}
+
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
@@ -252,7 +274,7 @@ void SendThreadStoppedEvent() {
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // We will have cleared g_vsc.focus_tid if the focus thread doesn't have
   // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
   // then set the focus thread to the first thread with a stop reason.
   if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
@@ -262,6 +284,7 @@ void SendThreadStoppedEvent() {
   // we at least let the UI know we stopped.
   if (num_threads_with_reason == 0) {
 lldb::SBThread thread = process.GetThreadAtIndex(0);
+g_vsc.focus_tid = thread.GetThreadID();
 g_vsc.SendJSON(CreateThreadStopped(thread, stop_id));
   } else {
 for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
@@ -468,6 +491,7 @@ void EventThreadFunction() {
 break;
   case lldb::eStateRunning:
 g_vsc.WillContinue();
+SendContinuedEvent();
 break;
   case lldb::eStateExited:
 // When restarting, we can get an "exited" event for the process we
@@ -766,10 +790,6 @@ void request_continue(const llvm::json::Object ) {
   llvm::json::Object response;
   FillResponse(request, response);
   lldb::SBProcess process = g_vsc.target.GetProcess();
-  auto arguments = request.getObject("arguments");
-  // Remember the thread ID that caused the resume so we can set the
-  // "threadCausedFocus" boolean value in the "stopped" events.
-  g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);




[Lldb-commits] [PATCH] D154989: [lldb-vsocde] Add a 'continued' event for programmatic continue events.

2023-07-25 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

+1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154989/new/

https://reviews.llvm.org/D154989

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156066: [lldb][LocateModuleCallback] Call locate module callback in Platform too

2023-07-25 Thread Kazuki Sakamoto via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c61c9b02b49: [lldb][LocateModuleCallback] Call locate 
module callback in Platform too (authored by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156066/new/

https://reviews.llvm.org/D156066

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Target/LocateModuleCallbackTest.cpp

Index: lldb/unittests/Target/LocateModuleCallbackTest.cpp
===
--- lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -231,6 +231,7 @@
 EXPECT_TRUE(m_process_sp);
 
 m_module_spec = GetTestModuleSpec();
+m_module_spec_without_uuid = ModuleSpec(GetRemotePath(), ArchSpec(k_arch));
   }
 
   void TearDown() override {
@@ -244,15 +245,33 @@
   }
 
   void CheckCallbackArgs(const ModuleSpec _spec,
- FileSpec _file_spec,
- FileSpec _file_spec) {
-EXPECT_TRUE(m_module_spec.Matches(module_spec,
-  /*exact_arch_match=*/true));
+ FileSpec _file_spec, FileSpec _file_spec,
+ const ModuleSpec _module_spec,
+ int expected_callback_call_count) {
+EXPECT_TRUE(expected_module_spec.Matches(module_spec,
+ /*exact_arch_match=*/true));
 EXPECT_FALSE(module_file_spec);
 EXPECT_FALSE(symbol_file_spec);
 
-EXPECT_EQ(m_callback_call_count, 0);
-m_callback_call_count++;
+EXPECT_EQ(++m_callback_call_count, expected_callback_call_count);
+  }
+
+  void CheckCallbackArgsWithUUID(const ModuleSpec _spec,
+ FileSpec _file_spec,
+ FileSpec _file_spec,
+ int expected_callback_call_count) {
+CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec,
+  m_module_spec, expected_callback_call_count);
+EXPECT_TRUE(module_spec.GetUUID().IsValid());
+  }
+
+  void CheckCallbackArgsWithoutUUID(const ModuleSpec _spec,
+FileSpec _file_spec,
+FileSpec _file_spec,
+int expected_callback_call_count) {
+CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec,
+  m_module_spec_without_uuid, expected_callback_call_count);
+EXPECT_FALSE(module_spec.GetUUID().IsValid());
   }
 
 protected:
@@ -262,6 +281,7 @@
   TargetSP m_target_sp;
   ProcessSP m_process_sp;
   ModuleSpec m_module_spec;
+  ModuleSpec m_module_spec_without_uuid;
   ModuleSP m_module_sp;
   int m_callback_call_count = 0;
 };
@@ -331,14 +351,18 @@
   // download the module and fails.
   BuildEmptyCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec _spec,
-FileSpec _file_spec,
-FileSpec _file_spec) {
-CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-return Status("The locate module callback failed");
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+  [this, _call_count](const ModuleSpec _spec,
+   FileSpec _file_spec,
+   FileSpec _file_spec) {
+CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+  symbol_file_spec, ++callback_call_count);
+return Status("The locate module callback failed");
+  });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   ASSERT_FALSE(m_module_sp);
 }
 
@@ -348,14 +372,18 @@
   // some reason.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec _spec,
-FileSpec _file_spec,
-FileSpec _file_spec) {
-CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-return Status("The locate module callback failed");
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+  [this, _call_count](const ModuleSpec _spec,
+   FileSpec _file_spec,
+   FileSpec _file_spec) {
+CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+  symbol_file_spec, ++callback_call_count);
+return Status("The locate module callback failed");
+  });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, 

[Lldb-commits] [lldb] 8c61c9b - [lldb][LocateModuleCallback] Call locate module callback in Platform too

2023-07-25 Thread Kazuki Sakamoto via lldb-commits

Author: Kazuki Sakamoto
Date: 2023-07-25T10:57:50-07:00
New Revision: 8c61c9b02b492b00179e6e2cd95d2c54dac69a0e

URL: 
https://github.com/llvm/llvm-project/commit/8c61c9b02b492b00179e6e2cd95d2c54dac69a0e
DIFF: 
https://github.com/llvm/llvm-project/commit/8c61c9b02b492b00179e6e2cd95d2c54dac69a0e.diff

LOG: [lldb][LocateModuleCallback] Call locate module callback in Platform too

This is an enhancement for the locate module callback.
https://discourse.llvm.org/t/rfc-python-callback-for-target-get-module/71580/6

On Android remote platform, module UUID is resolved by
Platform::GetRemoteSharedModule. Which means the current
Target::CallLocateModuleCallbackIfSet() call undesirably is not able to pass the
module UUID to the locate module callback.

This diff moves the CallLocateModuleCallbackIfSet() implementation from Target
to Platform to allows both Target and Platform can call it. One is from the
current Target call site, and second is from Platform after resolving the module
UUID.

As the result of this change, the locate module callback may be called twice
for a same module on remote platforms. And it should be ok.

- First, without UUID.
  - The locate module callback is allowed to return an error
if the callback requires UUID.
- Second, with UUID, if the first callback call did not return a module.

Differential Revision: https://reviews.llvm.org/D156066

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Target/Target.h
lldb/source/Target/Platform.cpp
lldb/source/Target/Target.cpp
lldb/unittests/Target/LocateModuleCallbackTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index a5a78fe0f86983..583e9a2e5a4c92 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -293,6 +293,11 @@ class Platform : public PluginInterface {
   lldb::ModuleSP _sp, const FileSpecList *module_search_paths_ptr,
   llvm::SmallVectorImpl *old_modules, bool 
*did_create_ptr);
 
+  void CallLocateModuleCallbackIfSet(const ModuleSpec _spec,
+ lldb::ModuleSP _sp,
+ FileSpec _file_spec,
+ bool *did_create_ptr);
+
   virtual bool GetModuleSpec(const FileSpec _file_spec,
  const ArchSpec , ModuleSpec _spec);
 

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index d5b4b8e18c2d2d..ed0ecbbddbf814 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1625,12 +1625,6 @@ class Target : public 
std::enable_shared_from_this,
 
   Target(const Target &) = delete;
   const Target =(const Target &) = delete;
-
-private:
-  void CallLocateModuleCallbackIfSet(const ModuleSpec _spec,
- lldb::ModuleSP _sp,
- FileSpec _file_spec,
- bool _create_module);
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 11a123fb6d583d..2802033cf63051 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1564,14 +1564,167 @@ Status Platform::GetRemoteSharedModule(const 
ModuleSpec _spec,
 resolved_module_spec.GetUUID() = module_spec.GetUUID();
   }
 
+  // Call locate module callback if set. This allows users to implement their
+  // own module cache system. For example, to leverage build system artifacts,
+  // to bypass pulling files from remote platform, or to search symbol files
+  // from symbol servers.
+  FileSpec symbol_file_spec;
+  CallLocateModuleCallbackIfSet(resolved_module_spec, module_sp,
+symbol_file_spec, did_create_ptr);
+  if (module_sp) {
+// The module is loaded.
+if (symbol_file_spec) {
+  // 1. module_sp:loaded, symbol_file_spec:set
+  //  The callback found a module file and a symbol file for this
+  //  resolved_module_spec. Set the symbol file to the module.
+  module_sp->SetSymbolFileFileSpec(symbol_file_spec);
+} else {
+  // 2. module_sp:loaded, symbol_file_spec:empty
+  //  The callback only found a module file for this
+  //  resolved_module_spec.
+}
+return Status();
+  }
+
+  // The module is not loaded by CallLocateModuleCallbackIfSet.
+  // 3. module_sp:empty, symbol_file_spec:set
+  //  The callback only found a symbol file for the module. We continue to
+  //  find a module file for this resolved_module_spec. and we will call
+  //  module_sp->SetSymbolFileFileSpec with the symbol_file_spec later.
+  // 4. module_sp:empty, symbol_file_spec:empty
+  //  The callback is not set. Or the callback did not find any module
+  //  files nor any symbol files. Or the 

[Lldb-commits] [PATCH] D156066: [lldb][LocateModuleCallback] Call locate module callback in Platform too

2023-07-25 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

lgtm! I like this being in platform better


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156066/new/

https://reviews.llvm.org/D156066

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D154989: [lldb-vsocde] Add a 'continued' event for programmatic continue events.

2023-07-25 Thread John Harrison via Phabricator via lldb-commits
ashgti updated this revision to Diff 544017.
ashgti added a comment.

Adding back the 'threadCausedFocus' field, that is used in tests to verify 
behavior.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154989/new/

https://reviews.llvm.org/D154989

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -191,6 +191,28 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// Send a "continued" event to indicate the process is in the running state.
+void SendContinuedEvent() {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  if (!process.IsValid()) {
+return;
+  }
+
+  // If the focus thread is not set then we haven't reported any thread status
+  // to the client, so nothing to report.
+  if (!g_vsc.configuration_done_sent ||
+  g_vsc.focus_tid == LLDB_INVALID_THREAD_ID) {
+return;
+  }
+
+  llvm::json::Object event(CreateEventObject("continued"));
+  llvm::json::Object body;
+  body.try_emplace("threadId", (int64_t)g_vsc.focus_tid);
+  body.try_emplace("allThreadsContinued", true);
+  event.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(event)));
+}
+
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
@@ -252,7 +274,7 @@
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // We will have cleared g_vsc.focus_tid if the focus thread doesn't have
   // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
   // then set the focus thread to the first thread with a stop reason.
   if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
@@ -262,6 +284,7 @@
   // we at least let the UI know we stopped.
   if (num_threads_with_reason == 0) {
 lldb::SBThread thread = process.GetThreadAtIndex(0);
+g_vsc.focus_tid = thread.GetThreadID();
 g_vsc.SendJSON(CreateThreadStopped(thread, stop_id));
   } else {
 for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
@@ -468,6 +491,7 @@
 break;
   case lldb::eStateRunning:
 g_vsc.WillContinue();
+SendContinuedEvent();
 break;
   case lldb::eStateExited:
 // When restarting, we can get an "exited" event for the process we
@@ -766,10 +790,6 @@
   llvm::json::Object response;
   FillResponse(request, response);
   lldb::SBProcess process = g_vsc.target.GetProcess();
-  auto arguments = request.getObject("arguments");
-  // Remember the thread ID that caused the resume so we can set the
-  // "threadCausedFocus" boolean value in the "stopped" events.
-  g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -907,6 +907,8 @@
   uint64_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
   snprintf(desc_str, sizeof(desc_str), "breakpoint %" PRIu64 ".%" PRIu64,
bp_id, bp_loc_id);
+  body.try_emplace("hitBreakpointIds",
+   llvm::json::Array{llvm::json::Value(bp_id)});
   EmplaceSafeString(body, "description", desc_str);
 }
   } break;
@@ -951,6 +953,7 @@
   EmplaceSafeString(body, "description", std::string(description));
 }
   }
+  // "threadCausedFocus" is used in tests to validate breaking behavior.
   if (tid == g_vsc.focus_tid) {
 body.try_emplace("threadCausedFocus", true);
   }


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -191,6 +191,28 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// Send a "continued" event to indicate the process is in the running state.
+void SendContinuedEvent() {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  if (!process.IsValid()) {
+return;
+  }
+
+  // If the focus thread is not set then we haven't reported any thread status
+  // to the client, so nothing to report.
+  if (!g_vsc.configuration_done_sent ||
+  g_vsc.focus_tid == LLDB_INVALID_THREAD_ID) {
+return;
+  }
+
+  llvm::json::Object event(CreateEventObject("continued"));
+  llvm::json::Object body;
+  body.try_emplace("threadId", (int64_t)g_vsc.focus_tid);
+  body.try_emplace("allThreadsContinued", true);
+  event.try_emplace("body", 

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

2023-07-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 543999.
DavidSpickett added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Rebase, add release note before I forget.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156118/new/

https://reviews.llvm.org/D156118

Files:
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -131,6 +131,9 @@
 Changes to LLDB
 -
 
+* AArch64 Linux targets now provide access to the Thread Local Storage
+  register ``tpidr``.
+
 Changes to Sanitizers
 -
 
Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
===
--- lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
@@ -1,6 +1,8 @@
 // compile with -march=armv8-a+simd on compatible aarch64 compiler
 // linux-aarch64-neon.core was generated by: aarch64-linux-gnu-gcc-8
 // commandline: -march=armv8-a+simd -nostdlib -static -g linux-aarch64-neon.c
+#include 
+
 static void bar(char *boom) {
   char F = 'b';
   asm volatile("fmov d0,  #0.5\n\t");
@@ -14,6 +16,9 @@
   asm volatile("movi v8.16b, #0x11\n\t");
   asm volatile("movi v31.16b, #0x30\n\t");
 
+  uint64_t pattern = 0x1122334455667788;
+  asm volatile("msr tpidr_el0, %0" ::"r"(pattern));
+
   *boom = 47; // Frame bar
 }
 
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -304,10 +304,10 @@
 values = {}
 values["x1"] = "0x002f"
 values["w1"] = "0x002f"
-values["fp"] = "0x007fc5dd7f20"
-values["lr"] = "0x00400180"
-values["sp"] = "0x007fc5dd7f00"
-values["pc"] = "0x0040014c"
+values["fp"] = "0xdab7c770"
+values["lr"] = "0x0040019c"
+values["sp"] = "0xdab7c750"
+values["pc"] = "0x00400168"
 values[
 "v0"
 ] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
@@ -366,6 +366,7 @@
 values["d31"] = "1.3980432860952889E-76"
 values["fpsr"] = "0x"
 values["fpcr"] = "0x"
+values["tpidr"] = "0x1122334455667788"
 
 for regname, value in values.items():
 self.expect(
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -123,6 +123,10 @@
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 };
 
+constexpr RegsetDesc AARCH64_TLS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_TLS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -57,6 +57,7 @@
   lldb_private::DataExtractor m_fpr_data;
   lldb_private::DataExtractor m_sve_data;
   lldb_private::DataExtractor m_pac_data;
+  lldb_private::DataExtractor m_tls_data;
 
   SVEState m_sve_state;
   uint16_t m_sve_vector_length = 0;
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ 

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-25 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 543967.
Michael137 marked an inline comment as not done.
Michael137 added a comment.

- Update unit-tests
- Expand function docs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156020/new/

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,181 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // 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.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  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:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x0010
+- CStr:"something.invalid.sdk"
+- CStr:"/invalid/path/to/something.invalid.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF _file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 4U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+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:0x0001
+  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
+  

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-25 Thread Michael Buch via Phabricator via lldb-commits
Michael137 marked 2 inline comments as not done.
Michael137 added inline comments.



Comment at: lldb/include/lldb/Target/Platform.h:479
+/// to an internal SDK
+bool found_internal_sdk = false;
+

Michael137 wrote:
> aprantl wrote:
> > 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.
> Yup I think that'd be better. That'll also make it easier to use from the 
> Swift plugin
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 [[ 
https://github.com/apple/llvm-project/blob/6c39bfc9d521dd8af03ca5e9e6ec7d5d4a6e5e6e/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp#L1700-L1704
 | 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 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156020/new/

https://reviews.llvm.org/D156020

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-25 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1363
+  XcodeSDK sdk;
+  for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i)
+if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i))

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 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156020/new/

https://reviews.llvm.org/D156020

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-25 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 543875.
Michael137 added a comment.

- Remove more headers


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156020/new/

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,169 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // 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.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  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
+- Code:0x0002
+  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:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF _file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+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:0x0001
+  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:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = PlatformDarwin::ResolveSDKPathFromDebugInfo(*module);
+  

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-25 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 543874.
Michael137 added a comment.

- Remove redundant header


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156020/new/

https://reviews.llvm.org/D156020

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,169 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // 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.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  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
+- Code:0x0002
+  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:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF _file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+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:0x0001
+  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:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"1abc@defgh2"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto path_or_err = 

[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

2023-07-25 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 543870.
Michael137 added a comment.

- 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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156020/new/

https://reviews.llvm.org/D156020

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Target/Platform.cpp
  lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -12,6 +12,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "TestingSupport/Symbol/YAMLModuleTester.h"
 #include "lldb/Core/PluginManager.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -72,4 +73,169 @@
   ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
   ASSERT_EQ(module->GetSourceMappingList().GetSize(), 1u);
 }
+
+TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InternalAndPublicSDK) {
+  // 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.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  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
+- Code:0x0002
+  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:
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+- CStr:"MacOSX10.9.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/MacOSX10.9.sdk"
+- AbbrCode:0x
+- Version: 2
+  AddrSize:8
+  AbbrevTableID:   0
+  AbbrOffset:  0x0
+  Entries:
+- AbbrCode:0x0002
+  Values:
+- Value:   0x0010
+- CStr:"iPhoneOS14.0.Internal.sdk"
+- CStr:"/Library/Developer/CommandLineTools/SDKs/iPhoneOS14.0.Internal.sdk"
+- AbbrCode:0x
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+  DWARFUnit *dwarf_unit = t.GetDwarfUnit();
+  auto *dwarf_cu = llvm::cast(dwarf_unit);
+  ASSERT_TRUE(static_cast(dwarf_cu));
+  SymbolFileDWARF _file = dwarf_cu->GetSymbolFileDWARF();
+  ASSERT_EQ(sym_file.GetNumCompileUnits(), 2U);
+  ModuleSP module = t.GetModule();
+  ASSERT_NE(module, nullptr);
+
+  auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module);
+  ASSERT_TRUE(static_cast(sdk_or_err));
+
+  ASSERT_TRUE(sdk_or_err->IsAppleInternalSDK());
+  ASSERT_NE(sdk_or_err->GetString().find("Internal.sdk"), std::string::npos);
+}
+
+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:0x0001
+  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:0x0001
+  Values:
+- Value:   0x000C
+-