JDevlieghere created this revision.
JDevlieghere added a reviewer: jingham.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLDB.

In order to avoid stranding the Objective-C runtime lock, we switched from 
`objc_copyRealizedClassList` to its non locking variant 
`objc_copyRealizedClassList_nolock`. Not taking the lock was relatively safe 
because we run this expression on one thread only, but it was still possible 
that someone was in the middle of modifying this list while we were trying to 
read it. Worst case that would result in a crash in the inferior without 
side-effects and we'd unwind and try again later.

With the introduction of macOS Ventura, we can use 
`objc_getRealizedClassList_trylock` instead. It has semantics similar to 
`objc_copyRealizedClassList_nolock`, but instead of not locking at all, the 
function returns if the lock is already taken, which avoids the aforementioned 
crash without stranding the Objective-C runtime lock. Because LLDB gets to 
allocate the underlying memory we also avoid stranding the malloc lock.

rdar://89373233


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127252

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -313,13 +313,15 @@
   };
 
   /// We can read the class info from the Objective-C runtime using
-  /// gdb_objc_realized_classes or objc_copyRealizedClassList. The latter is
-  /// preferred because it includes lazily named classes, but it's not always
-  /// available or safe to call.
+  /// gdb_objc_realized_classes, objc_copyRealizedClassList or
+  /// objc_getRealizedClassList_trylock. The RealizedClassList variants are
+  /// preferred because it includes lazily named classes, but they are not
+  /// always available or safe to call.
   ///
-  /// We potentially need both for the same process, because we may need to use
-  /// gdb_objc_realized_classes until dyld is initialized and then switch over
-  /// to objc_copyRealizedClassList for lazily named classes.
+  /// We potentially need more than one helper for the same process, because we
+  /// may need to use gdb_objc_realized_classes until dyld is initialized and
+  /// then switch over to objc_copyRealizedClassList or
+  /// objc_getRealizedClassList_trylock for lazily named classes.
   class DynamicClassInfoExtractor : public ClassInfoExtractor {
   public:
     DynamicClassInfoExtractor(AppleObjCRuntimeV2 &runtime)
@@ -329,11 +331,16 @@
     UpdateISAToDescriptorMap(RemoteNXMapTable &hash_table);
 
   private:
-    enum Helper { gdb_objc_realized_classes, objc_copyRealizedClassList };
+    enum Helper {
+      gdb_objc_realized_classes,
+      objc_copyRealizedClassList,
+      objc_getRealizedClassList_trylock
+    };
 
-    /// Compute which helper to use. Prefer objc_copyRealizedClassList if it's
-    /// available and it's safe to call (i.e. dyld is fully initialized). Use
-    /// gdb_objc_realized_classes otherwise.
+    /// Compute which helper to use. If dyld is not yet fully initialized we
+    /// must use gdb_objc_realized_classes. Otherwise, we prefer
+    /// objc_getRealizedClassList_trylock and objc_copyRealizedClassList
+    /// respectively, depending on availability.
     Helper ComputeHelper() const;
 
     UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
@@ -341,8 +348,8 @@
     lldb::addr_t &GetClassInfoArgs(Helper helper);
 
     std::unique_ptr<UtilityFunction>
-    GetClassInfoUtilityFunctionImpl(ExecutionContext &exe_ctx, std::string code,
-                                    std::string name);
+    GetClassInfoUtilityFunctionImpl(ExecutionContext &exe_ctx, Helper helper,
+                                    std::string code, std::string name);
 
     /// Helper to read class info using the gdb_objc_realized_classes.
     struct gdb_objc_realized_classes_helper {
@@ -356,8 +363,16 @@
       lldb::addr_t args = LLDB_INVALID_ADDRESS;
     };
 
+    /// Helper to read class info using objc_getRealizedClassList_trylock.
+    struct objc_getRealizedClassList_trylock_helper {
+      std::unique_ptr<UtilityFunction> utility_function;
+      lldb::addr_t args = LLDB_INVALID_ADDRESS;
+    };
+
     gdb_objc_realized_classes_helper m_gdb_objc_realized_classes_helper;
     objc_copyRealizedClassList_helper m_objc_copyRealizedClassList_helper;
+    objc_getRealizedClassList_trylock_helper
+        m_objc_getRealizedClassList_trylock_helper;
   };
 
   /// Abstraction to read the Objective-C class info from the shared cache.
