Rework debugger init script handling to handle LLDB quirks.

The current hack of `script eval(...)` for LLDB requires our script to
use `lldb.debugger`. Some other unrelated hacks involving LLDB create
a situation where `lldb.debugger` is sometimes nondeterministically
set to `None` temporarily and asynchronously, during a short period
after debugger startup. The recommended way to run a script from a
file is `command script import`. Unfortunately `command script import`
requires the script file basename to be a valid Python identifer plus
the `.py` suffix, so we can't use our proc-pid scheme anymore.
Instead we now make the init script be responsible for deleting itself
when it finishes running.

We also have to reformat the LLDB init script to put the code
that needs to run eagerly in a `__lldb_init_module()` function. This
function takes a `debugger` argument so we can avoid using
`lldb.debugger`, which is the ultimate point of this change.

Author: rocallahan

src/DebuggerExtensionCommandHandler.cc

@@ -58,19 +58,17 @@ def hex_escape(string):
             curr_char = ord(curr_char)
         result += format(curr_char, '02x')
     return result
-
 )Delimiter";
 
 string DebuggerExtensionCommandHandler::gdb_macros() {
   DebuggerExtensionCommand::init_auto_args();
   stringstream ss;
   ss << R"Delimiter(set python print-stack full
 python
+import re
 )Delimiter"
     << shared_python
     << R"Delimiter(
-import re
-
 class RRWhere(gdb.Command):
     """Helper to get the location for checkpoints/history. Used by auto-args"""
     def __init__(self):
@@ -193,7 +191,8 @@ end
   return ss.str();
 }
 
