[Lldb-commits] [lldb] [lldb][Darwin] Save "most recent SDK root" in local var (PR #200095)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)

<details>
<summary>Changes</summary>

In PlatformRemoteDarwinDevice::GetSharedModule we have an array of "sdk root" directories (aka DeviceSupport directories) for a remote device, which contain all of the system shared libraries on the local mac.  We may have 6 SDK Root directories for different OS builds.  We try to identify the correct one for the target based on version number, but when we find a file with a matching UUID in one directory we set that as the "most recently successful" directory, and start our searches with that one.  Then we fall back to searching all the other directories.

Since adding parallel module loading, because this method doesn't lock access to the "most recently successful directory" (m_last_module_sdk_idx), we could check the m_last_module_sdk_idx directory, and then if that was unsuccessful, check all other entries != m_last_module_sdk_idx.  But m_last_module_sdk_idx could mutate via another thread, and we skip one of our SDK Root directories. Resulting in reading a binary out of memory, when we had a local copy of it.

This PR reads the value of m_last_module_sdk_idx into a local const, and uses the same local index to check for it -- and if not found, to skip that directory when iterating over all directories again, instead of adding locking around access to this.

The array itself is created when the Platform is initialized and not mutated when parallel-binary-loading, so there are no concerns with that.

rdar://178096480

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


1 Files Affected:

- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp (+7-6) 


``````````diff
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
index ba7f97383f158..0f2667caa39fe 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
@@ -175,7 +175,7 @@ Status PlatformRemoteDarwinDevice::GetSharedModule(
 
     const uint32_t num_sdk_infos = m_sdk_directory_infos.size();
 
-    // If we are connected we migth be able to correctly deduce the SDK
+    // If we are connected we might be able to correctly deduce the SDK
     // directory using the OS build.
     const uint32_t connected_sdk_idx = GetConnectedSDKIndex();
     if (connected_sdk_idx < num_sdk_infos) {
@@ -193,12 +193,13 @@ Status PlatformRemoteDarwinDevice::GetSharedModule(
       }
     }
 
+    const uint32_t last_module_sdk_idx = m_last_module_sdk_idx;
     // Try the last SDK index if it is set as most files from an SDK will tend
     // to be valid in that same SDK.
-    if (m_last_module_sdk_idx < num_sdk_infos) {
+    if (last_module_sdk_idx < num_sdk_infos) {
       LLDB_LOG_VERBOSE(log, "Searching for {0} in sdk path {1}", platform_file,
-                       m_sdk_directory_infos[m_last_module_sdk_idx].directory);
-      if (GetFileInSDK(platform_file_path, m_last_module_sdk_idx,
+                       m_sdk_directory_infos[last_module_sdk_idx].directory);
+      if (GetFileInSDK(platform_file_path, last_module_sdk_idx,
                        platform_module_spec.GetFileSpec())) {
         module_sp.reset();
         error = ResolveExecutable(platform_module_spec, module_sp);
@@ -217,7 +218,7 @@ Status PlatformRemoteDarwinDevice::GetSharedModule(
     const uint32_t current_sdk_idx =
         GetSDKIndexBySDKDirectoryInfo(current_sdk_info);
     if (current_sdk_idx < num_sdk_infos &&
-        current_sdk_idx != m_last_module_sdk_idx) {
+        current_sdk_idx != last_module_sdk_idx) {
       LLDB_LOG_VERBOSE(log, "Searching for {0} in sdk path {1}", platform_file,
                        m_sdk_directory_infos[current_sdk_idx].directory);
       if (GetFileInSDK(platform_file_path, current_sdk_idx,
@@ -234,7 +235,7 @@ Status PlatformRemoteDarwinDevice::GetSharedModule(
 
     // Second try all SDKs that were found.
     for (uint32_t sdk_idx = 0; sdk_idx < num_sdk_infos; ++sdk_idx) {
-      if (m_last_module_sdk_idx == sdk_idx) {
+      if (last_module_sdk_idx == sdk_idx) {
         // Skip the last module SDK index if we already searched it above
         continue;
       }

``````````

</details>


https://github.com/llvm/llvm-project/pull/200095