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/trunk/packages/Python/lldbsuite/test/functionalities/jitloader_gdb/Makefile

 LEVEL = ../../make
 
 C_SOURCES := main.c
 
+all: a.out simple
+
 include $(LEVEL)/Makefile.rules
+
+simple:
+  $(MAKE) VPATH=$(VPATH) -f $(SRCDIR)/simple.mk
+
+clean::
+  $(MAKE) -f $(SRCDIR)/simple.mk clean
 No newline at end of file

lldb/trunk/packages/Python/lldbsuite/test/functionalities/jitloader_gdb/TestJITLoaderGDB.py

 """Test for the JITLoaderGDB interface"""
 
 from __future__ import print_function
 
 import unittest2
 import os
 import lldb
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import re
 
+file_index = 0
 
 class JITLoaderGDBTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipTestIfFn(
         lambda: "Skipped because the test crashes the test runner",
         bugnumber="llvm.org/pr24702")
def test_bogus_values(self):
             None, None, self.get_process_working_directory())
         self.assertTrue(process, PROCESS_IS_VALID)
 
         # The inferior will now pass bogus values over the interface. Make sure
         # we don't crash.
 
         self.assertEqual(process.GetState(), lldb.eStateExited)
         self.assertEqual(process.GetExitStatus(), 0)
+
+    def gen_log_file(self):
+        global file_index
+        ++file_index
+        logfile = os.path.join(
+            self.getBuildDir(),
+            "jitintgdb-" + self.getArchitecture() + "-" +
+                str(file_index) + ".txt")
+
+        def cleanup():
+            if os.path.exists(logfile):
+                os.unlink(logfile)
+        self.addTearDownHook(cleanup)
+        return logfile
+
+    def test_jit_int_default(self):
+        self.expect("settings show plugin.jit-loader.gdb.enable",
+                    substrs=["plugin.jit-loader.gdb.enable (enum) = default"])
+
+    def test_jit_int_on(self):
+        """Tests interface with 'enable' settings 'on'"""
+        self.build()
+        exe = self.getBuildArtifact("simple")
+
+        logfile = self.gen_log_file()
+        self.runCmd("log enable -f %s lldb jit" % (logfile))
+        self.runCmd("settings set plugin.jit-loader.gdb.enable on")
+        def cleanup():
+            self.runCmd("log disable lldb")
+            self.runCmd("settings set plugin.jit-loader.gdb.enable default")
+        self.addTearDownHook(cleanup)
+
+        # launch the process
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+        process = target.LaunchSimple(
+            None, None, self.get_process_working_directory())
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        self.assertEqual(process.GetState(), lldb.eStateExited)
+        self.assertEqual(process.GetExitStatus(), 0)
+
+        logcontent = ""
+        if os.path.exists(logfile):
+            logcontent = open(logfile).read()
+        self.assertIn(
+            "SetJITBreakpoint setting JIT breakpoint", logcontent)
+
+    def test_jit_int_off(self):
+        """Tests interface with 'enable' settings 'off'"""
+        self.build()
+        exe = self.getBuildArtifact("simple")
+
+        logfile = self.gen_log_file()
+        self.runCmd("log enable -f %s lldb jit" % (logfile))
+        self.runCmd("settings set plugin.jit-loader.gdb.enable off")
+        def cleanup():
+            self.runCmd("log disable lldb")
+            self.runCmd("settings set plugin.jit-loader.gdb.enable default")
+        self.addTearDownHook(cleanup)
+
+        # launch the process
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+        process = target.LaunchSimple(
+            None, None, self.get_process_working_directory())
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        self.assertEqual(process.GetState(), lldb.eStateExited)
+        self.assertEqual(process.GetExitStatus(), 0)
+
+        if os.path.exists(logfile):
+            logcontent = open(logfile).read()
+            self.assertNotIn(
+              "SetJITBreakpoint setting JIT breakpoint", logcontent)
+        else:
+            self.assertTrue(false)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/jitloader_gdb/simple.c

+#include <inttypes.h>
+
+// GDB JIT interface stub
+struct
+{
+    uint32_t version;
+    uint32_t action_flag;
+    void* relevant_entry;
+    void* first_entry;
+} __jit_debug_descriptor = { 1, 0, 0, 0 };
+
+void __jit_debug_register_code()
+{
+}
+// end GDB JIT interface stub
+
+int main()
+{
+    return 0;
+}

