[lldb/Lua] add support for Lua function breakpoint

[tammela]

Summary

Adds support for running a Lua function when a breakpoint is hit.

Example:

breakpoint command add -s lua -F abc

The above runs the Lua function 'abc' passing 2 arguments. 'frame', 'bp_loc' and 'extra_args'.

A third parameter 'extra_args' is only present when there is structured data
declared in the command line.

Example:

breakpoint command add -s lua -F abc -k foo -v bar

[tammela]

ping

[JDevlieghere]

lldb/bindings/lua/lua-wrapper.swig

25–29

I don't think you need the lambda. I used unique_ptr but llvm:Optional would work as well.

lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

16

Can we unpack the SBStructuredData and check for foo or 123 in the output?

[tammela]

lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

16

While I was doing this change, I noted that the SBStructuredData API for GetStringValue is quite odd.
For Lua, the auto-generated SWIG wrapper enforces code like this:

local result = "xxxx"
sd:GetStringValue(result, 3) -- 'sd' should be a value with at most 3 characters

This sort of API leaks all sorts of details to the Lua script. I think it could be improved to just return a std::string / llvm::StringRef object.

The SWIG wrapper also has some bugs for this class. For instance it's using lua_pushnumber where it should be lua_pushinteger for the GetIntegerValue method.

I will report this to the SWIG authors.

[tammela]

Addressing comment

[tammela]

lldb/bindings/lua/lua-wrapper.swig

25–29

{} is not valid on the ternary operator, but I think the version suits your suggestion.

[tammela]

Rebasing and unwrapping the SBStructuredData test case

[tammela]

@JDevlieghere Addressed all your comments.

[JDevlieghere]

LGTM

lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

16

I still think this is important. Can you add a FIXME so we remember update this in the future?

lldb/bindings/lua/lua-swigsafecast.swig

 template <typename SBClass>
 void
 PushSBClass (lua_State* L, SBClass* obj);
 
 void
 PushSBClass (lua_State* L, lldb::SBFrame* frame_sb)
 {
    SWIG_NewPointerObj(L, frame_sb, SWIGTYPE_p_lldb__SBFrame, 0);
 }
 
 void
 PushSBClass (lua_State* L, lldb::SBBreakpointLocation* breakpoint_location_sb)
 {
    SWIG_NewPointerObj(L, breakpoint_location_sb, SWIGTYPE_p_lldb__SBBreakpointLocation, 0);
 }
+
+void
+PushSBClass (lua_State* L, lldb::SBStructuredData* structured_data_sb)
+{
+   SWIG_NewPointerObj(L, structured_data_sb, SWIGTYPE_p_lldb__SBStructuredData, 0);
+}

lldb/bindings/lua/lua-typemaps.swig

 
 %apply unsigned long { size_t };
 %apply const unsigned long & { const size_t & };
 %apply long { ssize_t };
 %apply const long & { const ssize_t & };
 
 //===----------------------------------------------------------------------===//
 
