[Target] Decouple ObjCLanguageRuntime from LanguageRuntime

[xiaobai]

Summary

ObjCLanguageRuntime was being pulled into LanguageRuntime because of
Breakpoint Preconditions. If we move BreakpointPrecondition out of Breakpoint,
we can extend the LanguageRuntime plugin interface so that LanguageRuntimes
can give us a BreakpointPrecondition for exceptions.

[JDevlieghere]

Have you considered making just AddExceptionPrecondition virtual? Wouldn't that solve the problem too, without the code duplication of making CreateExceptionBreakpoint virtual? Also, I think it's totally reasonable to hoist BreakpointPrecondition out of Breakpoint. After having had to deal with the nuisance of not being able to forward declare nested classes, I try to avoid them in general.

[xiaobai]

In D63181#1539291, @JDevlieghere wrote:

Have you considered making just AddExceptionPrecondition virtual? Wouldn't that solve the problem too, without the code duplication of making CreateExceptionBreakpoint virtual?

I don't believe so. As I understand it, you might not have a process yet when you call LanguageRuntime::CreateExceptionBreakpoint from Target, so you won't have an instance of LanguageRuntime. You could create an instance of a LanguageRuntime object with a nullptr process just to invoke it as an instance method, but that seems like the wrong choice to me. All this combined has led me to believe that AddExceptionPrecondition needs to get a callback to a static function from PluginManager.

Also, I think it's totally reasonable to hoist BreakpointPrecondition out of Breakpoint. After having had to deal with the nuisance of not being able to forward declare nested classes, I try to avoid them in general.

Yeah, I was kinda iffy on this, but if you (and possibly others) think this is reasonable, I might go with this option. The only thing that would change is the callback would return a BreakpointPreconditionSP instead of void, and AddExceptionPrecondition would be the one to modify the breakpoint. I don't think the core idea would change much but the implementation might be slightly nicer. Let me know if you prefer that.

[labath]

Moving BreakpointPrecondition to a forward-declarable place sounds good to me.

source/Target/ObjCLanguageRuntime.cpp

379

Shouldn't you be checking the language here or something. Given that you're iterating over all plugins, won't this set the precondition unconditionally (:P) on all exception breakpoints?

384

new doesn't fail

[xiaobai]

In D63181#1539620, @labath wrote:

Moving BreakpointPrecondition to a forward-declarable place sounds good to me.

Alright, so I think I'll go ahead and change this patch to make that happen. Thanks for your input! :)

source/Target/ObjCLanguageRuntime.cpp

379

It will! For some reason I added a language paramter but forgot to use it. Thanks for pointing that out.

[xiaobai]

Implement suggestion
Address feedback

[jingham]

In D63181#1539292, @xiaobai wrote:

In D63181#1539291, @JDevlieghere wrote:

Have you considered making just AddExceptionPrecondition virtual? Wouldn't that solve the problem too, without the code duplication of making CreateExceptionBreakpoint virtual?

I don't believe so. As I understand it, you might not have a process yet when you call LanguageRuntime::CreateExceptionBreakpoint from Target, so you won't have an instance of LanguageRuntime. You could create an instance of a LanguageRuntime object with a nullptr process just to invoke it as an instance method, but that seems like the wrong choice to me. All this combined has led me to believe that AddExceptionPrecondition needs to get a callback to a static function from PluginManager.

That's right. The reason that this is necessary is that there were MacOS versions for a while where both the ObjC V1 and ObjC V2 runtimes were available on the same system. And since you could have a C main executable that loaded ObjC code after it was launched, you couldn't a priori tell what ObjC runtime was going to be in force till you saw the runtime actually get loaded. Since you could set an ObjC throw breakpoint, then run the process, we have to delay everything about choosing the actual exception breakpoint and its associated behaviors till after the process was launched.

That's also why setting exception breakpoints is a little contorted as well.

In the fullness of time we should get an lldb that could debug more than one version of swift in a single lldb, putting us back in this situation again. So I think we should maintain this bit of complexity.

Also, I think it's totally reasonable to hoist BreakpointPrecondition out of Breakpoint. After having had to deal with the nuisance of not being able to forward declare nested classes, I try to avoid them in general.

Yeah, I was kinda iffy on this, but if you (and possibly others) think this is reasonable, I might go with this option. The only thing that would change is the callback would return a BreakpointPreconditionSP instead of void, and AddExceptionPrecondition would be the one to modify the breakpoint. I don't think the core idea would change much but the implementation might be slightly nicer. Let me know if you prefer that.

