Page MenuHomePhabricator

[lldb/Lua] Make lldb.debugger et al available to Lua
ClosedPublic

Authored by JDevlieghere on Dec 21 2019, 3:42 PM.

Details

Reviewers
labath
jingham
Group Reviewers
Restricted Project
Commits
rG45c971f7eef1: [lldb/Lua] Make lldb.debugger et al available to Lua
Summary

The Python script interpreter makes the current debugger, target, process, thread and frame available to interactive scripting sessions through convenience variables. This patch does the same for Lua.

Diff Detail

Event Timeline

JDevlieghere created this revision.Dec 21 2019, 3:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 21 2019, 3:42 PM

+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...

So, the way I understand the python code, our intention really was to have each (SB)Debugger be independently scriptable, but achieving this with python was hard, as the python state is very global. That's why the python script interpreter needs to jump through a lot of hoops in order to make the Debuggers *appear* to be independent. Here, you're setting yourself up to do the same with lua. That is not completely unreasonable (it's consistent, at the very least), but:
a) it may not be possible if lua is not sufficiently flexible to enable faking independent global variables

(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> foobar = 47
>>> foobar
47
>>> d = lldb.SBDebugger.Create()
>>> d.HandleCommand("script foobar = 42")
>>> foobar
47
>>> d.HandleCommand("script foobar"))))))
42

In particular, a lua context (AFAIK) is completely single-threaded so two debugger objects would never be able to run the lua interpreter concurrently.

b) It is unnecessary, because lua is perfectly capable of creating completely independent contexts.

For these reasons, I'd like to explore the possibility of just creating distinct lua contexts for each (SB)Debugger object. I think we could drop a lot of complexity this way (the weird session_is_active "locking" is just the tip of the iceberg). Doing that will probably require a bit of refactoring, as right now the assumption is that each ScriptInterpreter instance is global, but I don't think that should be too hard (for python we could have a per-debugger shin, backed by a global object).

It may turn out that this is a dead-end, because the lua context will be "too independent", but I'd be sad if we didn't even try that. In particular, if this pans out and we think that's a good design, I think we could do some work to cleanup/simplify python as a result (PyInterpreterState_New and friends).

lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
45

This m_session_is_active business is very confusing, and probably incorrect.
Imagine the following sequence:

A: EnterSession() # sets m_session_is_active to true
B: EnterSession() # noop
B: LeaveSession() # sets m_session_is_active to false
B: EnterSession() # sets m_session_is_active to true, when it probably shouldn't
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
36

Does this mean that executing something like script lldb = nil will cause a crash? I don't think we necessarily need to protect against that, but I thought it's worth mentioning that...

JDevlieghere planned changes to this revision.Dec 23 2019, 9:42 AM

Thanks Pavel, I must admit I gave this little thought and just mirrored the Python approach. I *think* it should easy to implement things the way you describe (famous last words).

  • Keep one set of convenience variables per lua state/script interpreter
  • Add test case
JDevlieghere marked 2 inline comments as done.Dec 23 2019, 1:55 PM
jingham added a comment.EditedJan 6 2020, 5:12 PM

+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...

So, the way I understand the python code, our intention really was to have each (SB)Debugger be independently scriptable, but achieving this with python was hard, as the python state is very global. That's why the python script interpreter needs to jump through a lot of hoops in order to make the Debuggers *appear* to be independent. Here, you're setting yourself up to do the same with lua. That is not completely unreasonable (it's consistent, at the very least), but:
a) it may not be possible if lua is not sufficiently flexible to enable faking independent global variables

(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> foobar = 47
>>> foobar
47
>>> d = lldb.SBDebugger.Create()
>>> d.HandleCommand("script foobar = 42")
>>> foobar
47
>>> d.HandleCommand("script foobar"))))))
42

In particular, a lua context (AFAIK) is completely single-threaded so two debugger objects would never be able to run the lua interpreter concurrently.

b) It is unnecessary, because lua is perfectly capable of creating completely independent contexts.

For these reasons, I'd like to explore the possibility of just creating distinct lua contexts for each (SB)Debugger object. I think we could drop a lot of complexity this way (the weird session_is_active "locking" is just the tip of the iceberg). Doing that will probably require a bit of refactoring, as right now the assumption is that each ScriptInterpreter instance is global, but I don't think that should be too hard (for python we could have a per-debugger shin, backed by a global object).

It may turn out that this is a dead-end, because the lua context will be "too independent", but I'd be sad if we didn't even try that. In particular, if this pans out and we think that's a good design, I think we could do some work to cleanup/simplify python as a result (PyInterpreterState_New and friends).

labath added a comment.Jan 7 2020, 2:40 AM

+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).

lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
30–38

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.

lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
52

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.

  • Address feedback Pavel
  • Add test for nested session
JDevlieghere marked 2 inline comments as done.Jan 7 2020, 11:25 AM
JDevlieghere added inline comments.
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
52

I prefer setting/unsetting them together. This also ensures you don't have access to lldb.debugger outside of an interactive session.

labath added inline comments.Jan 8 2020, 3:38 AM
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
52

(Un)setting them together is fine, but i don't see why you need to enter a session in the ScriptInterpreterLua constructor now that it is done in IOHandlerLuaInterpreter. I think this part should be deleted.

lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test
8–18

Would it be possible to group these by two or otherwise give some context to each line (e.g. inlining nested_sessions.in into this file would help). This way, the checks are very opaque...

JDevlieghere updated this revision to Diff 236846.
JDevlieghere marked an inline comment as done.
JDevlieghere marked an inline comment as done.

Feedback Pavel

labath accepted this revision.Jan 9 2020, 12:04 AM

Thanks for your patience. Looks fine to me, just remove the dead code..

lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
29–38

This should be dead code now, right?

This revision is now accepted and ready to land.Jan 9 2020, 12:04 AM
This revision was automatically updated to reflect the committed changes.