+// FIXME:
+//  Ideally all the typemaps should be revisited in a future SB API revision.
+//  Typemaps, usually, modifies the function signatures and might spawn
+//  different LLDB APIs across languages (C++, Python, Lua...).
+//  Historically, typemaps have been used to replace SWIG's deficiencies,
+//  but SWIG itself evolved and some API design choices are now redundant.
+
+//===----------------------------------------------------------------------===//
+
 // Typemap definitions to allow SWIG to properly handle char buffer.
 
 // typemap for a char buffer
 %typemap(in) (char *dst, size_t dst_len) {
    $2 = luaL_checkinteger(L, $input);
    if ($2 <= 0) {
        return luaL_error(L, "Positive integer expected");
    }

lldb/bindings/lua/lua-wrapper.swig

 %header %{
 
 template <typename T>
 void
 PushSBClass(lua_State* L, T* obj);
 
 %}
 
 %wrapper %{
 
 // This function is called from Lua::CallBreakpointCallback
 SWIGEXPORT llvm::Expected<bool>
 LLDBSwigLuaBreakpointCallbackFunction
 (
    lua_State *L,
    lldb::StackFrameSP stop_frame_sp,
-   lldb::BreakpointLocationSP bp_loc_sp
+   lldb::BreakpointLocationSP bp_loc_sp,
+   StructuredDataImpl *extra_args_impl
 )
 {
    lldb::SBFrame sb_frame(stop_frame_sp);
    lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
+   int nargs = 2;
+
+   llvm::Optional<lldb::SBStructuredData> extra_args;
+   if (extra_args_impl)
+      extra_args = lldb::SBStructuredData(extra_args_impl);
 
    // Push the Lua wrappers
    PushSBClass(L, &sb_frame);
    PushSBClass(L, &sb_bp_loc);
 
+   if (extra_args.hasValue()) {
+      PushSBClass(L, extra_args.getPointer());
+      nargs++;
+   }
+
    // Call into the Lua callback passing 'sb_frame' and 'sb_bp_loc'.
    // Expects a boolean return.
-   if (lua_pcall(L, 2, 1, 0) != LUA_OK) {
+   if (lua_pcall(L, nargs, 1, 0) != LUA_OK) {
       llvm::Error E = llvm::make_error<llvm::StringError>(
             llvm::formatv("{0}\n", lua_tostring(L, -1)),
             llvm::inconvertibleErrorCode());
       // Pop error message from the stack.
       lua_pop(L, 1);
       return std::move(E);
    }
 

lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h

 //===-- ScriptInterpreterLua.h ----------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Lua_h_
 #define liblldb_Lua_h_
 
 #include "lldb/API/SBBreakpointLocation.h"
 #include "lldb/API/SBFrame.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
 #include "lua.hpp"
 
 #include <mutex>
 
 namespace lldb_private {
 
 extern "C" {
 int luaopen_lldb(lua_State *L);
 }
 
 class Lua {
 public:
   Lua();
   ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error RegisterBreakpointCallback(void *baton, const char *body);
   llvm::Expected<bool>
   CallBreakpointCallback(void *baton, lldb::StackFrameSP stop_frame_sp,
-                         lldb::BreakpointLocationSP bp_loc_sp);
+                         lldb::BreakpointLocationSP bp_loc_sp,
+                         StructuredData::ObjectSP extra_args_sp);
   llvm::Error LoadModule(llvm::StringRef filename);
   llvm::Error CheckSyntax(llvm::StringRef buffer);
   llvm::Error ChangeIO(FILE *out, FILE *err);
 
 private:
   lua_State *m_lua_state;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_Lua_h_

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

 // Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
 // C-linkage specified, but returns UDT 'llvm::Expected<bool>' which is
 // incompatible with C
 #if _MSC_VER
 #pragma warning (push)
 #pragma warning (disable : 4190)
 #endif
 
-extern "C" llvm::Expected<bool>
-LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
-                                      lldb::StackFrameSP stop_frame_sp,
-                                      lldb::BreakpointLocationSP bp_loc_sp);
+extern "C" llvm::Expected<bool> LLDBSwigLuaBreakpointCallbackFunction(
+    lua_State *L, lldb::StackFrameSP stop_frame_sp,
+    lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl);
 
 #if _MSC_VER
 #pragma warning (pop)
 #endif
 
 #pragma clang diagnostic pop
 
 static int lldb_print(lua_State *L) {
if (luaL_dostring(m_lua_state, func_str.c_str()) != LUA_OK) {
     return e;
   }
   lua_settable(m_lua_state, LUA_REGISTRYINDEX);
   return llvm::Error::success();
 }
 
 llvm::Expected<bool>
 Lua::CallBreakpointCallback(void *baton, lldb::StackFrameSP stop_frame_sp,
-                            lldb::BreakpointLocationSP bp_loc_sp) {
+                            lldb::BreakpointLocationSP bp_loc_sp,
+                            StructuredData::ObjectSP extra_args_sp) {
+
   lua_pushlightuserdata(m_lua_state, baton);
   lua_gettable(m_lua_state, LUA_REGISTRYINDEX);
+  auto *extra_args_impl = [&]() -> StructuredDataImpl * {
+    if (extra_args_sp == nullptr)
+      return nullptr;
+    auto *extra_args_impl = new StructuredDataImpl();
+    extra_args_impl->SetObjectSP(extra_args_sp);
+    return extra_args_impl;
+  }();
   return LLDBSwigLuaBreakpointCallbackFunction(m_lua_state, stop_frame_sp,
-                                               bp_loc_sp);
+                                               bp_loc_sp, extra_args_impl);
 }
 
 llvm::Error Lua::CheckSyntax(llvm::StringRef buffer) {
   int error =
       luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer");
   if (error == LUA_OK) {
     // Pop buffer
     lua_pop(m_lua_state, 1);

lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h

 //===-- ScriptInterpreterLua.h ----------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_ScriptInterpreterLua_h_
 #define liblldb_ScriptInterpreterLua_h_
 
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-enumerations.h"
 
 namespace lldb_private {
 class Lua;
 class ScriptInterpreterLua : public ScriptInterpreter {
 public:
   class CommandDataLua : public BreakpointOptions::CommandData {
   public:
     CommandDataLua() : BreakpointOptions::CommandData() {
       interpreter = lldb::eScriptLanguageLua;
     }
+    CommandDataLua(StructuredData::ObjectSP extra_args_sp)
+        : BreakpointOptions::CommandData(), m_extra_args_sp(extra_args_sp) {
+      interpreter = lldb::eScriptLanguageLua;
+    }
+    StructuredData::ObjectSP m_extra_args_sp;
   };
 
   ScriptInterpreterLua(Debugger &debugger);
 
   ~ScriptInterpreterLua() override;
 
   bool ExecuteOneLine(
       llvm::StringRef command, CommandReturnObject *result,
public:
 
   void CollectDataForBreakpointCommandCallback(
       std::vector<BreakpointOptions *> &bp_options_vec,
       CommandReturnObject &result) override;
 
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
                                       const char *command_body_text) override;
 
+  Status SetBreakpointCommandCallbackFunction(
+      BreakpointOptions *bp_options, const char *function_name,
+      StructuredData::ObjectSP extra_args_sp) override;
+
 private:
   std::unique_ptr<Lua> m_lua;
   bool m_session_is_active = false;
+
+  Status RegisterBreakpointCallback(BreakpointOptions *bp_options,
+                                    const char *command_body_text,
+                                    StructuredData::ObjectSP extra_args_sp);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_ScriptInterpreterLua_h_

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

bool ScriptInterpreterLua::BreakpointCallbackFunction(
   BreakpointSP breakpoint_sp = target->GetBreakpointByID(break_id);
   BreakpointLocationSP bp_loc_sp(breakpoint_sp->FindLocationByID(break_loc_id));
 
   Debugger &debugger = target->GetDebugger();
   ScriptInterpreterLua *lua_interpreter = static_cast<ScriptInterpreterLua *>(
       debugger.GetScriptInterpreter(true, eScriptLanguageLua));
   Lua &lua = lua_interpreter->GetLua();
 
-  llvm::Expected<bool> BoolOrErr =
-      lua.CallBreakpointCallback(baton, stop_frame_sp, bp_loc_sp);
+  CommandDataLua *bp_option_data = static_cast<CommandDataLua *>(baton);
+  llvm::Expected<bool> BoolOrErr = lua.CallBreakpointCallback(
+      baton, stop_frame_sp, bp_loc_sp, bp_option_data->m_extra_args_sp);
   if (llvm::Error E = BoolOrErr.takeError()) {
     debugger.GetErrorStream() << toString(std::move(E));
     return true;
   }
 
   return *BoolOrErr;
 }
 
 void ScriptInterpreterLua::CollectDataForBreakpointCommandCallback(
     std::vector<BreakpointOptions *> &bp_options_vec,
     CommandReturnObject &result) {
   IOHandlerSP io_handler_sp(
       new IOHandlerLuaInterpreter(m_debugger, *this, eIOHandlerBreakpoint));
   io_handler_sp->SetUserData(&bp_options_vec);
   m_debugger.RunIOHandlerAsync(io_handler_sp);
 }
 
+Status ScriptInterpreterLua::SetBreakpointCommandCallbackFunction(
+    BreakpointOptions *bp_options, const char *function_name,
+    StructuredData::ObjectSP extra_args_sp) {
+  const char *fmt_str = "return {0}(frame, bp_loc, ...)";
+  std::string oneliner = llvm::formatv(fmt_str, function_name).str();
+  return RegisterBreakpointCallback(bp_options, oneliner.c_str(),
+                                    extra_args_sp);
+}
+
 Status ScriptInterpreterLua::SetBreakpointCommandCallback(
     BreakpointOptions *bp_options, const char *command_body_text) {
+  return RegisterBreakpointCallback(bp_options, command_body_text, {});
+}
+
+Status ScriptInterpreterLua::RegisterBreakpointCallback(
+    BreakpointOptions *bp_options, const char *command_body_text,
+    StructuredData::ObjectSP extra_args_sp) {
   Status error;
-  auto data_up = std::make_unique<CommandDataLua>();
+  auto data_up = std::make_unique<CommandDataLua>(extra_args_sp);
   error = m_lua->RegisterBreakpointCallback(data_up.get(), command_body_text);
   if (error.Fail())
     return error;
   auto baton_sp =
       std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_up));
   bp_options->SetCallback(ScriptInterpreterLua::BreakpointCallbackFunction,
                           baton_sp);
   return error;

lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

+# REQUIRES: lua
+# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t
+# RUN: %lldb -s %s --script-language lua %t 2>&1 | FileCheck %s
+b main
+script
+function abc(a, b, c, ...)
+print(c)
+if c then print(c:GetValueForKey("foo"):GetStringValue(32)) end
+end
+quit
+breakpoint command add -s lua -F abc
+r
+# CHECK: nil
+breakpoint command add -s lua -F abc -k foo -v 123pizza!
+r
+# CHECK: <userdata of type 'lldb::SBStructuredData *' at {{0x[[:xdigit:]]+}}>
+# CHECK: 123pizza!
+breakpoint command add -s lua -o "abc(frame, bp_loc, ...)"
+r
+# CHECK: nil
+breakpoint command add -s lua -F typo
+r
+# CHECK: attempt to call a nil value

lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

 // Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
 // C-linkage specified, but returns UDT 'llvm::Expected<bool>' which is
 // incompatible with C
 #if _MSC_VER
 #pragma warning (push)
 #pragma warning (disable : 4190)
 #endif
 
-extern "C" llvm::Expected<bool>
-LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
-                                      lldb::StackFrameSP stop_frame_sp,
-                                      lldb::BreakpointLocationSP bp_loc_sp) {
+extern "C" llvm::Expected<bool> LLDBSwigLuaBreakpointCallbackFunction(
+    lua_State *L, lldb::StackFrameSP stop_frame_sp,
+    lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl) {
   return false;
 }
 
 #if _MSC_VER
 #pragma warning (pop)
 #endif
 
 #pragma clang diagnostic pop