Adds property to force enabling of GDB JIT loader for MacOS

[yurydelendik]

Summary

Based on https://gist.github.com/thlorenz/30bf0a3f67b1d97b2945#patching-and-rebuilding

The functionality was disabled at https://github.com/llvm/llvm-project/commit/521c2278abb16f0148cef1bd061cadb01ef43192

[yurydelendik]

Fix GetEnableLoaderForDarwin name

[jingham]

It would be better to have the setting be an enum of "on/off/default", and then have the somebody - the current DynamicLoader plugin seems the best somebody - provide the default value if the setting hasn't been explicitly set. That way on any platform one could turn the loading on and off, which seems useful, and we wouldn't have to have a Darwin specific setting that will cease being applicable when the Darwin default switches.

[yurydelendik]

• Change to on/off/default property.

[jingham]

That looks good. Could you add a test for this setting to the ./functionalities/jitloader_gdb/TestJITLoaderGDB.py test? I wouldn't test that the default has any particular behavior because that might change over time. But test that if you turn it on, you do get load notifications, and if you turn it off you don't. Thanks!

[labath]

Do we need both this and the enable-jit-breakpoint setting? My impression was that the latter was meant to be used for disabling the gdb jit feature. Is the gdb plugin useful for anything if it does not set the breakpoint (i.e. enable = on, but enable-jit-bkpt = off). If it isn't, then could we just remove the latter?

[yurydelendik]

• Add tests

[yurydelendik]

• Remove enable-jit-breakpoint option

[yurydelendik]

In D57689#1384148, @jingham wrote:

Could you add a test for this setting to the ./functionalities/jitloader_gdb/TestJITLoaderGDB.py test?

Added tests

In D57689#1384894, @labath wrote:

Do we need both this and the enable-jit-breakpoint setting? My impression was that the latter was meant to be used for disabling the gdb jit feature. Is the gdb plugin useful for anything if it does not set the breakpoint (i.e. enable = on, but enable-jit-bkpt = off). If it isn't, then could we just remove the latter?

Every time I tried to investigate that, I came to conclusion that it was just added as alternative for enable, but with intent to test plugin instantiation. Remoing in this patch. I can revert this change as needed.

[labath]

In D57689#1389212, @yurydelendik wrote:

In D57689#1384894, @labath wrote:

Do we need both this and the enable-jit-breakpoint setting? My impression was that the latter was meant to be used for disabling the gdb jit feature. Is the gdb plugin useful for anything if it does not set the breakpoint (i.e. enable = on, but enable-jit-bkpt = off). If it isn't, then could we just remove the latter?

Every time I tried to investigate that, I came to conclusion that it was just added as alternative for enable, but with intent to test plugin instantiation. Remoing in this patch. I can revert this change as needed.

Thank you for looking into that. I think removing that setting is fine.

[yurydelendik]

@jingham is this patch good to land?

[jingham]

Yes, looks fine.

[yurydelendik]

rebase

[stella.stamenova]

This is causing a failure on the Windows Bot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/2299/steps/test/logs/stdio

It was broken because of another change which is why you probably didn't get a notification. Please, fix this quickly or revert the change, so that we don't end up with more failures that are not sending notifications because the bot's been broken all morning.

lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp

template <typename ptr_t> struct jit_descriptor {
   ptr_t relevant_entry; // pointer
   ptr_t first_entry;    // pointer
 };
 
 namespace {
 
 static constexpr PropertyDefinition g_properties[] = {
     {"enable-jit-breakpoint", OptionValue::eTypeBoolean, true, true, nullptr,
-     {}, "Enable breakpoint on __jit_debug_register_code."}};
+     {}, "Enable breakpoint on __jit_debug_register_code."},
+    {"enable-loader-for-darwin", OptionValue::eTypeBoolean, true, false, nullptr,
+     {}, "Enable process loader monitor for macOS - disabled there for "
+     "performance reasons."}};
 
-enum { ePropertyEnableJITBreakpoint };
+enum { ePropertyEnableJITBreakpoint, ePropertyEnableLoaderForDarwin };
 
 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);
   }
+
+  bool GetEnableLoaderForDwarwin() const {
+    return m_collection_sp->GetPropertyAtIndexAsBoolean(
+        nullptr, ePropertyEnableLoaderForDarwin,
+        g_properties[ePropertyEnableLoaderForDarwin].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;
 }

 lldb_private::ConstString JITLoaderGDB::GetPluginNameStatic() {
   static ConstString g_name("gdb");
   return g_name;
 }
 
 JITLoaderSP JITLoaderGDB::CreateInstance(Process *process, bool force) {
   JITLoaderSP jit_loader_sp;
   ArchSpec arch(process->GetTarget().GetArchitecture());
-  if (arch.GetTriple().getVendor() != llvm::Triple::Apple)
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple ||
+      GetGlobalPluginProperties()->GetEnableLoaderForDwarwin())
     jit_loader_sp.reset(new JITLoaderGDB(process));
   return jit_loader_sp;
 }
 
 const char *JITLoaderGDB::GetPluginDescriptionStatic() {
   return "JIT loader plug-in that watches for JIT events using the GDB "
          "interface.";
 }