Add plugin.jit-loader.gdb.enable-jit-breakpoint property to make JIT loader breakpoint optional.

[ovyalov]

Summary

Add plugin.jit-loader.gdb.enable-jit-breakpoint property to make JIT loader breakpoint optional with default value set true.
If we're not interested in debugging JIT code there is no need to hit this breakpoint.

[ovyalov]

source/Core/PluginManager.cpp

2628

Do we still need this function? It's used only by platform plugin and there is no platform plugin properties right now in use - we may want to migrate from old style properties.

[clayborg]

Looks good.

source/Core/PluginManager.cpp

2628

I believe we do for other branches that we merge into... Leave it for now.

[ovyalov]

Files:

/lldb/trunk/include/lldb/Core/PluginManager.h
/lldb/trunk/source/Core/PluginManager.cpp
/lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
/lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.h

Users:

ovyalov (Author)

http://reviews.llvm.org/rL247821

[loladiro]

@clayborg Given that this was merged, does it make sense to now allow the JITLoader to be created on OS X as well, but set the setting to false by default? Does that address the performance concern you had initially?

[clayborg]

So you are saying to have a setting that allows this plug-in to be enabled and have it set to false by default? That sounds good to me. It probably should be false on all platforms and anyone wanting JIT support should enable it?

include/lldb/Core/PluginManager.h

public:
     GetSettingForSymbolFilePlugin (Debugger &debugger,
                                    const ConstString &setting_name);
 
     static bool
     CreateSettingForSymbolFilePlugin (Debugger &debugger,
                                       const lldb::OptionValuePropertiesSP &properties_sp,
                                       const ConstString &description,
                                       bool is_global_property);
+
+    static lldb::OptionValuePropertiesSP
+    GetSettingForJITLoaderPlugin (Debugger &debugger,
+                                   const ConstString &setting_name);
+
+    static bool
+    CreateSettingForJITLoaderPlugin (Debugger &debugger,
+                                     const lldb::OptionValuePropertiesSP &properties_sp,
+                                     const ConstString &description,
+                                     bool is_global_property);
 };
 
 
 } // namespace lldb_private
 
 #endif  // liblldb_PluginManager_h_

source/Core/PluginManager.cpp

GetDebuggerPropertyForPlugins (Debugger &debugger,
     }
     return lldb::OptionValuePropertiesSP();
 }
 
 // This is deprecated way to register plugin specific settings.  e.g.
 // "<plugin_type_name>.plugin.<plugin_type_desc>.SETTINGNAME"
 // and Platform generic settings would be under "platform.SETTINGNAME".
 static lldb::OptionValuePropertiesSP
 GetDebuggerPropertyForPluginsOldStyle (Debugger &debugger,
                                        const ConstString &plugin_type_name,
                                        const ConstString &plugin_type_desc,
                                        bool can_create)
 {
     static ConstString g_property_name("plugin");
     lldb::OptionValuePropertiesSP parent_properties_sp (debugger.GetValueProperties());
     if (parent_properties_sp)
     {
if (parent_properties_sp)
                                                       plugin_type_properties_sp);
             }
             return plugin_type_properties_sp;
         }
     }
     return lldb::OptionValuePropertiesSP();
 }
 
