[Lldb-commits] [lldb] fc43703 - [lldb] Use objc_getRealizedClassList_trylock on macOS Ventura and later

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Wed Jun 8 11:34:33 PDT 2022


Author: Jonas Devlieghere
Date: 2022-06-08T11:34:27-07:00
New Revision: fc43703481d858351a72d7ea6f439f4f682ba351

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

LOG: [lldb] Use objc_getRealizedClassList_trylock on macOS Ventura and later

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

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

Added: 
    

Modified: 
    lldb/packages/Python/lldbsuite/test/decorators.py
    lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
    lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
    lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py

Removed: 
    


################################################################################
diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index 467530fdfd7e7..191a1e0ad7ccc 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -284,7 +284,7 @@ def expectedFailureAll(bugnumber=None,
                          archs=archs, triple=triple,
                          debug_info=debug_info,
                          swig_version=swig_version, py_version=py_version,
-                         macos_version=None,
+                         macos_version=macos_version,
                          remote=remote,dwarf_version=dwarf_version,
                          setting=setting)
 

diff  --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index ccc68b55f5cb5..4194004fe61c7 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -6,20 +6,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <cstdint>
-
-#include <memory>
-#include <string>
-#include <vector>
-
-#include "clang/AST/ASTContext.h"
-#include "clang/AST/DeclObjC.h"
-
-#include "lldb/Host/OptionParser.h"
-#include "lldb/Symbol/CompilerType.h"
-#include "lldb/lldb-enumerations.h"
-
+#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/DebuggerEvents.h"
 #include "lldb/Core/Module.h"
@@ -30,11 +19,13 @@
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Expression/UtilityFunction.h"
+#include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandObjectMultiword.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionValueBoolean.h"
+#include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/TypeList.h"
@@ -56,6 +47,7 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
+#include "lldb/lldb-enumerations.h"
 
 #include "AppleObjCClassDescriptorV2.h"
 #include "AppleObjCDeclVendor.h"
@@ -66,9 +58,11 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/ScopeExit.h"
 
-#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
-
+#include <cstdint>
+#include <memory>
+#include <string>
 #include <vector>
 
 using namespace lldb;
@@ -235,6 +229,78 @@ __lldb_apple_objc_v2_get_dynamic_class_info2(void *gdb_objc_realized_classes_ptr
 }
 )";
 
+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 +729,8 @@ AppleObjCRuntimeV2::AppleObjCRuntimeV2(Process *process,
       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 +739,11 @@ AppleObjCRuntimeV2::AppleObjCRuntimeV2(Process *process,
   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 +1610,8 @@ lldb::addr_t AppleObjCRuntimeV2::GetISAHashTablePointer() {
 
 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 +1646,15 @@ AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoUtilityFunctionImpl(
   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 +1680,7 @@ AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoUtilityFunction(
   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 +1688,19 @@ AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoUtilityFunction(
   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 +1712,26 @@ AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoArgs(Helper helper) {
     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 +1911,55 @@ AppleObjCRuntimeV2::DynamicClassInfoExtractor::UpdateISAToDescriptorMap(
     return DescriptorMapUpdateResult::Fail();
   }
 
+  auto deallocate_class_infos = llvm::make_scope_exit([&] {
+    // Deallocate the memory we allocated for the ClassInfo array
+    if (class_infos_addr != LLDB_INVALID_ADDRESS)
+      process->DeallocateMemory(class_infos_addr);
+  });
+
+  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();
+    }
+  }
+
+  auto deallocate_class_buffer = llvm::make_scope_exit([&] {
+    // Deallocate the memory we allocated for the Class array
+    if (class_buffer_addr != LLDB_INVALID_ADDRESS)
+      process->DeallocateMemory(class_buffer_addr);
+  });
+
   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,9 +2020,6 @@ AppleObjCRuntimeV2::DynamicClassInfoExtractor::UpdateISAToDescriptorMap(
     }
   }
 
-  // Deallocate the memory we allocated for the ClassInfo array
-  process->DeallocateMemory(class_infos_addr);
-
   return DescriptorMapUpdateResult(success, num_class_infos);
 }
 

diff  --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
index 2ed2d8784698f..0a2c8ff29275a 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -313,13 +313,15 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime {
   };
 
   /// 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 they include 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 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime {
     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,23 +348,17 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime {
     lldb::addr_t &GetClassInfoArgs(Helper helper);
 
     std::unique_ptr<UtilityFunction>
-    GetClassInfoUtilityFunctionImpl(ExecutionContext &exe_ctx, std::string code,
-                                    std::string name);
-
-    /// Helper to read class info using the gdb_objc_realized_classes.
-    struct gdb_objc_realized_classes_helper {
-      std::unique_ptr<UtilityFunction> utility_function;
-      lldb::addr_t args = LLDB_INVALID_ADDRESS;
-    };
+    GetClassInfoUtilityFunctionImpl(ExecutionContext &exe_ctx, Helper helper,
+                                    std::string code, std::string name);
 
-    /// Helper to read class info using objc_copyRealizedClassList.
-    struct objc_copyRealizedClassList_helper {
+    struct UtilityFunctionHelper {
       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;
+    UtilityFunctionHelper m_gdb_objc_realized_classes_helper;
+    UtilityFunctionHelper m_objc_copyRealizedClassList_helper;
+    UtilityFunctionHelper m_objc_getRealizedClassList_trylock_helper;
   };
 
   /// Abstraction to read the Objective-C class info from the shared cache.
@@ -429,6 +430,7 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime {
   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;

diff  --git a/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
index 3b446d48a6995..87f6d2be0cc84 100644
--- a/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
+++ b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
@@ -10,7 +10,7 @@ class TestCase(TestBase):
     @skipUnlessDarwin
     # LLDB ends up calling the user-defined function (but at least doesn't
     # crash).
-    @expectedFailureDarwin
+    @skipIf(macos_version=["<", "13.0"])
     def test(self):
         """
         Tests LLDB's behaviour if the user defines their own conflicting


        


More information about the lldb-commits mailing list