[Lldb-commits] [PATCH] D71801: [lldb/Lua] Make lldb.debugger et al available to Lua

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D71801#1807109 <https://reviews.llvm.org/D71801#1807109>, @jingham wrote:

> In D71801#1794758 <https://reviews.llvm.org/D71801#1794758>, @labath wrote:
>
> > +Jim, for his thoughts on debugger+interpreter relationship
> >
> > I think this is the time to step back and discuss the relationship between debugger and script interpreter contexts...
>
>
> I actually have no strong opinion about HOW this should be implemented.  Before starting on lldb, I had mostly used Tcl when I was incorporating a scripting language into various tools.  In Tcl, it was natural to make separate independent interpreters.  For instance, the security model for Tcl involved making a secure interpreter that forwarded messages to a shadow - insecure - interpreter that would implement whatever sandboxing you wanted to do.  Tcl interpreters were independent entities with no shared state.  That was the model I was working on when we chose to use Python for lldb, but that's not really how Python thinks of interpreters, and as you note, that caused us a bunch of headaches.
>
> I do feel pretty strongly that to the user, every SBDebugger should have a separate interpreter state.  That really has to be true.  In Xcode, each SBDebugger represents a separate project that the user is debugging, and these often have no relation to one another.  Doing something in one script interpreter and having that affect what should be an entirely independent debugging session would be bad.  Since it is not uncommon for data formatters and Python command modules to use some global state, this could lead to some really frustrating bugs.
>
> It isn't surprising that we have to do different things for each interpreter.  For instance, in Tcl you'd just create a new interpreter and you'd be done.
>
> So my take is that being able to support multiple independent interpreters is a minimum requirement for supporting a scripting language in lldb.  If it can't do that, then we shouldn't try to support it.
>
> I don't know enough about Lua to know whether it passes this (fairly low) bar...


Thanks for your thoughts, Jim. I'm pretty sure that independent interpreters are possible in lua, and this patch does achieve that. Actually, even the previous version achieved that, but I somehow got confused there -- for some reason I thought that ScriptInterpreter objects are shared between Debuggers, but that is clearly not the case -- and the test that Jonas added demonstrates that.

As for the patch itself, I'm afraid that this version has simplified things too much. :D We can't set the `thread` (etc.) convenience variables during initialization, as these can change over time. Some EnterSession/LeaveSession logic will still be needed. I guess we will also need something like the `m_session_is_active` "locking" logic, but for a slightly different reason than I originally thought -- not to protect concurrent sessions in different debuggers, but to handle nested sessions in the same debugger/scriptinterpreter object:

  lldb) file /bin/ls
  Current executable set to '/bin/ls' (x86_64).
  (lldb) file /bin/cat
  Current executable set to '/bin/cat' (x86_64).
  (lldb) script
  >>> print lldb.target, lldb.debugger.GetSelectedTarget()
  cat cat
  >>> lldb.debugger.SetSelectedTarget(lldb.debugger.GetTargetAtIndex(0))
  >>> print lldb.target, lldb.debugger.GetSelectedTarget()
  cat ls
  >>> lldb.debugger.HandleCommand("script print lldb.target, lldb.debugger.GetSelectedTarget()")
  cat ls
  >>> print lldb.target, lldb.debugger.GetSelectedTarget()
  cat ls
  >>> ^D
  (lldb) script
  >>> print lldb.target, lldb.debugger.GetSelectedTarget()
  ls ls

The current python behavior is for nested sessions to not alter the convenience variable values (they preserve the value of the outermost session), so I guess we ought to emulate that (though I think a case could also be made for changing their values on every session entry, as long as the original values are restored when the session terminates).



================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:31-39
+llvm::Error Lua::EnterSession(user_id_t debugger_id) {
+  const char *fmt_str =
+      "lldb.debugger = lldb.SBDebugger.FindDebuggerWithID({0}); "
+      "lldb.target = lldb.debugger:GetSelectedTarget(); "
+      "lldb.process = lldb.target:GetProcess(); "
+      "lldb.thread = lldb.process:GetSelectedThread(); "
+      "lldb.frame = lldb.thread:GetSelectedFrame()";
----------------
I'd put this function into ScriptInterpreterLua. I'd try keep the Lua class relatively generic/low-level, where this thing is all about the specifics of the SB api.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:46
+      m_lua(std::make_unique<Lua>()) {
+  llvm::cantFail(GetLua().EnterSession(debugger.GetID()));
+}
----------------
I don't think this is right. You should be able to set `lldb.debugger` once and for all during initialization, but the rest of the variables (thread, process, target) still need to be set upon entering the lua script as these can vary during the lifetime of an Debugger.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71801/new/

https://reviews.llvm.org/D71801