+namespace {
+
+typedef lldb::OptionValuePropertiesSP
+GetDebuggerPropertyForPluginsPtr (Debugger&, const ConstString&, const ConstString&, bool can_create);
 
 lldb::OptionValuePropertiesSP
-PluginManager::GetSettingForDynamicLoaderPlugin (Debugger &debugger, const ConstString &setting_name)
+GetSettingForPlugin (Debugger &debugger,
+                     const ConstString &setting_name,
+                     const ConstString &plugin_type_name,
+                     GetDebuggerPropertyForPluginsPtr get_debugger_property= GetDebuggerPropertyForPlugins)
 {
     lldb::OptionValuePropertiesSP properties_sp;
-    lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPlugins (debugger,
-                                                                                            ConstString("dynamic-loader"),
-                                                                                            ConstString(), // not creating to so we don't need the description
-                                                                                            false));
+    lldb::OptionValuePropertiesSP plugin_type_properties_sp (get_debugger_property (debugger,
+                                                                                    plugin_type_name,
     if (plugin_type_properties_sp)
-        properties_sp = plugin_type_properties_sp->GetSubProperty (NULL, setting_name);
+        properties_sp = plugin_type_properties_sp->GetSubProperty (nullptr, setting_name);
     return properties_sp;
 }
 
 bool
-PluginManager::CreateSettingForDynamicLoaderPlugin (Debugger &debugger,
-                                                    const lldb::OptionValuePropertiesSP &properties_sp,
-                                                    const ConstString &description,
-                                                    bool is_global_property)
+CreateSettingForPlugin (Debugger &debugger,
+                        const ConstString &plugin_type_name,
+                        const ConstString &plugin_type_desc,
+                        bool is_global_property,
+                        GetDebuggerPropertyForPluginsPtr get_debugger_property = GetDebuggerPropertyForPlugins)
 {
     if (properties_sp)
     {
-        lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPlugins (debugger,
-                                                                                                ConstString("dynamic-loader"),
-                                                                                                ConstString("Settings for dynamic loader plug-ins"),
-                                                                                                true));
+        lldb::OptionValuePropertiesSP plugin_type_properties_sp (get_debugger_property (
+            debugger, plugin_type_name, plugin_type_desc, true));
         if (plugin_type_properties_sp)
         {
             plugin_type_properties_sp->AppendProperty (properties_sp->GetName(),
                                                        description,
                                                        is_global_property,
                                                        properties_sp);
             return true;
         }
     }
     return false;
 }
 
+const char* kDynamicLoaderPluginName("dynamic-loader");
+const char* kPlatformPluginName("platform");
+const char* kProcessPluginName("process");
+const char* kSymbolFilePluginName("symbol-file");
+const char* kJITLoaderPluginName("jit-loader");
+
+}
+
+lldb::OptionValuePropertiesSP
+PluginManager::GetSettingForDynamicLoaderPlugin (Debugger &debugger,
+                                                 const ConstString &setting_name)
+{
+    return GetSettingForPlugin(debugger, setting_name, ConstString(kDynamicLoaderPluginName));
+}
+
+bool
+PluginManager::CreateSettingForDynamicLoaderPlugin (Debugger &debugger,
+                                                    const lldb::OptionValuePropertiesSP &properties_sp,
+                                                    const ConstString &description,
+                                                    bool is_global_property)
+{
+    return CreateSettingForPlugin(debugger,
+                                  ConstString(kDynamicLoaderPluginName),
+                                  ConstString("Settings for dynamic loader plug-ins"),
+                                  properties_sp,
+                                  description,
+                                  is_global_property);
+}
+
 
 lldb::OptionValuePropertiesSP
 PluginManager::GetSettingForPlatformPlugin (Debugger &debugger, const ConstString &setting_name)
 {
-    lldb::OptionValuePropertiesSP properties_sp;
-    lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPluginsOldStyle (debugger,
-                                                                                                    ConstString("platform"),
-                                                                                                    ConstString(), // not creating to so we don't need the description
-                                                                                                    false));
-    if (plugin_type_properties_sp)
-        properties_sp = plugin_type_properties_sp->GetSubProperty (NULL, setting_name);
-    return properties_sp;
+    return GetSettingForPlugin(debugger,
+                               setting_name,
+                               ConstString(kPlatformPluginName),
+                               GetDebuggerPropertyForPluginsOldStyle);
 }
 
 bool
 PluginManager::CreateSettingForPlatformPlugin (Debugger &debugger,
-                                                    const lldb::OptionValuePropertiesSP &properties_sp,
-                                                    const ConstString &description,
-                                                    bool is_global_property)
 {
-    if (properties_sp)
-    {
-        lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPluginsOldStyle (debugger,
-                                                                                                        ConstString("platform"),
-                                                                                                        ConstString("Settings for platform plug-ins"),
-                                                                                                        true));
-        if (plugin_type_properties_sp)
-        {
-            plugin_type_properties_sp->AppendProperty (properties_sp->GetName(),
-                                                       description,
-                                                       is_global_property,
-                                                       properties_sp);
-            return true;
-        }
-    }
-    return false;
+    return CreateSettingForPlugin(debugger,
+                                  ConstString(kPlatformPluginName),
+                                  properties_sp,
+                                  GetDebuggerPropertyForPluginsOldStyle);
 }
 
 
 lldb::OptionValuePropertiesSP
 PluginManager::GetSettingForProcessPlugin (Debugger &debugger, const ConstString &setting_name)
 {
-    lldb::OptionValuePropertiesSP properties_sp;
-    lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPlugins (debugger,
-                                                                                            ConstString("process"),
-                                                                                            ConstString(), // not creating to so we don't need the description
-                                                                                            false));
-    if (plugin_type_properties_sp)
-        properties_sp = plugin_type_properties_sp->GetSubProperty (NULL, setting_name);
-    return properties_sp;
+    return GetSettingForPlugin(debugger, setting_name, ConstString(kProcessPluginName));
 }
 
 bool
 PluginManager::CreateSettingForProcessPlugin (Debugger &debugger,
                                               const lldb::OptionValuePropertiesSP &properties_sp,
                                               const ConstString &description,
                                               bool is_global_property)
 {
-    if (properties_sp)
-    {
-        lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPlugins (debugger,
-                                                                                                ConstString("process"),
-                                                                                                ConstString("Settings for process plug-ins"),
-                                                                                                true));
-        if (plugin_type_properties_sp)
-        {
-            plugin_type_properties_sp->AppendProperty (properties_sp->GetName(),
-                                                       description,
-                                                       is_global_property,
-                                                       properties_sp);
-            return true;
-        }
-    }
-    return false;
+    return CreateSettingForPlugin(debugger,
+                                  ConstString(kProcessPluginName),
+                                  properties_sp,
+                                  is_global_property);
 }
 
