[Lldb-commits] [PATCH] D136900: Don't test dyld_process_create_for_current_task symbol before calling it in HostInfoMacOSX.mm

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: aprantl.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

In a patch Fred landed a year and a half ago, he checks for a header to be available before enabling a block of code, and within that block of code, tests that a symbol, dyld_process_create_for_current_task(), is actually available before calling it - a trick with darwin weak leaking.  This SPI has been present in the system since macOS 10.12 (macOS Sierra, released September 2016).  We don't need to test for this symbol; this would only happen if you do have the new API on the build host, and were setting a deployment target of macOS 10.12, but you took that binary and tried to run it on an older system.

It also happens to generate a "always evaluates to true" warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136900

Files:
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm


Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -530,48 +530,43 @@
 
 bool SharedCacheInfo::CreateSharedCacheInfoWithInstrospectionSPIs() {
 #if defined(SDK_HAS_NEW_DYLD_INTROSPECTION_SPIS)
-  if (__builtin_available(macOS 12, *)) {
-    if (dyld_process_create_for_current_task) {
-      dyld_process_t dyld_process = dyld_process_create_for_current_task();
-      if (!dyld_process)
-        return false;
-
-      dyld_process_snapshot_t snapshot =
-          dyld_process_snapshot_create_for_process(dyld_process, nullptr);
-      if (!snapshot)
-        return false;
-
-      auto on_exit = llvm::make_scope_exit(
-          [&]() { dyld_process_snapshot_dispose(snapshot); });
-
-      dyld_shared_cache_t shared_cache =
-          dyld_process_snapshot_get_shared_cache(snapshot);
-      if (!shared_cache)
-        return false;
-
-      dyld_shared_cache_for_each_image(shared_cache, ^(dyld_image_t image) {
-        __block uint64_t minVmAddr = UINT64_MAX;
-        __block uint64_t maxVmAddr = 0;
-        uuid_t uuidStore;
-        __block uuid_t *uuid = &uuidStore;
-
-        dyld_image_for_each_segment_info(image, ^(const char *segmentName,
-                                                  uint64_t vmAddr,
-                                                  uint64_t vmSize, int perm) {
+  dyld_process_t dyld_process = dyld_process_create_for_current_task();
+  if (!dyld_process)
+    return false;
+
+  dyld_process_snapshot_t snapshot =
+      dyld_process_snapshot_create_for_process(dyld_process, nullptr);
+  if (!snapshot)
+    return false;
+
+  auto on_exit =
+      llvm::make_scope_exit([&]() { dyld_process_snapshot_dispose(snapshot); });
+
+  dyld_shared_cache_t shared_cache =
+      dyld_process_snapshot_get_shared_cache(snapshot);
+  if (!shared_cache)
+    return false;
+
+  dyld_shared_cache_for_each_image(shared_cache, ^(dyld_image_t image) {
+    __block uint64_t minVmAddr = UINT64_MAX;
+    __block uint64_t maxVmAddr = 0;
+    uuid_t uuidStore;
+    __block uuid_t *uuid = &uuidStore;
+
+    dyld_image_for_each_segment_info(
+        image,
+        ^(const char *segmentName, uint64_t vmAddr, uint64_t vmSize, int perm) {
           minVmAddr = std::min(minVmAddr, vmAddr);
           maxVmAddr = std::max(maxVmAddr, vmAddr + vmSize);
           dyld_image_copy_uuid(image, uuid);
         });
-        assert(minVmAddr != UINT_MAX);
-        assert(maxVmAddr != 0);
-        m_images[dyld_image_get_installname(image)] = SharedCacheImageInfo{
-            UUID(uuid, 16),
-            std::make_shared<DataBufferUnowned>((uint8_t *)minVmAddr,
-                                                maxVmAddr - minVmAddr)};
-      });
-      return true;
-    }
-  }
+    assert(minVmAddr != UINT_MAX);
+    assert(maxVmAddr != 0);
+    m_images[dyld_image_get_installname(image)] = SharedCacheImageInfo{
+        UUID(uuid, 16), std::make_shared<DataBufferUnowned>(
+                            (uint8_t *)minVmAddr, maxVmAddr - minVmAddr)};
+  });
+  return true;
 #endif
   return false;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136900.471321.patch
Type: text/x-patch
Size: 3221 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20221027/73ce49af/attachment.bin>