@@ -429,6 +444,7 @@
   HashTableSignature m_hash_signature;
   bool m_has_object_getClass;
   bool m_has_objc_copyRealizedClassList;
+  bool m_has_objc_getRealizedClassList_trylock;
   bool m_loaded_objc_opt;
   std::unique_ptr<NonPointerISACache> m_non_pointer_isa_cache_up;
   std::unique_ptr<TaggedPointerVendor> m_tagged_pointer_vendor_up;
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -235,6 +235,78 @@
 }
 )";
 
+static const char *g_get_dynamic_class_info3_name =
+    "__lldb_apple_objc_v2_get_dynamic_class_info3";
+
+static const char *g_get_dynamic_class_info3_body = R"(
+
+extern "C" {
+    int printf(const char * format, ...);
+    void free(void *ptr);
+    size_t objc_getRealizedClassList_trylock(Class *buffer, size_t len);
+    const char* objc_debug_class_getNameRaw(Class cls);
+}
+
+#define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
+
+struct ClassInfo
+{
+    Class isa;
+    uint32_t hash;
+} __attribute__((__packed__));
+
+uint32_t
+__lldb_apple_objc_v2_get_dynamic_class_info3(void *gdb_objc_realized_classes_ptr,
+                                             void *class_infos_ptr,
+                                             uint32_t class_infos_byte_size,
+                                             void *class_buffer,
+                                             uint32_t class_buffer_len,
+                                             uint32_t should_log)
+{
+    DEBUG_PRINTF ("class_infos_ptr = %p\n", class_infos_ptr);
+    DEBUG_PRINTF ("class_infos_byte_size = %u\n", class_infos_byte_size);
+
+    const size_t max_class_infos = class_infos_byte_size/sizeof(ClassInfo);
+    DEBUG_PRINTF ("max_class_infos = %u\n", max_class_infos);
+
+    ClassInfo *class_infos = (ClassInfo *)class_infos_ptr;
+
+    Class *realized_class_list = (Class*)class_buffer;
+
+    uint32_t count = objc_getRealizedClassList_trylock(realized_class_list,
+                                                       class_buffer_len);
+    DEBUG_PRINTF ("count = %u\n", count);
+
+    uint32_t idx = 0;
+    for (uint32_t i=0; i<=count; ++i)
+    {
+        if (idx < max_class_infos)
+        {
+            Class isa = realized_class_list[i];
+            const char *name_ptr = objc_debug_class_getNameRaw(isa);
+            if (name_ptr == NULL)
+                continue;
+            const char *s = name_ptr;
+            uint32_t h = 5381;
+            for (unsigned char c = *s; c; c = *++s)
+                h = ((h << 5) + h) + c;
+            class_infos[idx].hash = h;
+            class_infos[idx].isa = isa;
+            DEBUG_PRINTF ("[%u] isa = %8p %s\n", idx, class_infos[idx].isa, name_ptr);
+        }
+        idx++;
+    }
+
+    if (idx < max_class_infos)
+    {
+        class_infos[idx].isa = NULL;
+        class_infos[idx].hash = 0;
+    }
+
+    return count;
+}
+)";
+
 // We'll substitute in class_getName or class_getNameRaw depending
 // on which is present.
 static const char *g_shared_cache_class_name_funcptr = R"(
@@ -663,7 +735,8 @@
       m_isa_hash_table_ptr(LLDB_INVALID_ADDRESS),
       m_relative_selector_base(LLDB_INVALID_ADDRESS), m_hash_signature(),
       m_has_object_getClass(false), m_has_objc_copyRealizedClassList(false),