[jingham]

This looks fine to me.

include/lldb/Core/PluginManager.h

public:
   static bool UnregisterPlugin(LanguageCreateInstance create_callback);
 
   static LanguageCreateInstance GetLanguageCreateCallbackAtIndex(uint32_t idx);
 
   static LanguageCreateInstance
   GetLanguageCreateCallbackForPluginName(ConstString name);
 
   // LanguageRuntime
-  static bool
-  RegisterPlugin(ConstString name, const char *description,
-                 LanguageRuntimeCreateInstance create_callback,
-                 LanguageRuntimeGetCommandObject command_callback = nullptr);
+  static bool RegisterPlugin(
+      ConstString name, const char *description,
+      LanguageRuntimeGetCommandObject command_callback = nullptr,
+      LanguageRuntimeAddBreakpointPrecondition precondition_callback = nullptr);
 
   static bool UnregisterPlugin(LanguageRuntimeCreateInstance create_callback);
 
   static LanguageRuntimeCreateInstance
   GetLanguageRuntimeCreateCallbackAtIndex(uint32_t idx);
 
   static LanguageRuntimeGetCommandObject
   GetLanguageRuntimeGetCommandObjectAtIndex(uint32_t idx);
 
+  static LanguageRuntimeAddBreakpointPrecondition
+  GetLanguageRuntimeAddBreakpointPreconditionAtIndex(uint32_t idx);
+
   static LanguageRuntimeCreateInstance
   GetLanguageRuntimeCreateCallbackForPluginName(ConstString name);
 
   // SystemRuntime
   static bool RegisterPlugin(ConstString name, const char *description,
                              SystemRuntimeCreateInstance create_callback);
 
   static bool UnregisterPlugin(SystemRuntimeCreateInstance create_callback);

include/lldb/Target/LanguageRuntime.h

virtual bool ExceptionBreakpointsExplainStop(lldb::StopInfoSP stop_reason) {
     return false;
   }
 
   static lldb::BreakpointSP
   CreateExceptionBreakpoint(Target &target, lldb::LanguageType language,
                             bool catch_bp, bool throw_bp,
                             bool is_internal = false);
 