-static void lldb_macro_binding(ostream& ss, const DebuggerExtensionCommand& cmd) {
+static void lldb_macro_binding(ostream& def_stream, ostream& call_stream,
+                               const DebuggerExtensionCommand& cmd) {
   string func_name = "rr_command_";
   for (char ch : cmd.name()) {
     if (ch == ' ') {
@@ -205,29 +204,30 @@ static void lldb_macro_binding(ostream& ss, const DebuggerExtensionCommand& cmd)
     }
     func_name.push_back(ch);
   }
-  ss << "def " << func_name << "(debugger, command, exe_ctx, result, internal_dict):\n";
+  def_stream << "def " << func_name << "(debugger, command, exe_ctx, result, internal_dict):\n";
   if (!cmd.docs().empty()) {
-    ss << "    \"\"\"" << cmd.docs() << "\"\"\"\n";
+    def_stream << "    \"\"\"" << cmd.docs() << "\"\"\"\n";
   }
-  ss << "    cmd_name = '" << cmd.name() << "'\n"
-     << "    auto_args = ";
-  print_python_string_array(ss, cmd.auto_args());
-  ss << "\n"
-     << "    command_impl(debugger, command, exe_ctx, result, cmd_name, auto_args)\n"
-     << "\n"
-     << "lldb.debugger.HandleCommand('command script add -f " << func_name
-     << " " << cmd.name() << "')\n\n";
+  def_stream << "    cmd_name = '" << cmd.name() << "'\n"
+             << "    auto_args = ";
+  print_python_string_array(def_stream, cmd.auto_args());
+  def_stream << "\n"
+             << "    command_impl(debugger, command, exe_ctx, result, cmd_name, auto_args)\n"
+             << "\n";
+  call_stream << "    debugger.HandleCommand('command script add -f " << func_name
+              << " " << cmd.name() << "')\n";
 }
 
-string DebuggerExtensionCommandHandler::lldb_python_macros() {
+DebuggerExtensionCommandHandler::LldbCommands
+DebuggerExtensionCommandHandler::lldb_python_macros() {
   DebuggerExtensionCommand::init_auto_args();
   stringstream ss;
-  ss << shared_python
-     << R"Delimiter(
-import lldb
+  ss << R"Delimiter(import lldb
 import re
 import shlex
-
+)Delimiter"
+     << shared_python
+     << R"Delimiter(
 def run_command_and_get_output(debugger, command):
     result = lldb.SBCommandReturnObject()
     debugger.GetCommandInterpreter().HandleCommand(command, result)
@@ -256,12 +256,16 @@ def command_impl(debugger, command, exe_ctx, result, cmd_name, auto_args):
 
 )Delimiter";
 
+  stringstream call_stream;
   if (debugger_command_list) {
     for (auto& it : *debugger_command_list) {
-      lldb_macro_binding(ss, *it);
+      lldb_macro_binding(ss, call_stream, *it);
     }
   }
-  return ss.str();
+  LldbCommands cmds;
+  cmds.toplevel_definitions = ss.str();
+  cmds.run_on_startup = call_stream.str();
+  return cmds;
 }
 
 /*static*/ DebuggerExtensionCommand* DebuggerExtensionCommandHandler::command_for_name(const string& name) {

src/DebuggerExtensionCommandHandler.h

@@ -25,7 +25,12 @@ class DebuggerExtensionCommandHandler {
 
   // Declare any registered command with supporting
   // wrapper code --- LLDB Python script.
-  static std::string lldb_python_macros();
+  struct LldbCommands {
+    std::string toplevel_definitions;
+    // 2-space-indented code to run on startup.
+    std::string run_on_startup;
+  };
+  static LldbCommands lldb_python_macros();
 
   static void register_command(DebuggerExtensionCommand& cmd);
 

src/launch_debugger.cc

@@ -26,108 +26,115 @@ namespace rr {
 // Special-sauce macros defined by rr when launching the gdb client,
 // which implement functionality outside of the gdb remote protocol.
 // (Don't stare at them too long or you'll go blind ;).)
-static const string& gdb_rr_macros() {
-  static string s;
-
-  if (s.empty()) {
-    stringstream ss;
-    ss << DebuggerExtensionCommandHandler::gdb_macros()
-       // gdb warns about redefining inbuilt commands, silence that by
-       // wrapping it in python code
-       << "python gdb.execute('define jump\\nrr-denied jump\\nend')\n"
-       << "python gdb.execute('define restart\\nrun c$arg0\\nend')\n"
-       << "document restart\n"
-       << "restart at checkpoint N\n"
-       << "checkpoints are created with the 'checkpoint' command\n"
-       << "end\n"
-       << "define seek-ticks\n"
-       << "  run t$arg0\n"
-       << "end\n"
-       << "document seek-ticks\n"
-       << "restart at given ticks value\n"
-       << "end\n"
-       // In gdb version "Fedora 7.8.1-30.fc21", a raw "run" command
-       // issued before any user-generated resume-execution command
-       // results in gdb hanging just after the inferior hits an internal
-       // gdb breakpoint.  This happens outside of rr, with gdb
-       // controlling gdbserver, as well.  We work around that by
-       // ensuring *some* resume-execution command has been issued before
-       // restarting the session.  But, only if the inferior hasn't
-       // already finished execution ($_thread != 0).  If it has and we
-       // issue the "stepi" command, then gdb refuses to restart
-       // execution.
-       << "define hook-run\n"
-       << "  rr-hook-run\n"
-       << "end\n"
-       << "define hookpost-continue\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-step\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-stepi\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-next\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-nexti\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-finish\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-reverse-continue\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-reverse-step\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-reverse-stepi\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-reverse-finish\n"
-       << "  rr-set-suppress-run-hook 1\n"
-       << "end\n"
-       << "define hookpost-run\n"
-       << "  rr-set-suppress-run-hook 0\n"
-       << "end\n"
-       << "set unwindonsignal on\n"
-       << "set non-stop off\n"
-       << "handle SIGURG stop\n"
-       << "set prompt (rr) \n"
-       // Try both "set target-async" and "maint set target-async" since
-       // that changed recently.
-       << "python\n"
-       << "import re\n"
-       << "m = re.compile(r"
-       << "'[^0-9]*([0-9]+)\\.([0-9]+)(\\.([0-9]+))?'"
-       << ").match(gdb.VERSION)\n"
-       << "ver = int(m.group(1))*10000 + int(m.group(2))*100\n"
-       << "if m.group(4):\n"
-       << "    ver = ver + int(m.group(4))\n"
-       << "\n"
-       << "if ver == 71100:\n"
-       << "    gdb.write("
-       << "'This version of gdb (7.11.0) has known bugs that break rr. "
-       << "Install 7.11.1 or later.\\n', gdb.STDERR)\n"
-       << "\n"
-       << "if ver < 71101:\n"
-       << "    gdb.execute('set target-async 0')\n"
-       << "    gdb.execute('maint set target-async 0')\n"
-       << "end\n";
-    s = ss.str();
+static string gdb_rr_macros(const string* file_to_delete) {
+  stringstream ss;
+  ss << DebuggerExtensionCommandHandler::gdb_macros()
+     // gdb warns about redefining inbuilt commands, silence that by
+     // wrapping it in python code
+     << "python gdb.execute('define jump\\nrr-denied jump\\nend')\n"
+     << "python gdb.execute('define restart\\nrun c$arg0\\nend')\n"
+     << "document restart\n"
+     << "restart at checkpoint N\n"
+     << "checkpoints are created with the 'checkpoint' command\n"
+     << "end\n"
+     << "define seek-ticks\n"
+     << "  run t$arg0\n"
+     << "end\n"
+     << "document seek-ticks\n"
+     << "restart at given ticks value\n"
+     << "end\n"
+     // In gdb version "Fedora 7.8.1-30.fc21", a raw "run" command
+     // issued before any user-generated resume-execution command
+     // results in gdb hanging just after the inferior hits an internal
+     // gdb breakpoint.  This happens outside of rr, with gdb
+     // controlling gdbserver, as well.  We work around that by
+     // ensuring *some* resume-execution command has been issued before
+     // restarting the session.  But, only if the inferior hasn't
+     // already finished execution ($_thread != 0).  If it has and we
+     // issue the "stepi" command, then gdb refuses to restart
+     // execution.
+     << "define hook-run\n"
+     << "  rr-hook-run\n"
+     << "end\n"
+     << "define hookpost-continue\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-step\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-stepi\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-next\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-nexti\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-finish\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-reverse-continue\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-reverse-step\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-reverse-stepi\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-reverse-finish\n"
+     << "  rr-set-suppress-run-hook 1\n"
+     << "end\n"
+     << "define hookpost-run\n"
+     << "  rr-set-suppress-run-hook 0\n"
+     << "end\n"
+     << "set unwindonsignal on\n"
+     << "set non-stop off\n"
+     << "handle SIGURG stop\n"
+     << "set prompt (rr) \n"
+     // Try both "set target-async" and "maint set target-async" since
+     // that changed recently.
+     << "python\n"
+     << "import re\n"
+     << "import os\n"
+     << "m = re.compile(r"
+     << "'[^0-9]*([0-9]+)\\.([0-9]+)(\\.([0-9]+))?'"
+     << ").match(gdb.VERSION)\n"
+     << "ver = int(m.group(1))*10000 + int(m.group(2))*100\n"
+     << "if m.group(4):\n"
+     << "    ver = ver + int(m.group(4))\n"
+     << "\n"
+     << "if ver == 71100:\n"
+     << "    gdb.write("
+     << "'This version of gdb (7.11.0) has known bugs that break rr. "
+     << "Install 7.11.1 or later.\\n', gdb.STDERR)\n"
+     << "\n"
+     << "if ver < 71101:\n"
+     << "    gdb.execute('set target-async 0')\n"
+     << "    gdb.execute('maint set target-async 0')\n";
+  if (file_to_delete) {
+    ss << "os.unlink('" << *file_to_delete << "')\n";
   }
-  return s;
+  ss << "end\n";
+  return ss.str();
 }
 
-static const string& lldb_python_rr_macros() {
+static const string& lldb_python_rr_macros(const string* file_to_delete) {
   static string s;
 
   if (s.empty()) {
+    auto cmds = DebuggerExtensionCommandHandler::lldb_python_macros();
     stringstream ss;
-    ss << DebuggerExtensionCommandHandler::lldb_python_macros()
-       << "lldb.debugger.HandleCommand('set set prompt \"(rr) \"')\n";
+    ss << cmds.toplevel_definitions
+       << "import os\n"
+       << "def  __lldb_init_module(debugger, internal_dict):\n"
+       << cmds.run_on_startup
+       << "    debugger.HandleCommand('set set prompt \"(rr) \"')\n";
+    if (file_to_delete) {
+      ss << "    os.unlink('" << *file_to_delete << "')\n";
+    }
+    ss << "\n";
     s = ss.str();
   }
   return s;
@@ -229,24 +236,6 @@ vector<string> debugger_launch_command(Task* t, int socket_domain,
   return cmd;
 }
 
-static string create_command_file(const string& macros) {
-  TempFile file = create_temporary_file("rr-debugger-commands-XXXXXX");
-  // This fd is just leaked. That's fine since we only call this once
-  // per rr invocation at the moment.
-  int fd = file.fd.extract();
-  unlink(file.name.c_str());
-
-  ssize_t len = macros.size();
-  int written = write(fd, macros.c_str(), len);
-  if (written != len) {
-    FATAL() << "Failed to write gdb command file";
-  }
-
-  stringstream procfile;
-  procfile << "/proc/" << getpid() << "/fd/" << fd;
-  return procfile.str();
-}
-
 string to_shell_string(const vector<string>& args) {
   stringstream ss;
   for (auto& a : args) {
@@ -290,12 +279,16 @@ void launch_debugger(ScopedFd& params_pipe_fd,
   cmd.push_back(debugger_file_path);
   vector<string> env = current_env();
 
+  // LLDB 'command script import' requires the filename to be a valid Python
+  // identifier.
+  TempFile file = create_temporary_file("rr_debugger_commands_XXXXXX");
   switch (debugger_type) {
     case DebuggerType::GDB: {
       push_default_gdb_options(cmd, serve_files);
-      string gdb_command_file = create_command_file(gdb_rr_macros());
+      string script = gdb_rr_macros(&file.name);
+      write_all(file.fd, script.data(), script.size());
       cmd.push_back("-x");
-      cmd.push_back(gdb_command_file);
+      cmd.push_back(file.name);
 
       bool did_set_remote = false;
       for (size_t i = 0; i < options.size(); ++i) {
@@ -317,12 +310,17 @@ void launch_debugger(ScopedFd& params_pipe_fd,
       cmd.push_back("--source-quietly");
       cmd.insert(cmd.end(), options.begin(), options.end());
       push_lldb_target_remote_cmd(cmd, socket_domain, host, port);
-      // We have to load the commands as a Python script. If we
-      // use the "script" command to launch a nested Python interpreter,
-      // Python emits some annoying text that we dont want to see.
-      string lldb_command_file = create_command_file(lldb_python_rr_macros());
+      // LLDB 'command script import' requires the file to end in '.py'.
+      string new_name = file.name + ".py";
+      if (renameat2(AT_FDCWD, file.name.c_str(), AT_FDCWD, new_name.c_str(),
+                    RENAME_NOREPLACE)) {
+        FATAL() << "Can't fix temp file name";
+      }
+      file.name = new_name;
+      string script = lldb_python_rr_macros(&file.name);
+      write_all(file.fd, script.data(), script.size());
       cmd.push_back("-o");
-      cmd.push_back("script exec(open('" + lldb_command_file + "').read())");
+      cmd.push_back("command script import " + file.name);
       env.push_back("LLDB_UNDER_RR=1");
       break;
     }
@@ -392,8 +390,8 @@ void emergency_debug(Task* t) {
   GdbServer::serve_emergency_debugger(std::move(dbg), t);
 }
 
-string gdb_init_script() { return gdb_rr_macros(); }
+string gdb_init_script() { return gdb_rr_macros(nullptr); }
 
-string lldb_init_script() { return lldb_python_rr_macros(); }
+string lldb_init_script() { return lldb_python_rr_macros(nullptr); }
 
 } // namespace rr