-      m_loaded_objc_opt(false), m_non_pointer_isa_cache_up(),
+      m_has_objc_getRealizedClassList_trylock(false), m_loaded_objc_opt(false),
+      m_non_pointer_isa_cache_up(),
       m_tagged_pointer_vendor_up(
           TaggedPointerVendorV2::CreateInstance(*this, objc_module_sp)),
       m_encoding_to_type_sp(), m_CFBoolean_values(),
@@ -672,7 +745,11 @@
   m_has_object_getClass = HasSymbol(g_gdb_object_getClass);
   static const ConstString g_objc_copyRealizedClassList(
       "_ZL33objc_copyRealizedClassList_nolockPj");
+  static const ConstString g_objc_getRealizedClassList_trylock(
+      "_objc_getRealizedClassList_trylock");
   m_has_objc_copyRealizedClassList = HasSymbol(g_objc_copyRealizedClassList);
+  m_has_objc_getRealizedClassList_trylock =
+      HasSymbol(g_objc_getRealizedClassList_trylock);
   WarnIfNoExpandedSharedCache();
   RegisterObjCExceptionRecognizer(process);
 }
@@ -1539,7 +1616,8 @@
 
 std::unique_ptr<UtilityFunction>
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoUtilityFunctionImpl(
-    ExecutionContext &exe_ctx, std::string code, std::string name) {
+    ExecutionContext &exe_ctx, Helper helper, std::string code,
+    std::string name) {
   Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
 
   LLDB_LOG(log, "Creating utility function {0}", name);
@@ -1574,6 +1652,15 @@
   value.SetValueType(Value::ValueType::Scalar);
   value.SetCompilerType(clang_uint32_t_type);
   arguments.PushValue(value);
+
+  // objc_getRealizedClassList_trylock takes an additional buffer and length.
+  if (helper == Helper::objc_getRealizedClassList_trylock) {
+    value.SetCompilerType(clang_void_pointer_type);
+    arguments.PushValue(value);
+    value.SetCompilerType(clang_uint32_t_type);
+    arguments.PushValue(value);
+  }
+
   arguments.PushValue(value);
 
   std::unique_ptr<UtilityFunction> utility_fn = std::move(*utility_fn_or_error);
@@ -1599,7 +1686,7 @@
   case gdb_objc_realized_classes: {
     if (!m_gdb_objc_realized_classes_helper.utility_function)
       m_gdb_objc_realized_classes_helper.utility_function =
-          GetClassInfoUtilityFunctionImpl(exe_ctx,
+          GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
                                           g_get_dynamic_class_info_body,
                                           g_get_dynamic_class_info_name);
     return m_gdb_objc_realized_classes_helper.utility_function.get();
@@ -1607,11 +1694,19 @@
   case objc_copyRealizedClassList: {
     if (!m_objc_copyRealizedClassList_helper.utility_function)
       m_objc_copyRealizedClassList_helper.utility_function =
-          GetClassInfoUtilityFunctionImpl(exe_ctx,
+          GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
                                           g_get_dynamic_class_info2_body,
                                           g_get_dynamic_class_info2_name);
     return m_objc_copyRealizedClassList_helper.utility_function.get();
   }
+  case objc_getRealizedClassList_trylock: {
+    if (!m_objc_getRealizedClassList_trylock_helper.utility_function)
+      m_objc_copyRealizedClassList_helper.utility_function =
+          GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
+                                          g_get_dynamic_class_info3_body,
+                                          g_get_dynamic_class_info3_name);
+    return m_objc_copyRealizedClassList_helper.utility_function.get();
+  }
   }
   llvm_unreachable("Unexpected helper");
 }
@@ -1623,19 +1718,26 @@
     return m_gdb_objc_realized_classes_helper.args;
   case objc_copyRealizedClassList:
     return m_objc_copyRealizedClassList_helper.args;