-
-static const char* kSymbolFilePluginName("symbol-file");
-
 lldb::OptionValuePropertiesSP
 PluginManager::GetSettingForSymbolFilePlugin (Debugger &debugger,
                                               const ConstString &setting_name)
 {
-    lldb::OptionValuePropertiesSP properties_sp;
-    lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPlugins (debugger,
-                                                                                            ConstString(kSymbolFilePluginName),
-                                                                                            ConstString(), // not creating to so we don't need the description
-                                                                                            false));
-    if (plugin_type_properties_sp)
-        properties_sp = plugin_type_properties_sp->GetSubProperty (nullptr, setting_name);
-    return properties_sp;
+    return GetSettingForPlugin(debugger, setting_name, ConstString(kSymbolFilePluginName));
 }
 
 bool
 PluginManager::CreateSettingForSymbolFilePlugin (Debugger &debugger,
                                                  const lldb::OptionValuePropertiesSP &properties_sp,
                                                  const ConstString &description,
                                                  bool is_global_property)
 {
-    if (properties_sp)
-    {
-        lldb::OptionValuePropertiesSP plugin_type_properties_sp (GetDebuggerPropertyForPlugins (debugger,
-                                                                                                ConstString(kSymbolFilePluginName),
-                                                                                                ConstString("Settings for symbol file plug-ins"),
-                                                                                                true));
-        if (plugin_type_properties_sp)
-        {
-            plugin_type_properties_sp->AppendProperty (properties_sp->GetName(),
-                                                       description,
-                                                       is_global_property,
-                                                       properties_sp);
-            return true;
-        }
-    }
-    return false;
+    return CreateSettingForPlugin(debugger,
+                                  properties_sp,
+                                  is_global_property);
+
+lldb::OptionValuePropertiesSP
+PluginManager::GetSettingForJITLoaderPlugin (Debugger &debugger,
+                                             const ConstString &setting_name)
+{
+    return GetSettingForPlugin(debugger, setting_name, ConstString(kJITLoaderPluginName));
+
+bool
+PluginManager::CreateSettingForJITLoaderPlugin (Debugger &debugger,
+                                                const lldb::OptionValuePropertiesSP &properties_sp,
+                                                const ConstString &description,
+                                                bool is_global_property)
+{
+    return CreateSettingForPlugin(debugger,
+                                  ConstString(kJITLoaderPluginName),
+                                  ConstString("Settings for JIT loader plug-ins"),
+                                  properties_sp,
+                                  description,
+                                  is_global_property);
 }

source/Plugins/JITLoader/GDB/JITLoaderGDB.h

public:
     static const char *
     GetPluginDescriptionStatic();
 
     static lldb::JITLoaderSP
     CreateInstance (lldb_private::Process *process, bool force);
 
     JITLoaderGDB (lldb_private::Process *process);
 
+    static void
+    DebuggerInitialize(lldb_private::Debugger &debugger);
+
     virtual
     ~JITLoaderGDB ();
 
     //------------------------------------------------------------------
     // PluginInterface protocol
     //------------------------------------------------------------------
     virtual lldb_private::ConstString
     GetPluginName();

source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp

 
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Log.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamString.h"
+#include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 
 #include "JITLoaderGDB.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
+namespace {
+
+    PropertyDefinition
+    g_properties[] =
+    {
+        { "enable-jit-breakpoint", OptionValue::eTypeBoolean, true,  true , nullptr, nullptr, "Enable breakpoint on __jit_debug_register_code." },
+        {  nullptr               , OptionValue::eTypeInvalid, false, 0,     nullptr, nullptr, nullptr }
+    };
+
+    enum
+    {
+        ePropertyEnableJITBreakpoint
+    };
+
+
+    class PluginProperties : public Properties
+    {
+    public:
+        static ConstString
+        GetSettingName()
+        {
+            return JITLoaderGDB::GetPluginNameStatic();
+        }
+
+        PluginProperties()
+        {
+            m_collection_sp.reset (new OptionValueProperties(GetSettingName()));
+            m_collection_sp->Initialize(g_properties);
+        }
+
+        bool
+        GetEnableJITBreakpoint() const
+        {
+            return m_collection_sp->GetPropertyAtIndexAsBoolean(
+                nullptr,
+                ePropertyEnableJITBreakpoint,
+                g_properties[ePropertyEnableJITBreakpoint].default_uint_value != 0);
+        }
+
+    };
+
+    typedef std::shared_ptr<PluginProperties> JITLoaderGDBPropertiesSP;
+
+    static const JITLoaderGDBPropertiesSP&
+    GetGlobalPluginProperties()
+    {
+        static const auto g_settings_sp(std::make_shared<PluginProperties>());
+        return g_settings_sp;
+    }
+
+}  // anonymous namespace end
+
 //------------------------------------------------------------------
 // Debug Interface Structures
 //------------------------------------------------------------------
 typedef enum
 {
     JIT_NOACTION = 0,
     JIT_REGISTER_FN,
     JIT_UNREGISTER_FN

 }
 
 JITLoaderGDB::~JITLoaderGDB ()
 {
     if (LLDB_BREAK_ID_IS_VALID(m_jit_break_id))
         m_process->GetTarget().RemoveBreakpointByID (m_jit_break_id);
 }
 
+void
+JITLoaderGDB::DebuggerInitialize(Debugger &debugger)
+{
+    if (!PluginManager::GetSettingForJITLoaderPlugin(debugger, PluginProperties::GetSettingName()))
+    {
+        const bool is_global_setting = true;
+        PluginManager::CreateSettingForJITLoaderPlugin(debugger,
+                                                       GetGlobalPluginProperties()->GetValueProperties(),
+                                                       ConstString ("Properties for the JIT LoaderGDB plug-in."),
+                                                       is_global_setting);
+    }
+}
+
 void JITLoaderGDB::DidAttach()
 {
     Target &target = m_process->GetTarget();
     ModuleList &module_list = target.GetImages();
     SetJITBreakpoint(module_list);
 }
 
 void JITLoaderGDB::DidLaunch()
 {
     Target &target = m_process->GetTarget();
     ModuleList &module_list = target.GetImages();
     SetJITBreakpoint(module_list);
 }
 
 void
 JITLoaderGDB::ModulesDidLoad(ModuleList &module_list)
 {
     if (!DidSetJITBreakpoint() && m_process->IsAlive())
-  SetJITBreakpoint(module_list);
+        SetJITBreakpoint(module_list);
 }
 
 //------------------------------------------------------------------
 // Setup the JIT Breakpoint
 //------------------------------------------------------------------
 void
 JITLoaderGDB::SetJITBreakpoint(lldb_private::ModuleList &module_list)
 {
-    Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_JIT_LOADER));
+    if (!GetGlobalPluginProperties()->GetEnableJITBreakpoint())
+        return;
 
     if ( DidSetJITBreakpoint() )
         return;
 
+    Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_JIT_LOADER));
     if (log)
         log->Printf("JITLoaderGDB::%s looking for JIT register hook",
                     __FUNCTION__);
 
     addr_t jit_addr = GetSymbolAddress(module_list,
                                        ConstString("__jit_debug_register_code"),
                                        eSymbolTypeAny);
     if (jit_addr == LLDB_INVALID_ADDRESS)
JITLoaderGDB::GetPluginVersion()
     return 1;
 }
 
 void
 JITLoaderGDB::Initialize()
 {
     PluginManager::RegisterPlugin (GetPluginNameStatic(),
                                    GetPluginDescriptionStatic(),
-                                   CreateInstance);
+                                   CreateInstance,
+                                   DebuggerInitialize);
 }
 
 void
 JITLoaderGDB::Terminate()
 {
     PluginManager::UnregisterPlugin (CreateInstance);
 }