-  static Breakpoint::BreakpointPreconditionSP
-  CreateExceptionPrecondition(lldb::LanguageType language, bool catch_bp,
+  static void AddExceptionPrecondition(lldb::BreakpointSP breakpoint,
+                                       lldb::LanguageType language,
                                        bool throw_bp);
 
   virtual lldb::ValueObjectSP GetExceptionObjectForThread(
       lldb::ThreadSP thread_sp) {
     return lldb::ValueObjectSP();
   }
 
   virtual lldb::ThreadSP GetBacktraceThreadFromException(
       lldb::ValueObjectSP thread_sp) {

include/lldb/Target/ObjCLanguageRuntime.h

public:
 
   protected:
     void AddClassName(const char *class_name);
 
   private:
     std::unordered_set<std::string> m_class_names;
   };
 
+  static void SetBreakpointExceptionPrecondition(lldb::BreakpointSP breakpoint,
+                                                 lldb::LanguageType language,
+                                                 bool throw_bp);
+
   class TaggedPointerVendor {
   public:
     virtual ~TaggedPointerVendor() = default;
 
     virtual bool IsPossibleTaggedPointer(lldb::addr_t ptr) = 0;
 
     virtual ObjCLanguageRuntime::ClassDescriptorSP
     GetClassDescriptor(lldb::addr_t ptr) = 0;

include/lldb/lldb-private-interfaces.h

typedef EmulateInstruction *(*EmulateInstructionCreateInstance)(
     const ArchSpec &arch, InstructionType inst_type);
 typedef OperatingSystem *(*OperatingSystemCreateInstance)(Process *process,
                                                           bool force);
 typedef Language *(*LanguageCreateInstance)(lldb::LanguageType language);
 typedef LanguageRuntime *(*LanguageRuntimeCreateInstance)(
     Process *process, lldb::LanguageType language);
 typedef lldb::CommandObjectSP (*LanguageRuntimeGetCommandObject)(
     CommandInterpreter &interpreter);
+typedef void (*LanguageRuntimeAddBreakpointPrecondition)(
+    lldb::BreakpointSP breakpoint, lldb::LanguageType language, bool throw_bp);
 typedef lldb::StructuredDataPluginSP (*StructuredDataPluginCreateInstance)(
     Process &process);
 typedef Status (*StructuredDataFilterLaunchInfo)(ProcessLaunchInfo &launch_info,
                                                  Target *target);
 typedef SystemRuntime *(*SystemRuntimeCreateInstance)(Process *process);
 typedef lldb::PlatformSP (*PlatformCreateInstance)(bool force,
                                                    const ArchSpec *arch);
 typedef lldb::ProcessSP (*ProcessCreateInstance)(

source/Core/PluginManager.cpp

 
 struct LanguageRuntimeInstance {
   LanguageRuntimeInstance() : name(), description(), create_callback(nullptr) {}
 
   ConstString name;
   std::string description;
   LanguageRuntimeCreateInstance create_callback;
   LanguageRuntimeGetCommandObject command_callback;
+  LanguageRuntimeAddBreakpointPrecondition precondition_callback;
 };
 
 typedef std::vector<LanguageRuntimeInstance> LanguageRuntimeInstances;
 
 static std::recursive_mutex &GetLanguageRuntimeMutex() {
   static std::recursive_mutex g_instances_mutex;
   return g_instances_mutex;
 }
 
 static LanguageRuntimeInstances &GetLanguageRuntimeInstances() {
   static LanguageRuntimeInstances g_instances;
   return g_instances;
 }
 
 bool PluginManager::RegisterPlugin(
     ConstString name, const char *description,
     LanguageRuntimeCreateInstance create_callback,
-    LanguageRuntimeGetCommandObject command_callback) {
+    LanguageRuntimeGetCommandObject command_callback,
+    LanguageRuntimeAddBreakpointPrecondition precondition_callback) {
   if (create_callback) {
     LanguageRuntimeInstance instance;
     assert((bool)name);
     instance.name = name;
     if (description && description[0])
       instance.description = description;
     instance.create_callback = create_callback;
     instance.command_callback = command_callback;
+    instance.precondition_callback = precondition_callback;
     std::lock_guard<std::recursive_mutex> guard(GetLanguageRuntimeMutex());
     GetLanguageRuntimeInstances().push_back(instance);
   }
   return false;
 }
 
 bool PluginManager::UnregisterPlugin(
     LanguageRuntimeCreateInstance create_callback) {

 PluginManager::GetLanguageRuntimeGetCommandObjectAtIndex(uint32_t idx) {
   std::lock_guard<std::recursive_mutex> guard(GetLanguageRuntimeMutex());
   LanguageRuntimeInstances &instances = GetLanguageRuntimeInstances();
   if (idx < instances.size())
     return instances[idx].command_callback;
   return nullptr;
 }
 
+LanguageRuntimeAddBreakpointPrecondition
+PluginManager::GetLanguageRuntimeAddBreakpointPreconditionAtIndex(uint32_t idx) {
+  std::lock_guard<std::recursive_mutex> guard(GetLanguageRuntimeMutex());
+  LanguageRuntimeInstances &instances = GetLanguageRuntimeInstances();
+  if (idx < instances.size())
+    return instances[idx].precondition_callback;
+  return nullptr;
+}
+
 LanguageRuntimeCreateInstance
 PluginManager::GetLanguageRuntimeCreateCallbackForPluginName(
     ConstString name) {
   if (name) {
     std::lock_guard<std::recursive_mutex> guard(GetLanguageRuntimeMutex());
     LanguageRuntimeInstances &instances = GetLanguageRuntimeInstances();
 
     LanguageRuntimeInstances::iterator pos, end = instances.end();

source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp

else
       return nullptr;
   } else
     return nullptr;
 }
 
 void AppleObjCRuntimeV1::Initialize() {
   PluginManager::RegisterPlugin(
       GetPluginNameStatic(), "Apple Objective-C Language Runtime - Version 1",
-      CreateInstance);
+      CreateInstance,
+      /*command_callback = */ nullptr,
+      ObjCLanguageRuntime::SetBreakpointExceptionPrecondition);
 }
 
 void AppleObjCRuntimeV1::Terminate() {
   PluginManager::UnregisterPlugin(CreateInstance);
 }
 
 lldb_private::ConstString AppleObjCRuntimeV1::GetPluginNameStatic() {
   static ConstString g_name("apple-objc-v1");

source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

 };
 
 void AppleObjCRuntimeV2::Initialize() {
   PluginManager::RegisterPlugin(
       GetPluginNameStatic(), "Apple Objective-C Language Runtime - Version 2",
       CreateInstance,
       [](CommandInterpreter &interpreter) -> lldb::CommandObjectSP {
         return CommandObjectSP(new CommandObjectMultiwordObjC(interpreter));
-      });
+      },
+      ObjCLanguageRuntime::SetBreakpointExceptionPrecondition);
 }
 
 void AppleObjCRuntimeV2::Terminate() {
   PluginManager::UnregisterPlugin(CreateInstance);
 }
 
 lldb_private::ConstString AppleObjCRuntimeV2::GetPluginNameStatic() {
   static ConstString g_name("apple-objc-v2");

source/Target/LanguageRuntime.cpp

 //===-- LanguageRuntime.cpp -------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Target.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 char LanguageRuntime::ID = 0;
 
 ExceptionSearchFilter::ExceptionSearchFilter(const lldb::TargetSP &target_sp,
LanguageRuntime *LanguageRuntime::FindPlugin(Process *process,
 
   return nullptr;
 }
 
 LanguageRuntime::LanguageRuntime(Process *process) : m_process(process) {}
 
 LanguageRuntime::~LanguageRuntime() = default;
 
-Breakpoint::BreakpointPreconditionSP
-LanguageRuntime::CreateExceptionPrecondition(lldb::LanguageType language,
-                                             bool catch_bp, bool throw_bp) {
-  switch (language) {
-  case eLanguageTypeObjC:
-    if (throw_bp)
-      return Breakpoint::BreakpointPreconditionSP(
-          new ObjCLanguageRuntime::ObjCExceptionPrecondition());
-    break;
-  default:
+void LanguageRuntime::AddExceptionPrecondition(BreakpointSP breakpoint,
+                                               LanguageType language,
+                                               bool throw_bp) {
+  LanguageRuntimeCreateInstance create_callback;
+  for (uint32_t idx = 0;
+       (create_callback =
+            PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx)) !=
+       nullptr;
+       idx++) {
+    if (auto precondition_callback =
+            PluginManager::GetLanguageRuntimeAddBreakpointPreconditionAtIndex(
+                idx)) {
+      precondition_callback(breakpoint, language, throw_bp);
+      if (breakpoint->GetPrecondition())
         break;
     }
-  return Breakpoint::BreakpointPreconditionSP();
+  }
 }
 
 BreakpointSP LanguageRuntime::CreateExceptionBreakpoint(
     Target &target, lldb::LanguageType language, bool catch_bp, bool throw_bp,
     bool is_internal) {
   BreakpointResolverSP resolver_sp(
       new ExceptionBreakpointResolver(language, catch_bp, throw_bp));
   SearchFilterSP filter_sp(
       new ExceptionSearchFilter(target.shared_from_this(), language));
   bool hardware = false;
   bool resolve_indirect_functions = false;
   BreakpointSP exc_breakpt_sp(
       target.CreateBreakpoint(filter_sp, resolver_sp, is_internal, hardware,
                               resolve_indirect_functions));
   if (exc_breakpt_sp) {
-    Breakpoint::BreakpointPreconditionSP precondition_sp =
-        CreateExceptionPrecondition(language, catch_bp, throw_bp);
-    if (precondition_sp)
-      exc_breakpt_sp->SetPrecondition(precondition_sp);
+    AddExceptionPrecondition(exc_breakpt_sp, language, throw_bp);
 
     if (is_internal)
       exc_breakpt_sp->SetBreakpointKind("exception");
   }
 
   return exc_breakpt_sp;
 }
 

source/Target/ObjCLanguageRuntime.cpp

bool ObjCLanguageRuntime::GetTypeBitSize(const CompilerType &compiler_type,
 
   size = 8 * (max_offset + sizeof_max);
   if (found)
     m_type_size_cache.Insert(opaque_ptr, size);
 
   return found;
 }
 
+void ObjCLanguageRuntime::SetBreakpointExceptionPrecondition(
+    BreakpointSP breakpoint, LanguageType language, bool throw_bp) {
+  if (!throw_bp)
+    return;
+  Breakpoint::BreakpointPreconditionSP precondition_sp(
+      new ObjCLanguageRuntime::ObjCExceptionPrecondition());
+  if (!precondition_sp)
+    return;
+  breakpoint->SetPrecondition(precondition_sp);
+}
+
 // Exception breakpoint Precondition class for ObjC:
 void ObjCLanguageRuntime::ObjCExceptionPrecondition::AddClassName(
     const char *class_name) {
   m_class_names.insert(class_name);
 }
 
 ObjCLanguageRuntime::ObjCExceptionPrecondition::ObjCExceptionPrecondition() {}