+  case objc_getRealizedClassList_trylock:
+    return m_objc_getRealizedClassList_trylock_helper.args;
   }
   llvm_unreachable("Unexpected helper");
 }
 
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::Helper
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper() const {
-  if (!m_runtime.m_has_objc_copyRealizedClassList)
+  if (!m_runtime.m_has_objc_copyRealizedClassList &&
+      !m_runtime.m_has_objc_getRealizedClassList_trylock)
     return DynamicClassInfoExtractor::gdb_objc_realized_classes;
 
   if (Process *process = m_runtime.GetProcess()) {
     if (DynamicLoader *loader = process->GetDynamicLoader()) {
-      if (loader->IsFullyInitialized())
-        return DynamicClassInfoExtractor::objc_copyRealizedClassList;
+      if (loader->IsFullyInitialized()) {
+        if (m_runtime.m_has_objc_getRealizedClassList_trylock)
+          return DynamicClassInfoExtractor::objc_getRealizedClassList_trylock;
+        if (m_runtime.m_has_objc_copyRealizedClassList)
+          return DynamicClassInfoExtractor::objc_copyRealizedClassList;
+      }
     }
   }
 
@@ -1815,19 +1917,43 @@
     return DescriptorMapUpdateResult::Fail();
   }
 
+  lldb::addr_t class_buffer_addr = LLDB_INVALID_ADDRESS;
+  const uint32_t class_byte_size = addr_size;
+  const uint32_t class_buffer_len = num_classes;
+  const uint32_t class_buffer_byte_size = class_buffer_len * class_byte_size;
+  if (helper == Helper::objc_getRealizedClassList_trylock) {
+    class_buffer_addr = process->AllocateMemory(
+        class_buffer_byte_size, ePermissionsReadable | ePermissionsWritable,
+        err);
+    if (class_buffer_addr == LLDB_INVALID_ADDRESS) {
+      LLDB_LOGF(log,
+                "unable to allocate %" PRIu32
+                " bytes in process for shared cache read",
+                class_buffer_byte_size);
+      return DescriptorMapUpdateResult::Fail();
+    }
+  }
+
   std::lock_guard<std::mutex> guard(m_mutex);
 
   // Fill in our function argument values
-  arguments.GetValueAtIndex(0)->GetScalar() = hash_table.GetTableLoadAddress();
-  arguments.GetValueAtIndex(1)->GetScalar() = class_infos_addr;
-  arguments.GetValueAtIndex(2)->GetScalar() = class_infos_byte_size;
+  uint32_t index = 0;
+  arguments.GetValueAtIndex(index++)->GetScalar() =
+      hash_table.GetTableLoadAddress();
+  arguments.GetValueAtIndex(index++)->GetScalar() = class_infos_addr;
+  arguments.GetValueAtIndex(index++)->GetScalar() = class_infos_byte_size;
+
+  if (class_buffer_addr != LLDB_INVALID_ADDRESS) {
+    arguments.GetValueAtIndex(index++)->GetScalar() = class_buffer_addr;
+    arguments.GetValueAtIndex(index++)->GetScalar() = class_buffer_byte_size;
+  }
 
   // Only dump the runtime classes from the expression evaluation if the log is
   // verbose:
   Log *type_log = GetLog(LLDBLog::Types);
   bool dump_log = type_log && type_log->GetVerbose();
 
-  arguments.GetValueAtIndex(3)->GetScalar() = dump_log ? 1 : 0;
+  arguments.GetValueAtIndex(index++)->GetScalar() = dump_log ? 1 : 0;
 
   bool success = false;
 
@@ -1888,8 +2014,13 @@
     }
   }
 
+  // Deallocate the memory we allocated for the Class array
+  if (class_buffer_addr != LLDB_INVALID_ADDRESS)
+    process->DeallocateMemory(class_buffer_addr);
+
   // Deallocate the memory we allocated for the ClassInfo array
-  process->DeallocateMemory(class_infos_addr);
+  if (class_infos_addr != LLDB_INVALID_ADDRESS)
+    process->DeallocateMemory(class_infos_addr);
 
   return DescriptorMapUpdateResult(success, num_class_infos);
 }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to