lldb/trunk/packages/Python/lldbsuite/test/functionalities/jitloader_gdb/simple.mk

+LEVEL = ../../make
+
+C_SOURCES := simple.c
+EXE = simple
+
+include $(LEVEL)/Makefile.rules

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

template <typename ptr_t> struct jit_descriptor {
   uint32_t version;
   uint32_t action_flag; // Values are jit_action_t
   ptr_t relevant_entry; // pointer
   ptr_t first_entry;    // pointer
 };
 
 namespace {
 
+enum EnableJITLoaderGDB {
+  eEnableJITLoaderGDBDefault,
+  eEnableJITLoaderGDBOn,
+  eEnableJITLoaderGDBOff,
+};
+
+static constexpr OptionEnumValueElement g_enable_jit_loader_gdb_enumerators[] = {
+    {eEnableJITLoaderGDBDefault, "default", "Enable JIT compilation interface "
+     "for all platforms except macOS"},
+    {eEnableJITLoaderGDBOn, "on", "Enable JIT compilation interface"},
+    {eEnableJITLoaderGDBOff, "off", "Disable JIT compilation interface"}
+ };
+
 static constexpr PropertyDefinition g_properties[] = {
-    {"enable-jit-breakpoint", OptionValue::eTypeBoolean, true, true, nullptr,
-     {}, "Enable breakpoint on __jit_debug_register_code."}};
+    {"enable", OptionValue::eTypeEnum, true,
+     eEnableJITLoaderGDBDefault, nullptr,
+     OptionEnumValues(g_enable_jit_loader_gdb_enumerators),
+     "Enable GDB's JIT compilation interface (default: enabled on "
+     "all platforms except macOS)"}};
 
-enum { ePropertyEnableJITBreakpoint };
+enum { ePropertyEnable, ePropertyEnableJITBreakpoint };
 
 class PluginProperties : public Properties {
 public:
   static ConstString GetSettingName() {
     return JITLoaderGDB::GetPluginNameStatic();
   }
 
   PluginProperties() {
     m_collection_sp = std::make_shared<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);
+  EnableJITLoaderGDB GetEnable() const {
+    return (EnableJITLoaderGDB)m_collection_sp->GetPropertyAtIndexAsEnumeration(
+        nullptr, ePropertyEnable,
+        g_properties[ePropertyEnable].default_uint_value);
   }
 };
 
 typedef std::shared_ptr<PluginProperties> JITLoaderGDBPropertiesSP;
 
 static const JITLoaderGDBPropertiesSP &GetGlobalPluginProperties() {
   static const auto g_settings_sp(std::make_shared<PluginProperties>());
   return g_settings_sp;
void JITLoaderGDB::ModulesDidLoad(ModuleList &module_list) {
   if (!DidSetJITBreakpoint() && m_process->IsAlive())
     SetJITBreakpoint(module_list);
 }
 
 //------------------------------------------------------------------
 // Setup the JIT Breakpoint
 //------------------------------------------------------------------
 void JITLoaderGDB::SetJITBreakpoint(lldb_private::ModuleList &module_list) {
-  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(

 //------------------------------------------------------------------
 lldb_private::ConstString JITLoaderGDB::GetPluginNameStatic() {
   static ConstString g_name("gdb");
   return g_name;
 }
 
 JITLoaderSP JITLoaderGDB::CreateInstance(Process *process, bool force) {
   JITLoaderSP jit_loader_sp;
+  bool enable;
+  switch (GetGlobalPluginProperties()->GetEnable()) {
+    case EnableJITLoaderGDB::eEnableJITLoaderGDBOn:
+      enable = true;
+      break;
+    case EnableJITLoaderGDB::eEnableJITLoaderGDBOff:
+      enable = false;
+      break;
+    case EnableJITLoaderGDB::eEnableJITLoaderGDBDefault:
       ArchSpec arch(process->GetTarget().GetArchitecture());
-  if (arch.GetTriple().getVendor() != llvm::Triple::Apple)
+      enable = arch.GetTriple().getVendor() != llvm::Triple::Apple;
+      break;
+  }
+  if (enable)
     jit_loader_sp = std::make_shared<JITLoaderGDB>(process);
   return jit_loader_sp;
 }
 
 const char *JITLoaderGDB::GetPluginDescriptionStatic() {
   return "JIT loader plug-in that watches for JIT events using the GDB "
          "interface.";
 }