[Lldb-commits] [PATCH] D104281: [lldb][docs] Add reference docs for Lua scripting

[Pedro Tammela via Phabricator via lldb-commits]

tammela requested changes to this revision.
tammela added a comment.
This revision now requires changes to proceed.

@siger-young

Are these still relevant to the code that was landed recently?
For this one, please wait @JDevlieghere's review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104281

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela accepted this revision.
tammela added a comment.

LGTM.

You will have to rebase to main.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+

siger-young wrote:
> tammela wrote:
> > Could we not use an external dependency?
> > For instance in my setup it fails because it couldn't find this library.
> Does it make sense to directly copy "luaunit.lua" to the Lua test dir?
You don't seem to have a hard dependency on it.
Couldn't you just replicate what you are interested? Instead of bringing in a 
full blown unit testing framework...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

Just one last thing and I think we are done!

Thanks for your work.




Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+

Could we not use an external dependency?
For instance in my setup it fails because it couldn't find this library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

Hi Siger,

We are almost there.
I encourage you to continue working on this patch.

I have been busy the past weeks, but I will try to review ASAP once you address 
my comments.




Comment at: lldb/bindings/lua/lua-typemaps.swig:219-221
+%typecheck(SWIG_TYPECHECK_STRING_ARRAY) char ** {
+   $1 = (lua_istable(L, $input) || lua_isnil(L, $input));
+}

siger-young wrote:
> tammela wrote:
> > This is not being generated by SWIG for some reason.
> > 
> > I think it's more readable to just have an else clause that raises an error 
> > on line 212.
> I tried commenting and uncommenting these lines then diff, found that the 
> typecheck is actually generated on those overloaded functions. 
> `SBTarget::Launch` no longer works after commenting in my cases:
> ```
> lua5.3: test.lua:27: Wrong arguments for overloaded function 'SBTarget_Launch'
>   Possible C/C++ prototypes are:
> lldb::SBTarget::Launch(lldb::SBListener &,char const **,char const 
> **,char const *,char const *,char const *,char const 
> *,uint32_t,bool,lldb::SBError &)
> lldb::SBTarget::Launch(lldb::SBLaunchInfo &,lldb::SBError &)
> ```
> 
> It seems that SWIG uses "SWIG_isptrtype" to decide arg matches "char**", so 
> this typecheck is necessary.
> 
> For those functions with just one definition (i.e. not overloaded), this 
> typecheck does nothing on them, so it will be `typemap(in)`'s responsibility 
> to check arg types.
I see, Thanks.
That was indeed a good way to see what SWIG is generating :).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

Trying to run the tests in my system failed with the following:




Comment at: lldb/CMakeLists.txt:60-68
+  execute_process(
+ COMMAND ${Lua_EXECUTABLE}
+ -e "for w in string.gmatch(package.cpath, ';?([^;]+);?') do \
+ if string.match(w, '%?%.so') then print(string.sub(w, 1, #w - 4)) 
break end end"
+ OUTPUT_VARIABLE LLDB_LUA_DEFAULT_INSTALL_PATH
+ OUTPUT_STRIP_TRAILING_WHITESPACE)
+  file(TO_CMAKE_PATH ${LLDB_LUA_DEFAULT_INSTALL_PATH} 
LLDB_LUA_DEFAULT_INSTALL_PATH)

This is broken if the user specifies another INSTALL_PREFIX, as it forces it to 
`/usr/local/lib/lua/5.3`.
I think a safe option is to always set to `lib/lua/5.3`, so install prefixes 
can be concatenated freely. 



Comment at: lldb/CMakeLists.txt:116
+if (LLDB_ENABLE_LUA)
+  set(lldb_lua_target_dir 
"${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_INSTALL_LIBDIR}/lua")
+  get_target_property(lldb_lua_bindings_dir swig_wrapper_lua BINARY_DIR)

I think we are missing the Framework build here.
LLDB_BUILD_FRAMEWORK is a Darwin specific variable, so you will most likely 
need one to test this.
I can help if you don't have access to a Darwin device.



Comment at: lldb/bindings/lua/lua-typemaps.swig:195-213
+%typemap(in) char ** {
+   if (lua_istable(L, $input)) {
+  size_t size = lua_rawlen(L, $input);
+  $1 = (char **)malloc((size + 1) * sizeof(char *));
+  int i = 0, j = 0;
+  while (i++ < size) {
+ lua_rawgeti(L, $input, i);

Add comment here saying that the raw calls will restrict the table to be a 
sequence of strings.

Please add it in other functions as well.



Comment at: lldb/bindings/lua/lua-typemaps.swig:219-221
+%typecheck(SWIG_TYPECHECK_STRING_ARRAY) char ** {
+   $1 = (lua_istable(L, $input) || lua_isnil(L, $input));
+}

This is not being generated by SWIG for some reason.

I think it's more readable to just have an else clause that raises an error on 
line 212.



Comment at: lldb/bindings/lua/lua-typemaps.swig:275
+  $2 = 0;
+   }
+}

Else clause here for raising an error for types that are not supported



Comment at: lldb/bindings/lua/lua-typemaps.swig:278
+
+%typemap(in) (uint64_t* array, size_t array_len),
+ (uint32_t* array, size_t array_len),

This bug was not caught by the tests, perhaps we need to expand it more?
This would certainly generate a NULL pointer de-reference as the argument would 
always be NULL, since it's replacing the above code with just `free()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108515: [lldb/lua] Force Lua version to be 5.3

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/cmake/modules/FindLuaAndSwig.cmake:12
   if (SWIG_FOUND)
-find_package(Lua 5.3)
+find_package(Lua 5.3 EXACT REQUIRED)
 if(LUA_FOUND AND SWIG_FOUND)

mstorsjo wrote:
> siger-young wrote:
> > mstorsjo wrote:
> > > This breaks building in setups where SWIG is available, but not Lua. 
> > > Previously this detected Lua and took it into use if both Lua and SWIG 
> > > were available, and if not , proceeded without them.
> > I think removing the "REQUIRED" flags might work. I will revert the broken 
> > commit first.
> Just removing the `REQUIRED` from here seems to fix my build. Or I could just 
> revert the patch to return to discussing how it should be done.
> 
> I'm doing either of them fairly soon in any case, to unbreak my build.
That's look OK


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108515

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104281: [lldb][docs] Add reference docs for Lua scripting

[Pedro Tammela via Phabricator via lldb-commits]

tammela accepted this revision.
tammela added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104281

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108515: [lldb/lua] Force Lua version to be 5.3

[Pedro Tammela via Phabricator via lldb-commits]

tammela accepted this revision.
tammela added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108515

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104281: [lldb][docs] Add reference docs for Lua scripting

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

I think we are getting there, just some more details




Comment at: lldb/docs/use/scripting-example.rst:822
+The complete code for the Dictionary program (with case-conversion bug), the
+DFS function and other Python script examples (tree_utils.py) used for this
+example are available below.





Comment at: lldb/docs/use/scripting-reference.rst:1
-Python Reference
-
+Scripting Reference
+===

I'm seeing some references to a "python script" when it should be really just 
"the following script" or something more neutral.

A quick way to solve this is to search for [Pp]ython and try to rewrite the 
phrase into something generic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104281

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela requested changes to this revision.
tammela added a comment.
This revision now requires changes to proceed.

Missing test cases!
Either a script that test it all, individual unit tests or a combination of 
both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/bindings/lua/lua-typemaps.swig:198
+  size_t size = lua_rawlen(L, $input);
+  $1 = (char **)malloc((size + 1) * sizeof(char *));
+  int i = 0, j = 0;

This seems it could leak.
Are you sure it doesn't? If so add a comment here explaining where it's freed.



Comment at: lldb/bindings/lua/lua-typemaps.swig:221
+   luaL_Stream *p = (luaL_Stream *)luaL_checkudata(L, $input, LUA_FILEHANDLE);
+   FileSP file_sp;
+   file_sp = std::make_shared(p->f, false);







Comment at: lldb/bindings/lua/lua-typemaps.swig:222
+   FileSP file_sp;
+   file_sp = std::make_shared(p->f, false);
+   if (!file_sp->IsValid())





Comment at: lldb/bindings/lua/lua-typemaps.swig:229
+%typecheck(SWIG_TYPECHECK_POINTER) lldb::FileSP {
+   $1 = lua_isuserdata(L, $input) ? 1 : 0;
+}





Comment at: lldb/bindings/lua/lua-typemaps.swig:249
+
+%typemap(in) (uint64_t* array, size_t array_len),
+ (uint32_t* array, size_t array_len),

The raw variants here are overkill, you can the use normal ones



Comment at: lldb/bindings/lua/lua-typemaps.swig:256
+  $2 = lua_rawlen(L, $input);
+  $1 = ($1_ltype)malloc(($2) * sizeof($*1_type));
+  int i = 0, j = 0;

Leaking?



Comment at: lldb/bindings/lua/lua-typemaps.swig:266
+  $2 = 0;
+   }
+}

else Lua error?



Comment at: lldb/bindings/lua/lua-typemaps.swig:284
+//===--===//
\ No newline at end of file


Please fix this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104281: [lldb][docs] Add reference docs for Lua scripting

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/docs/use/scripting-reference.rst:572
+Create a new lldb command
+-
 

So I'm guessing some sections are still missing some updates because the Lua 
functionality is not there yet right?

If so, I would add some note that this is not currently supported in Lua.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104281

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105034: [lldb/lua] Add scripted watchpoints for Lua

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe81ba283131c: [lldb/lua] Add scripted watchpoints for Lua 
(authored by Siger Yang sigerye...@gmail.com, committed by tammela).

Changed prior to commit:
  https://reviews.llvm.org/D105034?vs=355429=357000#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105034

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -30,6 +30,11 @@
   return false;
 }
 
+extern "C" llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp) {
+  return false;
+}
+
 #if _MSC_VER
 #pragma warning (pop)
 #endif
Index: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -1,9 +1,33 @@
 # REQUIRES: lua
 # XFAIL: system-netbsd
-# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t
+# RUN: echo "int main() { int val = 1; val++; return 0; }" | %clang_host -x c - -g -o %t
 # RUN: %lldb -s %s --script-language lua %t 2>&1 | FileCheck %s
 b main
 r
-watchpoint set expr 0x0
+watchpoint set variable val
 watchpoint command add -s lua
-# CHECK: error: This script interpreter does not support watchpoint callbacks
+print("val=" .. tostring(frame:FindVariable("val"):GetValue()))
+quit
+c
+# CHECK: val=1
+# CHECK: val=2
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set variable val
+watchpoint modify 1 -c "(val == 1)"
+watchpoint command add -s lua
+print("conditional watchpoint")
+wp:SetEnabled(false)
+quit
+c
+# CHECK-COUNT-1: conditional watchpoint
+# CHECK-NOT: conditional watchpoint
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set expr 0x00
+watchpoint command add -s lua
+print("never triggers")
+quit
+c
+# CHECK-NOT: never triggers
+# CHECK: Process {{[0-9]+}} exited
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -11,6 +11,7 @@
 
 #include 
 
+#include "lldb/Breakpoint/WatchpointOptions.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Utility/Status.h"
@@ -63,6 +64,10 @@
  lldb::user_id_t break_id,
  lldb::user_id_t break_loc_id);
 
+  static bool WatchpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t watch_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -77,9 +82,16 @@
   std::vector> _options_vec,
   CommandReturnObject ) override;
 
+  void
+  CollectDataForWatchpointCommandCallback(WatchpointOptions *wp_options,
+  CommandReturnObject ) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions _options,
   const char *command_body_text) override;
 
+  void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
+const char *command_body_text) override;
+
   Status SetBreakpointCommandCallbackFunction(
   BreakpointOptions _options, const char *function_name,
   StructuredData::ObjectSP extra_args_sp) override;
@@ -91,6 +103,10 @@
   Status RegisterBreakpointCallback(BreakpointOptions _options,
 const char *command_body_text,
 StructuredData::ObjectSP extra_args_sp);
+
+  Status RegisterWatchpointCallback(WatchpointOptions *wp_options,
+const char *command_body_text,
+StructuredData::ObjectSP extra_args_sp);
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp

[Lldb-commits] [PATCH] D105034: [lldb/lua] Add scripted watchpoints for Lua

[Pedro Tammela via Phabricator via lldb-commits]

tammela accepted this revision.
tammela added a comment.
This revision is now accepted and ready to land.

I tested locally and it seems fine!

Thanks.




Comment at: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test:23
+c
+# CHECK: conditional watchpoint
+# CHECK-NOT: conditional watchpoint

nit:

I think it makes more readable to change this to `CHECK-COUNT-1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105034

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105034: [lldb/lua] Add scripted watchpoints for Lua

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

Clang format complained a bit, please fix those.




Comment at: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test:15
+# CHECK: val=nil
+# CHECK: Process {{[0-9]+}} exited

More tests cases would be nice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105034

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG532e4203c5be: [lldb/Lua] add support for Lua function 
breakpoint (authored by tammela).

Changed prior to commit:
  https://reviews.llvm.org/D93649?vs=318775=319148#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-typemaps.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -24,10 +24,9 @@
 #pragma warning (disable : 4190)
 #endif
 
-extern "C" llvm::Expected
-LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
-  lldb::StackFrameSP stop_frame_sp,
-  lldb::BreakpointLocationSP bp_loc_sp) {
+extern "C" llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp,
+lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl) {
   return false;
 }
 
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -0,0 +1,23 @@
+# 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: 
+# 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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -9,6 +9,7 @@
 #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"
@@ -22,6 +23,11 @@
 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 );
@@ -72,9 +78,17 @@
   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 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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -264,8 +264,9 @@
   debugger.GetScriptInterpreter(true, eScriptLanguageLua));
   Lua  = lua_interpreter->GetLua();
 
-  llvm::Expected BoolOrErr =
-  lua.CallBreakpointCallback(baton, stop_frame_sp, bp_loc_sp);
+  CommandDataLua *bp_option_data = static_cast(baton);
+  llvm::Expected 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;
@@ -283,10 +284,25 @@
 

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

@JDevlieghere Addressed all your comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 318775.
tammela added a comment.

Rebasing and unwrapping the SBStructuredData test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -24,10 +24,9 @@
 #pragma warning (disable : 4190)
 #endif
 
-extern "C" llvm::Expected
-LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
-  lldb::StackFrameSP stop_frame_sp,
-  lldb::BreakpointLocationSP bp_loc_sp) {
+extern "C" llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp,
+lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl) {
   return false;
 }
 
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -0,0 +1,23 @@
+# 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: 
+# 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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -9,6 +9,7 @@
 #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"
@@ -22,6 +23,11 @@
 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 );
@@ -72,9 +78,17 @@
   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 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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -264,8 +264,9 @@
   debugger.GetScriptInterpreter(true, eScriptLanguageLua));
   Lua  = lua_interpreter->GetLua();
 
-  llvm::Expected BoolOrErr =
-  lua.CallBreakpointCallback(baton, stop_frame_sp, bp_loc_sp);
+  CommandDataLua *bp_option_data = static_cast(baton);
+  llvm::Expected 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;
@@ -283,10 +284,25 @@
   m_debugger.RunIOHandlerAsync(io_handler_sp);
 }
 
+Status ScriptInterpreterLua::SetBreakpointCommandCallbackFunction(
+BreakpointOptions *bp_options, const char *function_name,
+StructuredData::ObjectSP extra_args_sp) 

[Lldb-commits] [PATCH] D94937: [lldb/Lua] add initial Lua typemaps

[Pedro Tammela via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5997e8987f68: [lldb/Lua] add initial Lua typemaps (authored 
by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/lua/lua-typemaps.swig

Index: lldb/bindings/lua/lua-typemaps.swig
===
--- lldb/bindings/lua/lua-typemaps.swig
+++ lldb/bindings/lua/lua-typemaps.swig
@@ -1 +1,96 @@
 %include 
+
+// FIXME: We need to port more typemaps from Python
+
+//===--===//
+
+// In 5.3 and beyond the VM supports integers, so we need to remap
+// SWIG's internal handling of integers.
+
+
+%define LLDB_NUMBER_TYPEMAP(TYPE)
+
+// Primitive integer mapping
+%typemap(in,checkfn="lua_isinteger") TYPE
+%{ $1 = (TYPE)lua_tointeger(L, $input); %}
+%typemap(in,checkfn="lua_isinteger") const TYPE&($basetype temp)
+%{ temp=($basetype)lua_tointeger(L,$input); $1=%}
+%typemap(out) TYPE
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+%typemap(out) const TYPE&
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+
+// Pointer and reference mapping
+%typemap(in,checkfn="lua_isinteger") TYPE *INPUT($*ltype temp), TYPE ($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+%typemap(in, numinputs=0) TYPE *OUTPUT ($*ltype temp)
+%{ $1 =  %}
+%typemap(argout) TYPE *OUTPUT
+%{  lua_pushinteger(L, (lua_Integer) *$1); SWIG_arg++;%}
+%typemap(in) TYPE *INOUT = TYPE *INPUT;
+%typemap(argout) TYPE *INOUT = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *OUTPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *INPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+%typemap(in,checkfn="lua_isinteger") const TYPE *INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+
+%enddef // LLDB_NUMBER_TYPEMAP
+
+LLDB_NUMBER_TYPEMAP(unsigned char);
+LLDB_NUMBER_TYPEMAP(signed char);
+LLDB_NUMBER_TYPEMAP(short);
+LLDB_NUMBER_TYPEMAP(unsigned short);
+LLDB_NUMBER_TYPEMAP(signed short);
+LLDB_NUMBER_TYPEMAP(int);
+LLDB_NUMBER_TYPEMAP(unsigned int);
+LLDB_NUMBER_TYPEMAP(signed int);
+LLDB_NUMBER_TYPEMAP(long);
+LLDB_NUMBER_TYPEMAP(unsigned long);
+LLDB_NUMBER_TYPEMAP(signed long);
+LLDB_NUMBER_TYPEMAP(long long);
+LLDB_NUMBER_TYPEMAP(unsigned long long);
+LLDB_NUMBER_TYPEMAP(signed long long);
+
+%apply unsigned long { size_t };
+%apply const unsigned long & { const size_t & };
+%apply long { ssize_t };
+%apply const long & { const ssize_t & };
+
+//===--===//
+
+// 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");
+   }
+   $1 = (char *) malloc($2);
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+// Return the char buffer.  Discarding any previous return result
+%typemap(argout) (char *dst, size_t dst_len) {
+   lua_pop(L, 1); // Blow away the previous result
+   if ($result == 0) {
+  lua_pushliteral(L, "");
+   } else {
+  lua_pushlstring(L, (const char *)$1, $result);
+   }
+   free($1);
+   // SWIG_arg was already incremented
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+//===--===//
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb/Lua] add initial Lua typemaps

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 318300.
tammela added a comment.

Typos


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/lua/lua-typemaps.swig

Index: lldb/bindings/lua/lua-typemaps.swig
===
--- lldb/bindings/lua/lua-typemaps.swig
+++ lldb/bindings/lua/lua-typemaps.swig
@@ -1 +1,96 @@
 %include 
+
+// FIXME: We need to port more typemaps from Python
+
+//===--===//
+
+// In 5.3 and beyond the VM supports integers, so we need to remap
+// SWIG's internal handling of integers.
+
+
+%define LLDB_NUMBER_TYPEMAP(TYPE)
+
+// Primitive integer mapping
+%typemap(in,checkfn="lua_isinteger") TYPE
+%{ $1 = (TYPE)lua_tointeger(L, $input); %}
+%typemap(in,checkfn="lua_isinteger") const TYPE&($basetype temp)
+%{ temp=($basetype)lua_tointeger(L,$input); $1=%}
+%typemap(out) TYPE
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+%typemap(out) const TYPE&
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+
+// Pointer and reference mapping
+%typemap(in,checkfn="lua_isinteger") TYPE *INPUT($*ltype temp), TYPE ($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+%typemap(in, numinputs=0) TYPE *OUTPUT ($*ltype temp)
+%{ $1 =  %}
+%typemap(argout) TYPE *OUTPUT
+%{  lua_pushinteger(L, (lua_Integer) *$1); SWIG_arg++;%}
+%typemap(in) TYPE *INOUT = TYPE *INPUT;
+%typemap(argout) TYPE *INOUT = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *OUTPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *INPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+%typemap(in,checkfn="lua_isinteger") const TYPE *INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+
+%enddef // LLDB_NUMBER_TYPEMAP
+
+LLDB_NUMBER_TYPEMAP(unsigned char);
+LLDB_NUMBER_TYPEMAP(signed char);
+LLDB_NUMBER_TYPEMAP(short);
+LLDB_NUMBER_TYPEMAP(unsigned short);
+LLDB_NUMBER_TYPEMAP(signed short);
+LLDB_NUMBER_TYPEMAP(int);
+LLDB_NUMBER_TYPEMAP(unsigned int);
+LLDB_NUMBER_TYPEMAP(signed int);
+LLDB_NUMBER_TYPEMAP(long);
+LLDB_NUMBER_TYPEMAP(unsigned long);
+LLDB_NUMBER_TYPEMAP(signed long);
+LLDB_NUMBER_TYPEMAP(long long);
+LLDB_NUMBER_TYPEMAP(unsigned long long);
+LLDB_NUMBER_TYPEMAP(signed long long);
+
+%apply unsigned long { size_t };
+%apply const unsigned long & { const size_t & };
+%apply long { ssize_t };
+%apply const long & { const ssize_t & };
+
+//===--===//
+
+// 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");
+   }
+   $1 = (char *) malloc($2);
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+// Return the char buffer.  Discarding any previous return result
+%typemap(argout) (char *dst, size_t dst_len) {
+   lua_pop(L, 1); // Blow away the previous result
+   if ($result == 0) {
+  lua_pushliteral(L, "");
+   } else {
+  lua_pushlstring(L, (const char *)$1, $result);
+   }
+   free($1);
+   // SWIG_arg was already incremented
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+//===--===//
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 318298.
tammela added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/lua/lua-typemaps.swig

Index: lldb/bindings/lua/lua-typemaps.swig
===
--- lldb/bindings/lua/lua-typemaps.swig
+++ lldb/bindings/lua/lua-typemaps.swig
@@ -1 +1,97 @@
 %include 
+
+// FIXME: We need to port more typemaps from Python
+
+//===--===//
+
+// In 5.3 and beyond the VM supports integers, so we need to remap
+// SWIG's internal handling to integers.
+
+
+%define LLDB_NUMBER_TYPEMAP(TYPE)
+
+// Primitive integer mapping
+%typemap(in,checkfn="lua_isinteger") TYPE
+%{ $1 = (TYPE)lua_tointeger(L, $input); %}
+%typemap(in,checkfn="lua_isinteger") const TYPE&($basetype temp)
+%{ temp=($basetype)lua_tointeger(L,$input); $1=%}
+%typemap(out) TYPE
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+%typemap(out) const TYPE&
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+
+// Pointer and reference mapping
+%typemap(in,checkfn="lua_isinteger") TYPE *INPUT($*ltype temp), TYPE ($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+%typemap(in, numinputs=0) TYPE *OUTPUT ($*ltype temp)
+%{ $1 =  %}
+%typemap(argout) TYPE *OUTPUT
+%{  lua_pushinteger(L, (lua_Integer) *$1); SWIG_arg++;%}
+%typemap(in) TYPE *INOUT = TYPE *INPUT;
+%typemap(argout) TYPE *INOUT = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *OUTPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *INPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+// const version (the $*ltype is the basic number without ptr or const's)
+%typemap(in,checkfn="lua_isinteger") const TYPE *INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+
+%enddef // LLDB_NUMBER_TYPEMAP
+
+LLDB_NUMBER_TYPEMAP(unsigned char);
+LLDB_NUMBER_TYPEMAP(signed char);
+LLDB_NUMBER_TYPEMAP(short);
+LLDB_NUMBER_TYPEMAP(unsigned short);
+LLDB_NUMBER_TYPEMAP(signed short);
+LLDB_NUMBER_TYPEMAP(int);
+LLDB_NUMBER_TYPEMAP(unsigned int);
+LLDB_NUMBER_TYPEMAP(signed int);
+LLDB_NUMBER_TYPEMAP(long);
+LLDB_NUMBER_TYPEMAP(unsigned long);
+LLDB_NUMBER_TYPEMAP(signed long);
+LLDB_NUMBER_TYPEMAP(long long);
+LLDB_NUMBER_TYPEMAP(unsigned long long);
+LLDB_NUMBER_TYPEMAP(signed long long);
+
+%apply unsigned long { size_t };
+%apply const unsigned long & { const size_t & };
+%apply long { ssize_t };
+%apply const long & { const ssize_t & };
+
+//===--===//
+
+/* 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");
+   }
+   $1 = (char *) malloc($2);
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+// Return the char buffer.  Discarding any previous return result
+%typemap(argout) (char *dst, size_t dst_len) {
+   lua_pop(L, 1); // Blow away the previous result
+   if ($result == 0) {
+  lua_pushliteral(L, "");
+   } else {
+  lua_pushlstring(L, (const char *)$1, $result);
+   }
+   free($1);
+   // SWIG_arg was already incremented
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+//===--===//
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D94937#2511222 , @JDevlieghere 
wrote:

> I replied before I actually tried to understand what your'e trying to 
> achieve, and it's still not entirely clear to me. I think what you need is 
> something similar like `python-typemaps.swig` which helps swig understand 
> `char**` types and such.
>
> For context, originally SWIG wasn't great at handling `std::string`s (which 
> has since been fixed) and the designers of the SB API were worried about the 
> ABI stability of the `std::string` class. None of these are concerns anymore 
> today, but the ABI being stable means we're stuck with it for now. If we ever 
> do an SB API v2, this is definitely one of the things we'd fix, but currently 
> there are no plans for that.

That's exactly what I need, thanks. I will adjust accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D94937#2505676 , @JDevlieghere 
wrote:

> We guarantee that the SB API is ABI stable so you cannot change the signature 
> of existing functions. Would an overload do the trick?

If overloaded with `const char *GetStringValue(const char *fail_value = 
nullptr)`, SWIG emits a warning when building the Python wrapper saying that 
the method is shadowed and doesn't emit it.
`const char *GetStringValue()` works, however it doesn't follow the API 
contract.
Would adding a new method `GetConstStringValue()` also be acceptable?

I'm out of ideas to solve this. This method is very Python oriented, as the 
SWIG wrapper understands the {in,out} documentation. This doesn't happen for 
the Lua wrapper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D94937#2505676 , @JDevlieghere 
wrote:

> We guarantee that the SB API is ABI stable so you cannot change the signature 
> of existing functions. Would an overload do the trick?

Hmmm, that's tricky. I will see if I can come up with something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 317439.
tammela added a comment.

Removing spurious line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/interface/SBStructuredData.i
  lldb/examples/python/in_call_stack.py
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/source/API/SBStructuredData.cpp
  lldb/test/API/commands/platform/basic/TestPlatformPython.py
  lldb/test/API/commands/target/stop-hooks/stop_hook.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/bktptcmd.py
  lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
  
lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
  lldb/test/API/functionalities/step_scripted/Steps.py
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py

Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -100,7 +100,7 @@
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
 
 # Check API returning 'string' value
-output = string_struct.GetStringValue(25)
+output = string_struct.GetStringValue()
 if not "STRING" in output:
 self.fail("wrong output: " + output)
 
@@ -131,7 +131,7 @@
 
 # Calling wrong API on a SBStructuredData
 # (e.g. getting a string value from an integer type structure)
-output = int_struct.GetStringValue(25)
+output = int_struct.GetStringValue()
 if output:
 self.fail(
 "Valid string " +
@@ -192,7 +192,7 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "23":
 self.fail("wrong output: " + str(output))
 
@@ -201,6 +201,6 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "arr":
 self.fail("wrong output: " + str(output))
Index: lldb/test/API/functionalities/step_scripted/Steps.py
===
--- lldb/test/API/functionalities/step_scripted/Steps.py
+++ lldb/test/API/functionalities/step_scripted/Steps.py
@@ -45,7 +45,7 @@
 
 if not func_entry.IsValid():
 print("Did not get a valid entry for variable_name")
-func_name = func_entry.GetStringValue(100)
+func_name = func_entry.GetStringValue()
 
 self.value = self.frame.FindVariable(func_name)
 if self.value.GetError().Fail():
@@ -88,7 +88,7 @@
 
 def __init__(self, thread_plan, args_data, dict):
 self.thread_plan = thread_plan
-self.key = args_data.GetValueForKey("token").GetStringValue(1000)
+self.key = args_data.GetValueForKey("token").GetStringValue()
 
 def should_stop(self, event):
 self.thread_plan.SetPlanComplete(True)
Index: lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
===
--- lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
+++ lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
@@ -407,7 +407,7 @@
 
 for idx in range(0, orig_keys.GetSize()):
 key = orig_keys.GetStringAtIndex(idx)
-copy_value = copy_extra_args.GetValueForKey(key).GetStringValue(100)
+copy_value = copy_extra_args.GetValueForKey(key).GetStringValue()
 
 if key == "first_arg":
 self.assertEqual(copy_value, "first_value")
Index: lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
===
--- lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
+++ lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
@@ -15,7 +15,7 @@
   sym_name = "not_a_real_function_name"
   sym_item = self.extra_args.GetValueForKey("symbol")
   if sym_item.IsValid():
-  sym_name = sym_item.GetStringValue(1000)
+  sym_name = sym_item.GetStringValue()
   else:
   print("Didn't have 

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
tammela requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch changes the GetStringValue method on the SBStructuredData
class to return a `const char *`.

This change allows the use of the method on the Lua interpreter.
The auto-generated Lua binding can't handle `char *` arguments
properly. That's because it forces the method call with a `char *` like
Lua object.

This means that to retrieve the string value an operation would need to write 
to an
internal Lua string, instead of using the Lua C API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/interface/SBStructuredData.i
  lldb/examples/python/in_call_stack.py
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/source/API/SBStructuredData.cpp
  lldb/test/API/commands/platform/basic/TestPlatformPython.py
  lldb/test/API/commands/target/stop-hooks/stop_hook.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/bktptcmd.py
  lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
  
lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
  lldb/test/API/functionalities/step_scripted/Steps.py
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -5,6 +5,7 @@
 script
 function abc(a, b, c, ...)
 print(c)
+if c then print(c:GetValueForKey("foo"):GetStringValue()) end
 end
 quit
 breakpoint command add -s lua -F abc
Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -100,7 +100,7 @@
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
 
 # Check API returning 'string' value
-output = string_struct.GetStringValue(25)
+output = string_struct.GetStringValue()
 if not "STRING" in output:
 self.fail("wrong output: " + output)
 
@@ -131,7 +131,7 @@
 
 # Calling wrong API on a SBStructuredData
 # (e.g. getting a string value from an integer type structure)
-output = int_struct.GetStringValue(25)
+output = int_struct.GetStringValue()
 if output:
 self.fail(
 "Valid string " +
@@ -192,7 +192,7 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "23":
 self.fail("wrong output: " + str(output))
 
@@ -201,6 +201,6 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "arr":
 self.fail("wrong output: " + str(output))
Index: lldb/test/API/functionalities/step_scripted/Steps.py
===
--- lldb/test/API/functionalities/step_scripted/Steps.py
+++ lldb/test/API/functionalities/step_scripted/Steps.py
@@ -45,7 +45,7 @@
 
 if not func_entry.IsValid():
 print("Did not get a valid entry for variable_name")
-func_name = func_entry.GetStringValue(100)
+func_name = func_entry.GetStringValue()
 
 self.value = self.frame.FindVariable(func_name)
 if self.value.GetError().Fail():
@@ -88,7 +88,7 @@
 
 def __init__(self, thread_plan, args_data, dict):
 self.thread_plan = thread_plan
-self.key = args_data.GetValueForKey("token").GetStringValue(1000)
+self.key = args_data.GetValueForKey("token").GetStringValue()
 
 def should_stop(self, event):
 self.thread_plan.SetPlanComplete(True)
Index: lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
===
--- lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
+++ 

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/bindings/lua/lua-wrapper.swig:25-29
+   auto extra_args = [&]() -> llvm::Optional {
+  if (extra_args_impl == nullptr)
+ return {};
+  return lldb::SBStructuredData(extra_args_impl);
+   } ();

JDevlieghere wrote:
> I don't think you need the lambda. I used `unique_ptr` but `llvm:Optional` 
> would work as well. 
`{}` is not valid on the ternary operator, but I think the version suits your 
suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 317395.
tammela added a comment.

Addressing comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -24,10 +24,9 @@
 #pragma warning (disable : 4190)
 #endif
 
-extern "C" llvm::Expected
-LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
-  lldb::StackFrameSP stop_frame_sp,
-  lldb::BreakpointLocationSP bp_loc_sp) {
+extern "C" llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp,
+lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl) {
   return false;
 }
 
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -0,0 +1,21 @@
+# 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)
+end
+quit
+breakpoint command add -s lua -F abc
+r
+# CHECK: nil
+breakpoint command add -s lua -F abc -k foo -v 123
+r
+# CHECK: 
+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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -9,6 +9,7 @@
 #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"
@@ -22,6 +23,11 @@
 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 );
@@ -72,9 +78,17 @@
   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 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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -264,8 +264,9 @@
   debugger.GetScriptInterpreter(true, eScriptLanguageLua));
   Lua  = lua_interpreter->GetLua();
 
-  llvm::Expected BoolOrErr =
-  lua.CallBreakpointCallback(baton, stop_frame_sp, bp_loc_sp);
+  CommandDataLua *bp_option_data = static_cast(baton);
+  llvm::Expected 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;
@@ -283,10 +284,25 @@
   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, 

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: 
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test:15
+r
+# CHECK: 
+breakpoint command add -s lua -o "abc(frame, bp_loc, ...)"

JDevlieghere wrote:
> Can we unpack the SBStructuredData and check for `foo` or `123` in the output?
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 = ""
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd853bd7a4e86: [lldb/Lua] add support for multiline scripted 
breakpoints (authored by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
  lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
@@ -0,0 +1,15 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+script
+do
+local a = 123
+print(a)
+end
+# CHECK: 123
+str = 'hello there!'
+function callme()
+print(str)
+end
+callme()
+# CHECK: hello there!
+# CHECK-NOT: error
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
@@ -1,5 +1,13 @@
 # REQUIRES: lua
-# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+# 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
 breakpoint command add -s lua
-# CHECK: error: This script interpreter does not support breakpoint callbacks
+local a = 123
+print(a)
+quit
+run
+# CHECK: 123
+breakpoint command add -s lua
+789_nil
+# CHECK: unexpected symbol near '789'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -65,6 +65,10 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  void CollectDataForBreakpointCommandCallback(
+  std::vector _options_vec,
+  CommandReturnObject ) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *command_body_text) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -17,23 +17,33 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatAdapters.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(ScriptInterpreterLua)
 
+enum ActiveIOHandler {
+  eIOHandlerNone,
+  eIOHandlerBreakpoint,
+  eIOHandlerWatchpoint
+};
+
 class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 public IOHandlerEditline {
 public:
   IOHandlerLuaInterpreter(Debugger ,
-  ScriptInterpreterLua _interpreter)
+  ScriptInterpreterLua _interpreter,
+  ActiveIOHandler active_io_handler = eIOHandlerNone)
   : IOHandlerEditline(debugger, IOHandler::Type::LuaInterpreter, "lua",
   ">>> ", "..> ", true, debugger.GetUseColor(), 0,
   *this, nullptr),
-m_script_interpreter(script_interpreter) {
+m_script_interpreter(script_interpreter),
+m_active_io_handler(active_io_handler) {
 llvm::cantFail(m_script_interpreter.GetLua().ChangeIO(
 debugger.GetOutputFile().GetStream(),
 debugger.GetErrorFile().GetStream()));
@@ -44,20 +54,79 @@
 llvm::cantFail(m_script_interpreter.LeaveSession());
   }
 
-  void IOHandlerInputComplete(IOHandler _handler,
-  std::string ) override {
-if (llvm::StringRef(data).rtrim() == "quit") {
-  io_handler.SetIsDone(true);
+  void IOHandlerActivated(IOHandler _handler, bool interactive) override {
+const char *instructions = nullptr;
+switch (m_active_io_handler) {
+case eIOHandlerNone:
+case eIOHandlerWatchpoint:
+  break;
+case eIOHandlerBreakpoint:
+  instructions = "Enter your Lua command(s). Type 'quit' to end.\n"
+ "The commands are compiled as the body of the following "
+ "Lua function\n"
+ "function (frame, 

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h:39
   llvm::Error LoadModule(llvm::StringRef filename);
+  llvm::Error LoadBuffer(llvm::StringRef buffer, bool pop_result = true);
   llvm::Error ChangeIO(FILE *out, FILE *err);

labath wrote:
> tammela wrote:
> > labath wrote:
> > > This default value does not seem particularly useful. In fact, I am not 
> > > sure if this argument should even exist at all. The usage in 
> > > `IOHandlerIsInputComplete` is sufficiently unorthodox that it might be a 
> > > good idea to draw attention to the fact that something funky is happening 
> > > via an explicit pop (and maybe a comment).
> > We can't explicitly pop, as in calling `lua_pop()`, from 
> > `ScriptInterpreterLua.cpp` because the `lua_State` is not exposed. Are you 
> > suggesting to add a method that wraps `lua_pop()` as well?
> Good question. I didn't realize this aspect of the problem. I think the 
> answer to this depends on the direction we want the Lua class to go in. Back 
> when it was first introduced, my idea was for it to be some sort of a c++ 
> wrapper for the lua (C) api. It would offset a nicer interface to interact 
> with lua, but would otherwise be (more-or-less) lldb-agnostic (kind of like 
> our PythonDataObjects). In that world, it would make sense to expose the lua 
> stack and the pop function (among other things).
> 
> However, I don't think that's really how things have worked out. The current 
> Lua interface is much more high-level and much-more debugger-oriented than I 
> originally imagined. Among functions like `RegisterBreakpointCallback` and 
> `ChangeIO`, a function like `Pop` seems misplaced. I'm not sure this is a 
> good position to be in (there's still a need to find a home for the c++ 
> wrapper-like functions, for example to handle exceptions), but if we want to 
> go down that path, then a pop function is not the right answer. However, I 
> think the same applies to the LoadBuffer function, as it cannot be useful 
> (beyond this syntax-check use case) without exposing the lua stack, 
> implicitly or explicitly.  In this world it would be better to make the 
> function even more high-level, and have something like `CheckSyntax(buffer)` 
> (which does pretty much what LoadBuffer does, but it always pops the result). 
> Internally it can be implemented as a call to LoadBuffer, but this function 
> would now be a private implementation detail instead of a part of the 
> interface.
I think it's better to keep it as high level as possible. Exposing the 
`lua_State` spills the stack control to whom ever has access to the Lua 
interpreter, which might be undesirable and cumbersome to keep track of.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 313870.
tammela added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
  lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
@@ -0,0 +1,15 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+script
+do
+local a = 123
+print(a)
+end
+# CHECK: 123
+str = 'hello there!'
+function callme()
+print(str)
+end
+callme()
+# CHECK: hello there!
+# CHECK-NOT: error
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
@@ -1,5 +1,13 @@
 # REQUIRES: lua
-# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+# 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
 breakpoint command add -s lua
-# CHECK: error: This script interpreter does not support breakpoint callbacks
+local a = 123
+print(a)
+quit
+run
+# CHECK: 123
+breakpoint command add -s lua
+789_nil
+# CHECK: unexpected symbol near '789'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -65,6 +65,10 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  void CollectDataForBreakpointCommandCallback(
+  std::vector _options_vec,
+  CommandReturnObject ) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *command_body_text) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -17,23 +17,33 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatAdapters.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(ScriptInterpreterLua)
 
+enum ActiveIOHandler {
+  eIOHandlerNone,
+  eIOHandlerBreakpoint,
+  eIOHandlerWatchpoint
+};
+
 class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 public IOHandlerEditline {
 public:
   IOHandlerLuaInterpreter(Debugger ,
-  ScriptInterpreterLua _interpreter)
+  ScriptInterpreterLua _interpreter,
+  ActiveIOHandler active_io_handler = eIOHandlerNone)
   : IOHandlerEditline(debugger, IOHandler::Type::LuaInterpreter, "lua",
   ">>> ", "..> ", true, debugger.GetUseColor(), 0,
   *this, nullptr),
-m_script_interpreter(script_interpreter) {
+m_script_interpreter(script_interpreter),
+m_active_io_handler(active_io_handler) {
 llvm::cantFail(m_script_interpreter.GetLua().ChangeIO(
 debugger.GetOutputFile().GetStream(),
 debugger.GetErrorFile().GetStream()));
@@ -44,20 +54,79 @@
 llvm::cantFail(m_script_interpreter.LeaveSession());
   }
 
-  void IOHandlerInputComplete(IOHandler _handler,
-  std::string ) override {
-if (llvm::StringRef(data).rtrim() == "quit") {
-  io_handler.SetIsDone(true);
+  void IOHandlerActivated(IOHandler _handler, bool interactive) override {
+const char *instructions = nullptr;
+switch (m_active_io_handler) {
+case eIOHandlerNone:
+case eIOHandlerWatchpoint:
+  break;
+case eIOHandlerBreakpoint:
+  instructions = "Enter your Lua command(s). Type 'quit' to end.\n"
+ "The commands are compiled as the body of the following "
+ "Lua function\n"
+ "function (frame, bp_loc, ...) end\n";
+  SetPrompt(llvm::StringRef("..> "));
+  break;
+}
+if (instructions == nullptr)
   return;
+if 

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h:39
   llvm::Error LoadModule(llvm::StringRef filename);
+  llvm::Error LoadBuffer(llvm::StringRef buffer, bool pop_result = true);
   llvm::Error ChangeIO(FILE *out, FILE *err);

labath wrote:
> This default value does not seem particularly useful. In fact, I am not sure 
> if this argument should even exist at all. The usage in 
> `IOHandlerIsInputComplete` is sufficiently unorthodox that it might be a good 
> idea to draw attention to the fact that something funky is happening via an 
> explicit pop (and maybe a comment).
We can't explicitly pop, as in calling `lua_pop()`, from 
`ScriptInterpreterLua.cpp` because the `lua_State` is not exposed. Are you 
suggesting to add a method that wraps `lua_pop()` as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
tammela requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93649

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -24,10 +24,9 @@
 #pragma warning (disable : 4190)
 #endif
 
-extern "C" llvm::Expected
-LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
-  lldb::StackFrameSP stop_frame_sp,
-  lldb::BreakpointLocationSP bp_loc_sp) {
+extern "C" llvm::Expected LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp,
+lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl) {
   return false;
 }
 
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -0,0 +1,21 @@
+# 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)
+end
+quit
+breakpoint command add -s lua -F abc
+r
+# CHECK: nil
+breakpoint command add -s lua -F abc -k foo -v 123
+r
+# CHECK: 
+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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -9,6 +9,7 @@
 #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"
@@ -22,6 +23,11 @@
 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 );
@@ -72,9 +78,17 @@
   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 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
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -265,8 +265,9 @@
   debugger.GetScriptInterpreter(true, eScriptLanguageLua));
   Lua  = lua_interpreter->GetLua();
 
-  llvm::Expected BoolOrErr =
-  lua.CallBreakpointCallback(baton, stop_frame_sp, bp_loc_sp);
+  CommandDataLua *bp_option_data = static_cast(baton);
+  llvm::Expected 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;
@@ 

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:62
+case eIOHandlerWatchpoint:
+  break;
+case eIOHandlerBreakpoint:

JDevlieghere wrote:
> Shouldn't this be the same for break- and watchpoints? 
Probably, we can address this when we add the support for watchpoints later


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 312942.
tammela added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93481

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
  lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
@@ -0,0 +1,15 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+script
+do
+local a = 123
+print(a)
+end
+# CHECK: 123
+str = 'hello there!'
+function callme()
+print(str)
+end
+callme()
+# CHECK: hello there!
+# CHECK-NOT: error
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
@@ -1,5 +1,13 @@
 # REQUIRES: lua
-# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+# 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
 breakpoint command add -s lua
-# CHECK: error: This script interpreter does not support breakpoint callbacks
+local a = 123
+print(a)
+quit
+run
+# CHECK: 123
+breakpoint command add -s lua
+789_nil
+# CHECK: unexpected symbol near '789'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -65,6 +65,10 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  void CollectDataForBreakpointCommandCallback(
+  std::vector _options_vec,
+  CommandReturnObject ) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *command_body_text) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -17,23 +17,33 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatAdapters.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(ScriptInterpreterLua)
 
+enum ActiveIOHandler {
+  eIOHandlerNone,
+  eIOHandlerBreakpoint,
+  eIOHandlerWatchpoint
+};
+
 class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 public IOHandlerEditline {
 public:
   IOHandlerLuaInterpreter(Debugger ,
-  ScriptInterpreterLua _interpreter)
+  ScriptInterpreterLua _interpreter,
+  ActiveIOHandler active_io_handler = eIOHandlerNone)
   : IOHandlerEditline(debugger, IOHandler::Type::LuaInterpreter, "lua",
   ">>> ", "..> ", true, debugger.GetUseColor(), 0,
   *this, nullptr),
-m_script_interpreter(script_interpreter) {
+m_script_interpreter(script_interpreter),
+m_active_io_handler(active_io_handler) {
 llvm::cantFail(m_script_interpreter.GetLua().ChangeIO(
 debugger.GetOutputFile().GetStream(),
 debugger.GetErrorFile().GetStream()));
@@ -44,20 +54,79 @@
 llvm::cantFail(m_script_interpreter.LeaveSession());
   }
 
-  void IOHandlerInputComplete(IOHandler _handler,
-  std::string ) override {
-if (llvm::StringRef(data).rtrim() == "quit") {
-  io_handler.SetIsDone(true);
+  void IOHandlerActivated(IOHandler _handler, bool interactive) override {
+const char *instructions = nullptr;
+switch (m_active_io_handler) {
+case eIOHandlerNone:
+case eIOHandlerWatchpoint:
+  break;
+case eIOHandlerBreakpoint:
+  instructions = "Enter your Lua command(s). Type 'quit' to end.\n"
+ "The commands are compiled as the body of the following "
+ "Lua function\n"
+ "function (frame, bp_loc, ...) end\n";
+  SetPrompt(llvm::StringRef("..> "));
+  break;
+}
+if (instructions == nullptr)
   return;
+if 

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
tammela requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

1 - Partial Statements

The interpreter loop runs every line it receives, so partial
Lua statements are not being handled properly. This is a problem for
multiline breakpoint scripts since the interpreter loop, for this
particular case, is just an abstraction to a partially parsed function
body declaration.

This patch addresses this issue and as a side effect improves the
general Lua interpreter loop as well. It's now possible to write partial
statements in the 'script' command.

Example:

  (lldb) script
  >>>   do
  ..>   local a = 123
  ..>   print(a)
  ..>   end
  123

The technique implemented is the same as the one employed by Lua's own REPL 
implementation.
Partial statements always errors out with the '' tag in the error
message.

2 - LoadBuffer in Lua.h

In order to support (1), we need an API for just loading string buffers. This
was not available as so far we only needed to run string buffers.

3 - Multiline scripted breakpoints

Finally, with all the base features implemented this feature is
straightforward. The interpreter loop behaves exactly the same, the
difference is that it will aggregate all Lua statements into the body of
the breakpoint function. An explicit 'quit' statement is needed to exit the
interpreter loop.

Example:

  (lldb) breakpoint command add -s lua
  Enter your Lua command(s). Type 'quit' to end.
  The commands are compiled as the body of the following Lua function
  function (frame, bp_loc, ...) end
  ..> print(456)
  ..> a = 123
  ..> quit


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93481

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
  lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
@@ -0,0 +1,15 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+script
+do
+local a = 123
+print(a)
+end
+# CHECK: 123
+str = 'hello there!'
+function callme()
+print(str)
+end
+callme()
+# CHECK: hello there!
+# CHECK-NOT: error
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
@@ -1,5 +1,13 @@
 # REQUIRES: lua
-# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+# 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
 breakpoint command add -s lua
-# CHECK: error: This script interpreter does not support breakpoint callbacks
+local a = 123
+print(a)
+quit
+run
+# CHECK: 123
+breakpoint command add -s lua
+789_nil
+# CHECK: unexpected symbol near '789'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -65,6 +65,10 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  void CollectDataForBreakpointCommandCallback(
+  std::vector _options_vec,
+  CommandReturnObject ) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *command_body_text) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -17,23 +17,33 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatAdapters.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(ScriptInterpreterLua)
 
+enum ActiveIOHandler {
+  eIOHandlerNone,
+  eIOHandlerBreakpoint,
+  eIOHandlerWatchpoint
+};
+
 class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 public IOHandlerEditline {
 public:
   IOHandlerLuaInterpreter(Debugger ,
-  ScriptInterpreterLua _interpreter)
+  ScriptInterpreterLua _interpreter,
+   

[Lldb-commits] [PATCH] D92729: [LLDB] fix error message for one-line breakpoint scripts

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG280ae10774ab: [LLDB] fix error message for one-line 
breakpoint scripts (authored by tammela).

Changed prior to commit:
  https://reviews.llvm.org/D92729?vs=309765=309858#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92729

Files:
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
  lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test


Index: lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
@@ -0,0 +1,7 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: %lldb -s %s --script-language python 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o "1234_foo"
+# CHECK: error: SyntaxError({{.*}})
Index: lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
@@ -0,0 +1,5 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o '1234_foo'
+# CHECK: error: {{.*}} unexpected symbol near '1234'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -86,7 +86,7 @@
   std::string func_str = llvm::formatv(fmt_str, body).str();
   if (luaL_dostring(m_lua_state, func_str.c_str()) != LUA_OK) {
 llvm::Error e = llvm::make_error(
-llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
+llvm::formatv("{0}", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());
 // Pop error message from the stack.
 lua_pop(m_lua_state, 2);
Index: lldb/source/Commands/CommandObjectBreakpointCommand.cpp
===
--- lldb/source/Commands/CommandObjectBreakpointCommand.cpp
+++ lldb/source/Commands/CommandObjectBreakpointCommand.cpp
@@ -414,22 +414,23 @@
   // to set or collect command callback.  Otherwise, call the methods
   // associated with this object.
   if (m_options.m_use_script_language) {
+Status error;
 ScriptInterpreter *script_interp = GetDebugger().GetScriptInterpreter(
 /*can_create=*/true, m_options.m_script_language);
 // Special handling for one-liner specified inline.
 if (m_options.m_use_one_liner) {
-  script_interp->SetBreakpointCommandCallback(
+  error = script_interp->SetBreakpointCommandCallback(
   m_bp_options_vec, m_options.m_one_liner.c_str());
 } else if (!m_func_options.GetName().empty()) {
-  Status error = script_interp->SetBreakpointCommandCallbackFunction(
+  error = script_interp->SetBreakpointCommandCallbackFunction(
   m_bp_options_vec, m_func_options.GetName().c_str(),
   m_func_options.GetStructuredData());
-  if (!error.Success())
-result.SetError(error);
 } else {
   script_interp->CollectDataForBreakpointCommandCallback(
   m_bp_options_vec, result);
 }
+if (!error.Success())
+  result.SetError(error);
   } else {
 // Special handling for one-liner specified inline.
 if (m_options.m_use_one_liner)


Index: lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
@@ -0,0 +1,7 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: %lldb -s %s --script-language python 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o "1234_foo"
+# CHECK: error: SyntaxError({{.*}})
Index: lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
@@ -0,0 +1,5 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o '1234_foo'
+# CHECK: error: {{.*}} unexpected symbol near '1234'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ 

[Lldb-commits] [PATCH] D92729: [LLDB] fix error message for one-line breakpoint scripts

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
tammela requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

LLDB is ignoring compilation errors for one-line breakpoint scripts.
This patch fixes the issues and now the error message of the
ScriptInterpreter is shown to the user.

I had to remove a new-line character for the Lua interpreter since it
was duplicated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92729

Files:
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
  lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test


Index: lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
@@ -0,0 +1,7 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language python 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o "1234_foo"
+# CHECK: error: SyntaxError({{.*}})
Index: lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
@@ -0,0 +1,5 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o '1234_foo'
+# CHECK: error: {{.*}} unexpected symbol near '1234'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -86,7 +86,7 @@
   std::string func_str = llvm::formatv(fmt_str, body).str();
   if (luaL_dostring(m_lua_state, func_str.c_str()) != LUA_OK) {
 llvm::Error e = llvm::make_error(
-llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
+llvm::formatv("{0}", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());
 // Pop error message from the stack.
 lua_pop(m_lua_state, 2);
Index: lldb/source/Commands/CommandObjectBreakpointCommand.cpp
===
--- lldb/source/Commands/CommandObjectBreakpointCommand.cpp
+++ lldb/source/Commands/CommandObjectBreakpointCommand.cpp
@@ -414,22 +414,23 @@
   // to set or collect command callback.  Otherwise, call the methods
   // associated with this object.
   if (m_options.m_use_script_language) {
+Status error;
 ScriptInterpreter *script_interp = GetDebugger().GetScriptInterpreter(
 /*can_create=*/true, m_options.m_script_language);
 // Special handling for one-liner specified inline.
 if (m_options.m_use_one_liner) {
-  script_interp->SetBreakpointCommandCallback(
+  error = script_interp->SetBreakpointCommandCallback(
   m_bp_options_vec, m_options.m_one_liner.c_str());
 } else if (!m_func_options.GetName().empty()) {
-  Status error = script_interp->SetBreakpointCommandCallbackFunction(
+  error = script_interp->SetBreakpointCommandCallbackFunction(
   m_bp_options_vec, m_func_options.GetName().c_str(),
   m_func_options.GetStructuredData());
-  if (!error.Success())
-result.SetError(error);
 } else {
   script_interp->CollectDataForBreakpointCommandCallback(
   m_bp_options_vec, result);
 }
+if (!error.Success())
+  result.SetError(error);
   } else {
 // Special handling for one-liner specified inline.
 if (m_options.m_use_one_liner)


Index: lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
@@ -0,0 +1,7 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language python 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o "1234_foo"
+# CHECK: error: SyntaxError({{.*}})
Index: lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
@@ -0,0 +1,5 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o '1234_foo'
+# CHECK: error: {{.*}} unexpected symbol near '1234'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ 

[Lldb-commits] [PATCH] D92249: [LLDB/Python] Fix segfault on Python scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd055e3a0eb4e: [LLDB/Python] Fix segfault on Python scripted 
entrypoints (authored by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92249

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test

Index: lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
@@ -0,0 +1,8 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o 'print(frame); return False'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Host/Config.h"
+#include "lldb/lldb-enumerations.h"
 
 #if LLDB_ENABLE_PYTHON
 
@@ -214,6 +215,12 @@
 LLDBSWIGPython_GetDynamicSetting(void *module, const char *setting,
  const lldb::TargetSP _sp);
 
+static ScriptInterpreterPythonImpl *GetPythonInterpreter(Debugger ) {
+  ScriptInterpreter *script_interpreter =
+  debugger.GetScriptInterpreter(true, lldb::eScriptLanguagePython);
+  return static_cast(script_interpreter);
+}
+
 static bool g_initialized = false;
 
 namespace {
@@ -1825,11 +1832,10 @@
 return {};
 
   Debugger  = thread_plan_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return {};
 
   void *ret_val;
@@ -1929,11 +1935,10 @@
 return StructuredData::GenericSP();
 
   Debugger  = bkpt_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::GenericSP();
 
   void *ret_val;
@@ -2003,11 +2008,10 @@
 return StructuredData::GenericSP();
   }
 
-  ScriptInterpreter *script_interpreter = m_debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(m_debugger);
 
-  if (!script_interpreter) {
+  if (!python_interpreter) {
 error.SetErrorString("No script interpreter for scripted stop-hook.");
 return StructuredData::GenericSP();
   }
@@ -2103,11 +2107,10 @@
 return StructuredData::ObjectSP();
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::ObjectSP();
 
   void *ret_val = nullptr;
@@ -2274,11 +2277,10 @@
 return true;
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {
@@ -2340,11 +2342,10 @@
 return true;
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92249: [LLDB/Python] Fix segfault on Python scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 308608.
tammela added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92249

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test

Index: lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
@@ -0,0 +1,8 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o 'print(frame); return False'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Host/Config.h"
+#include "lldb/lldb-enumerations.h"
 
 #if LLDB_ENABLE_PYTHON
 
@@ -214,6 +215,12 @@
 LLDBSWIGPython_GetDynamicSetting(void *module, const char *setting,
  const lldb::TargetSP _sp);
 
+static ScriptInterpreterPythonImpl *GetPythonInterpreter(Debugger ) {
+  ScriptInterpreter *script_interpreter =
+  debugger.GetScriptInterpreter(true, lldb::eScriptLanguagePython);
+  return static_cast(script_interpreter);
+}
+
 static bool g_initialized = false;
 
 namespace {
@@ -1825,11 +1832,10 @@
 return {};
 
   Debugger  = thread_plan_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return {};
 
   void *ret_val;
@@ -1929,11 +1935,10 @@
 return StructuredData::GenericSP();
 
   Debugger  = bkpt_sp->GetTarget().GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::GenericSP();
 
   void *ret_val;
@@ -2003,11 +2008,10 @@
 return StructuredData::GenericSP();
   }
 
-  ScriptInterpreter *script_interpreter = m_debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  static_cast(script_interpreter);
+  GetPythonInterpreter(m_debugger);
 
-  if (!script_interpreter) {
+  if (!python_interpreter) {
 error.SetErrorString("No script interpreter for scripted stop-hook.");
 return StructuredData::GenericSP();
   }
@@ -2103,11 +2107,10 @@
 return StructuredData::ObjectSP();
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return StructuredData::ObjectSP();
 
   void *ret_val = nullptr;
@@ -2274,11 +2277,10 @@
 return true;
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {
@@ -2340,11 +2342,10 @@
 return true;
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
   ScriptInterpreterPythonImpl *python_interpreter =
-  (ScriptInterpreterPythonImpl *)script_interpreter;
+  GetPythonInterpreter(debugger);
 
-  if (!script_interpreter)
+  if (!python_interpreter)
 return true;
 
   if (python_function_name && python_function_name[0]) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0d7406ae800: [LLDB/Lua] add support for one-liner 
breakpoint callback (authored by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -13,6 +13,30 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
+  lldb::StackFrameSP stop_frame_sp,
+  lldb::BreakpointLocationSP bp_loc_sp) {
+  return false;
+}
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,18 @@
+# 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
+breakpoint command add -s lua -o 'return false'
+run
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o 'print(bacon)'
+run
+# CHECK: bacon
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o "return true"
+run
+# CHECK: Process {{[0-9]+}} stopped
+breakpoint command add -s lua -o 'error("my error message")'
+run
+# CHECK: my error message
+# CHECK: Process {{[0-9]+}} stopped
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -8,14 +8,17 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include 

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 308319.
tammela added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -13,6 +13,30 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
+  lldb::StackFrameSP stop_frame_sp,
+  lldb::BreakpointLocationSP bp_loc_sp) {
+  return false;
+}
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,18 @@
+# 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
+breakpoint command add -s lua -o 'return false'
+run
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o 'print(bacon)'
+run
+# CHECK: bacon
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o "return true"
+run
+# CHECK: Process {{[0-9]+}} stopped
+breakpoint command add -s lua -o 'error("my error message")'
+run
+# CHECK: my error message
+# CHECK: Process {{[0-9]+}} stopped
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -8,14 +8,17 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
 #include 

[Lldb-commits] [PATCH] D92249: [LLDB/Python] Fix segfault on Python scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D92249#2422205 , @labath wrote:

> In D92249#2421639 , @JDevlieghere 
> wrote:
>
>> It appears there are many similar calls in this file, can you update those 
>> as well?
>
> If there are many of these calls, adding a static helper function for this 
> purpose might be a good idea as well...

Yes, it looks like the same 3-line routine is being replicated across multiple 
functions. I will clean it up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92249

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92249: [LLDB/Python] Fix segfault on Python scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
tammela requested review of this revision.
Herald added a subscriber: JDevlieghere.

The code that calls into the ScriptInterpreter was not considering the
case that it receives a Lua interpreter.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92249

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test


Index: lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
@@ -0,0 +1,8 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o 'print(frame); return False'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "lldb/Host/Config.h"
+#include "lldb/lldb-enumerations.h"
 
 #if LLDB_ENABLE_PYTHON
 
@@ -2274,7 +2275,8 @@
 return true;
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
+  ScriptInterpreter *script_interpreter =
+  debugger.GetScriptInterpreter(true, eScriptLanguagePython);
   ScriptInterpreterPythonImpl *python_interpreter =
   (ScriptInterpreterPythonImpl *)script_interpreter;
 


Index: lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/scripted_breakpoint_lua.test
@@ -0,0 +1,8 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o 'print(frame); return False'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Host/Config.h"
+#include "lldb/lldb-enumerations.h"
 
 #if LLDB_ENABLE_PYTHON
 
@@ -2274,7 +2275,8 @@
 return true;
 
   Debugger  = target->GetDebugger();
-  ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
+  ScriptInterpreter *script_interpreter =
+  debugger.GetScriptInterpreter(true, eScriptLanguagePython);
   ScriptInterpreterPythonImpl *python_interpreter =
   (ScriptInterpreterPythonImpl *)script_interpreter;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/bindings/lua/lua-wrapper.swig:21-22
+{
+   lldb::SBFrame sb_frame(stop_frame_sp);
+   lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
+

labath wrote:
> What's up with the copying? Is it to ensure the callback does not modify the 
> caller's values? If so, you could just drop the `&` from the signature, and 
> have the compiler copy these for you...
`SBFrame` and `SBBreakpointLocation` only provide copy constructors. I can't 
see the difference between the two strategies, could you elaborate? 



Comment at: lldb/bindings/lua/lua-wrapper.swig:25-26
+   // Get the Lua callback
+   lua_pushlightuserdata(L, baton);
+   lua_gettable(L, LUA_REGISTRYINDEX);
+

labath wrote:
> Is there a reason this function has to be in this file? It would be nice all 
> the baton/registry handling was happening in the same place...
My reasoning was to have the `lua_pcall` on the same function that pushes the 
Lua callback but I changed to your version as it's clearer who is handling the 
baton (`Lua` class)



Comment at: 
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test:5
+b main
+breakpoint command add -s lua -o 'print(frame)'
+run

labath wrote:
> Could we also have a test where calling the callback raises an error? And for 
> the different values of the "stop" argument that can be returned from the 
> callback? And when compiling the callback expression results in an error?
Apparently when the user sets the breakpoint and it fails to compile the code 
lldb will not report any errors and it will fail silently.  I will address this 
particular test case in another patch since it affects Python as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 308029.
tammela marked 3 inline comments as done.
tammela added a comment.

Addressing review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -13,6 +13,30 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
+  lldb::StackFrameSP _frame_sp,
+  lldb::BreakpointLocationSP _loc_sp) {
+  return false;
+}
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,18 @@
+# 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
+breakpoint command add -s lua -o 'return false'
+run
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o 'print(frame)'
+run
+# CHECK: frame #0
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o "return true"
+run
+# CHECK: Process {{[0-9]+}} stopped
+breakpoint command add -s lua -o 'error("my error message")'
+run
+# CHECK: my error message
+# CHECK: Process {{[0-9]+}} stopped
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -8,14 +8,17 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include 

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 307440.
tammela added a comment.

Remove unnecessary wrappers/checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -13,6 +13,30 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L, void *baton,
+  lldb::StackFrameSP _frame_sp,
+  lldb::BreakpointLocationSP _loc_sp) {
+  return false;
+}
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,7 @@
+# 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
+breakpoint command add -s lua -o 'print(frame)'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -8,14 +8,17 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/Support/FormatAdapters.h"
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -174,6 +177,49 @@
   return m_lua->Run(str);
 }
 
+bool ScriptInterpreterLua::BreakpointCallbackFunction(
+void *baton, StoppointCallbackContext *context, user_id_t break_id,
+user_id_t break_loc_id) {
+  assert(context);
+
+  ExecutionContext 

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela marked an inline comment as done.
tammela added a comment.

In D91508#2413582 , @labath wrote:

> Right. That's why I'd like to have good wrappers, which make it easy to do 
> the right thing, and hard to do the wrong one.
>
> I don't think we're quite there yet, but before I comment on the API, I want 
> to understand one other thing.
>
> I am puzzled by all the wrapping that's happening inside the `PushSBClass` 
> functions. What is that protecting us from? I would hope that pushing a swig 
> wrapper on the stack is a safe operation...

I thought that too, but internally it's a naked call to `lua_newuserdata()` 
which might throw in case of a memory error.

> So, IIUC, this can only fail if we are running out of memory? If that's the 
> case, then I would remove these checks, as (for better or worse) llvm is not 
> robust against memory allocation errors, and they add a fair amount of cruft 
> to the code.

Fair enough. Will remove those.
Since this seems to be a fact of life for LLVM, perhaps wrapping potential 
memory errors turns out to be just bloat. If that's the case, then the wrapping 
in `PushSBClass` is not needed and the `abort()` call that Lua does is honest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

> I'm not sure what you mean by that. Can you elaborate?

Sure, it's was just a nod to your previous comment about that running callbacks 
(C++ lambdas) inside a `pcall` is a dangerous API. Although possible, it 
requires that the developer be very cautious about implicit throws.

> The way I see it, the problem is not about which environment is the c++ code 
> running in, as it can't throw an exception (in llvm). The problem is what 
> happens when we call lua from c++, and the lua code throws. And this is a 
> problem regardless of the environment the C(++) code is in -- the difference 
> is "only" in whether lua will panic or jump over the C code, but something 
> bad will happen anyway.

I agree 100% with your analysis of the problem.

> I think that could be solved by ensuring we always call lua in a protected 
> environment. That way lua will never unwind through the C code (regardless of 
> what environment it is in) because it would always stop right at the boundary.
>
> That would essentially mean, lua_call is banned (except maybe as an 
> optimization, if you can be sure that the code you're about to call will not 
> throw), and all C->lua transitions should go through lua_pcall. And we can 
> provide a wrapper that handles catches those errors and converts them to 
> llvm::Error/Expected, similar to how the python things do it.

I have posted a redesign based on your comments. The `lua_push*` functions are 
all safe-ish. The Lua implementation guarantees at least 20 stack slots when 
the `lua_State` is created so I've added the stack checks for sanity rather 
than necessity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 307202.
tammela added a comment.

Redesign of the API


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -13,6 +13,30 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L, void *baton,
+  lldb::StackFrameSP _frame_sp,
+  lldb::BreakpointLocationSP _loc_sp) {
+  return false;
+}
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,7 @@
+# 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
+breakpoint command add -s lua -o 'print(frame)'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -8,14 +8,17 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/Support/FormatAdapters.h"
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -174,6 +177,49 @@
   return m_lua->Run(str);
 }
 
+bool ScriptInterpreterLua::BreakpointCallbackFunction(
+void *baton, StoppointCallbackContext *context, user_id_t break_id,
+user_id_t break_loc_id) {
+  assert(context);
+
+  ExecutionContext exe_ctx(context->exe_ctx_ref);
+  

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:60-78
+static int runCallback(lua_State *L) {
+  LuaCallback *callback = static_cast(lua_touserdata(L, -1));
+  return (*callback)(L);
+}
+
+llvm::Error Lua::Run(LuaCallback ) {
+  lua_pushcfunction(m_lua_state, runCallback);

labath wrote:
> tammela wrote:
> > labath wrote:
> > > I'm confused. Why use lua to call a c callback, when you could just do 
> > > `calllback()`?
> > Some Lua API functions throw errors, if there's any it will `abort()` the 
> > program if no panic handler or protected call is provided.
> > To guarantee that the callback always runs in a protected environment and 
> > it has error handling, we do the above.
> > Delegating this to the callback itself makes it cumbersome to write.
> Aha, I see.
> 
> So, if I remember my lua correctly (I wrote a c++ lua wrapper once, but that 
> was years ago..), whenever there's a lua exception inside this (c++) 
> callback, lua will longjmp(3) back to the lua_pcall call on line 68, skipping 
> any destructors that should normally be invoked. Is that so?
> 
> If that's the case, then I think this is a dangerous api, that should at the 
> very least get a big fat warning, but that ideally shouldn't exist at all.
> 
> What's the part that makes delegating this to the callback "cumersome to 
> write"? And why can't that be overcome by a suitable utility function which 
> wraps `lua_pcall` or whatever else is needed?
> 
> The approach that we've chosen in python is to have very little code 
> interacting with the python C API directly. Instead, code generally works 
> with our C++ wrappers defined in `PythonDataObject.h`. These functions try to 
> hide the python exception magic as much as possible, and present a c++-y 
> version of the interface.
> 
> Now, since lua is stack-based, something like LuaDataObjects.h would probably 
> not work. However, that doesn't meant we should give up on the c++ wrapper  
> idea altogether. During the intitial reviews, my intention was for the `Lua` 
> class to serve this purpose. I still think this can be achieved if we make 
> the callback functions take `Lua&` instead of `lua_State*` as an argument, 
> and then ensure the class contains whatever is needed to make the callbacks 
> not cumerbsome to write.
> So, if I remember my lua correctly (I wrote a c++ lua wrapper once, but that 
> was years ago..), whenever there's a lua exception inside this (c++) 
> callback, lua will longjmp(3) back to the lua_pcall call on line 68, skipping 
> any destructors that should normally be invoked. Is that so?
>
> If that's the case, then I think this is a dangerous api, that should at the 
> very least get a big fat warning, but that ideally shouldn't exist at all.

You are right. This escaped me completely.
Lua can be compiled to use `try/catch` instead of `longjmp`, but that is an 
exception (no pun intended). Since we rely on the host's library, we need to 
assume that it's using `longjmp`.

> What's the part that makes delegating this to the callback "cumersome to 
> write"? And why can't that be overcome by a suitable utility function which 
> wraps lua_pcall or whatever else is needed?
>
> The approach that we've chosen in python is to have very little code 
> interacting with the python C API directly. Instead, code generally works 
> with our C++ wrappers defined in PythonDataObject.h. These functions try to 
> hide the python exception magic as much as possible, and present a c++-y 
> version of the interface.
>
> Now, since lua is stack-based, something like LuaDataObjects.h would probably 
> not work. However, that doesn't meant we should give up on the c++ wrapper 
> idea altogether. During the intitial reviews, my intention was for the Lua 
> class to serve this purpose. I still think this can be achieved if we make 
> the callback functions take Lua& instead of lua_State* as an argument, and 
> then ensure the class contains whatever is needed to make the callbacks not 
> cumerbsome to write.

Any C++ code that runs in a `lua_pcall` seems to face this issue. I checked a 
very complete C++ Lua wrapper called `sol2` and they seem to have the same 
issue. I don't think there's a way out of it except code discipline.

Lua provides `lua_atpanic` as a way to recover from unprotected throws. Perhaps 
we can leverage this to throw an exception, guaranteeing stack unwinding and 
avoiding a call to `abort()`. We would then let the callback run unprotected 
and delegate any calls to `lua_pcall` to the callback.

Changes proposed:
- Register the panic function on `Lua` ctor
- Let the `Callback` run unprotected
- Wrap the `Callback` call in a `try catch` block
- Change the `Callback` signature to receive `Lua&` and return `llvm::Error`

It looks like it ticks all goals.
- Unprotected Lua errors do not cause lldb to abort.
- Stack unwinding is guaranteed for C++ code interacting with the Lua state.
- 

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D91508#2400222 , @JDevlieghere 
wrote:

> In D91508#2396392 , @tammela wrote:
>
>> @JDevlieghere
>>
>> When writing this patch I noticed that there is no mechanism in-place to 
>> remove the Python/Lua function when the breakpoint is removed or when the 
>> callback function is replaced.
>> The class that sets the callback provides `ClearCallback()` but it never 
>> calls into the `ScriptInterpreter` to clean up.
>> Although the real world impact is not that big, it's crucial for a small 
>> memory footprint. Any thoughts on this?
>
> I see, it looks like we just clean up the LLDB side but never tell the 
> runtimes about it. What exactly are we leaking in that case? Do you think 
> it's worth it to wire that up?

For both interpreters, the function objects itself. I think it's worth it, 
specially for long running debugging sessions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:200
+  Debugger  = target->GetDebugger();
+  ScriptInterpreterLua *lua_interpreter =
+  static_cast(debugger.GetScriptInterpreter());

labath wrote:
> How is `lua_interpreter` different from `this`?
Are you asking why I didn't go for `m_script_interpreter`?

This function is static (on the class declaration), perhaps it's clearer a 
`static` keyword here as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 305874.
tammela marked 4 inline comments as done and an inline comment as not done.
tammela added a comment.

Adressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -43,6 +43,13 @@
 };
 } // namespace
 
+struct lua_State;
+extern "C" bool LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, void *baton, const lldb::StackFrameSP _frame,
+const lldb::BreakpointLocationSP _bp_loc) {
+  return false;
+}
+
 TEST_F(ScriptInterpreterTest, Plugin) {
   EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
   EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -26,3 +26,27 @@
   EXPECT_EQ(llvm::toString(std::move(error)),
 "[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
 }
+
+TEST(LuaTest, RunCallbackValid) {
+  Lua lua;
+  Lua::Callback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "foo = 1") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunCallbackInvalid) {
+  Lua lua;
+  Lua::Callback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "nil = foo") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"nil = foo\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,7 @@
+# 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
+breakpoint command add -s lua -o 'print(frame)'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ 

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:60-78
+static int runCallback(lua_State *L) {
+  LuaCallback *callback = static_cast(lua_touserdata(L, -1));
+  return (*callback)(L);
+}
+
+llvm::Error Lua::Run(LuaCallback ) {
+  lua_pushcfunction(m_lua_state, runCallback);

labath wrote:
> I'm confused. Why use lua to call a c callback, when you could just do 
> `calllback()`?
Some Lua API functions throw errors, if there's any it will `abort()` the 
program if no panic handler or protected call is provided.
To guarantee that the callback always runs in a protected environment and it 
has error handling, we do the above.
Delegating this to the callback itself makes it cumbersome to write.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 305427.
tammela added a comment.

Clean up includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -43,6 +43,13 @@
 };
 } // namespace
 
+struct lua_State;
+extern "C" bool LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, void *baton, const lldb::StackFrameSP _frame,
+const lldb::BreakpointLocationSP _bp_loc) {
+  return false;
+}
+
 TEST_F(ScriptInterpreterTest, Plugin) {
   EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
   EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -26,3 +26,27 @@
   EXPECT_EQ(llvm::toString(std::move(error)),
 "[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
 }
+
+TEST(LuaTest, RunCallbackValid) {
+  Lua lua;
+  LuaCallback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "foo = 1") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunCallbackInvalid) {
+  Lua lua;
+  LuaCallback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "nil = foo") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"nil = foo\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,5 @@
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o 'print(1)'
+run
+# CHECK: 1
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -8,18 +8,26 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include 

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

@JDevlieghere

When writing this patch I noticed that there is no mechanism in-place to remove 
the Python/Lua function when the breakpoint is removed or when the callback 
function is replaced.
The class that sets the callback provides `ClearCallback()` but it never calls 
into the `ScriptInterpreter` to clean up.
Although the real world impact is not that big, it's crucial for a small memory 
footprint. Any thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
tammela requested review of this revision.
Herald added a subscriber: JDevlieghere.

These callbacks are set using the following:

  breakpoint command add -s lua -o "print('hello world!')"

The user supplied script is executed as:

  function (frame, bp_loc, ...)
 
  end

So the local variables 'frame', 'bp_loc' and vararg are all accessible.
Any global variables declared will persist in the Lua interpreter.
A user should never hold 'frame' and 'bp_loc' in a global variable as
these userdatas are context dependent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -43,6 +43,13 @@
 };
 } // namespace
 
+struct lua_State;
+extern "C" bool LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, void *baton, const lldb::StackFrameSP _frame,
+const lldb::BreakpointLocationSP _bp_loc) {
+  return false;
+}
+
 TEST_F(ScriptInterpreterTest, Plugin) {
   EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
   EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -26,3 +26,27 @@
   EXPECT_EQ(llvm::toString(std::move(error)),
 "[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
 }
+
+TEST(LuaTest, RunCallbackValid) {
+  Lua lua;
+  LuaCallback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "foo = 1") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunCallbackInvalid) {
+  Lua lua;
+  LuaCallback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "nil = foo") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"nil = foo\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,5 @@
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o 'print(1)'
+run
+# CHECK: 1
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_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;
+}
+  };
+
   ScriptInterpreterLua(Debugger );
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = 

[Lldb-commits] [PATCH] D90787: [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGca17571051d4: [LLDB-lua] modify Luas print 
to respect io.stdout (authored by tammela).

Changed prior to commit:
  https://reviews.llvm.org/D90787?vs=303088=303249#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90787

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/test/Shell/ScriptInterpreter/Lua/print.test


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126nil a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> 
%t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,34 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_getfield(L, -1, "stdout");
+  lua_getfield(L, -1, "write");
+  for (int i = 1; i <= n; i++) {
+lua_pushvalue(L, -1); // write()
+lua_pushvalue(L, -3); // io.stdout
+luaL_tolstring(L, i, nullptr);
+lua_pushstring(L, i != n ? "\t" : "\n");
+lua_call(L, 3, 0);
+  }
+  return 0;
+}
+
+Lua::Lua() : m_lua_state(luaL_newstate()) {
+  assert(m_lua_state);
+  luaL_openlibs(m_lua_state);
+  luaopen_lldb(m_lua_state);
+  lua_pushcfunction(m_lua_state, lldb_print);
+  lua_setglobal(m_lua_state, "print");
+}
+
+Lua::~Lua() {
+  assert(m_lua_state);
+  lua_close(m_lua_state);
+}
+
 llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126	nil	a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp

[Lldb-commits] [PATCH] D90787: [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 303088.
tammela added a comment.

Add test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90787

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/test/Shell/ScriptInterpreter/Lua/print.test


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126nil a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> 
%t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,35 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_getfield(L, -1, "stdout");
+  lua_getfield(L, -1, "write");
+  for (int i = 1; i <= n; i++) {
+lua_pushvalue(L, -1); // write()
+lua_pushvalue(L, -3); // io.stdout
+luaL_tolstring(L, i, nullptr);
+lua_pushstring(L, i != n ? "\t" : "\n");
+lua_call(L, 3, 0);
+  }
+  return 0;
+}
+
+Lua::Lua() {
+  m_lua_state = luaL_newstate();
+  assert(m_lua_state);
+  luaL_openlibs(m_lua_state);
+  luaopen_lldb(m_lua_state);
+  lua_pushcfunction(m_lua_state, lldb_print);
+  lua_setglobal(m_lua_state, "print");
+}
+
+Lua::~Lua() {
+  assert(m_lua_state);
+  lua_close(m_lua_state);
+}
+
 llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126	nil	a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,35 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  

[Lldb-commits] [PATCH] D90787: [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
tammela requested review of this revision.
Herald added a subscriber: JDevlieghere.

This patch changes the implementation of Lua's `print()` function to
respect `io.stdout`.

The original implementation uses `lua_writestring()` internally, which is
hardcoded to `stdout`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90787

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,35 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_getfield(L, -1, "stdout");
+  lua_getfield(L, -1, "write");
+  for (int i = 1; i <= n; i++) {
+lua_pushvalue(L, -1); // write()
+lua_pushvalue(L, -3); // io.stdout
+luaL_tolstring(L, i, nullptr);
+lua_pushstring(L, i != n ? "\t" : "\n");
+lua_call(L, 3, 0);
+  }
+  return 0;
+}
+
+Lua::Lua() {
+  m_lua_state = luaL_newstate();
+  assert(m_lua_state);
+  luaL_openlibs(m_lua_state);
+  luaopen_lldb(m_lua_state);
+  lua_pushcfunction(m_lua_state, lldb_print);
+  lua_setglobal(m_lua_state, "print");
+}
+
+Lua::~Lua() {
+  assert(m_lua_state);
+  lua_close(m_lua_state);
+}
+
 llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,35 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_getfield(L, -1, "stdout");
+  lua_getfield(L, -1, "write");
+  for (int i = 1; i <= n; i++) {
+lua_pushvalue(L, -1); // write()
+lua_pushvalue(L, -3); // io.stdout
+luaL_tolstring(L, i, nullptr);
+lua_pushstring(L, i != n ? "\t" : "\n");
+lua_call(L, 3, 0);
+  }
+  return 0;
+}
+
+Lua::Lua() {
+  m_lua_state = luaL_newstate();
+  assert(m_lua_state);
+  luaL_openlibs(m_lua_state);
+  luaopen_lldb(m_lua_state);
+  lua_pushcfunction(m_lua_state, lldb_print);
+  lua_setglobal(m_lua_state, "print");
+}
+
+Lua::~Lua() {
+  assert(m_lua_state);
+  lua_close(m_lua_state);
+}
+
 llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90556: [LLDB][NFC] treat Lua error codes in a more explicit manner

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b9fa3b705c8: [LLDB][NFC] treat Lua error codes in a more 
explicit manner (authored by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90556

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


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -18,7 +18,7 @@
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
   lua_pcall(m_lua_state, 0, 0, 0);
-  if (!error)
+  if (error == LUA_OK)
 return llvm::Error::success();
 
   llvm::Error e = llvm::make_error(
@@ -44,7 +44,7 @@
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
   lua_pcall(m_lua_state, 0, 1, 0);
-  if (error) {
+  if (error != LUA_OK) {
 llvm::Error e = llvm::make_error(
 llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -18,7 +18,7 @@
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
   lua_pcall(m_lua_state, 0, 0, 0);
-  if (!error)
+  if (error == LUA_OK)
 return llvm::Error::success();
 
   llvm::Error e = llvm::make_error(
@@ -44,7 +44,7 @@
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
   lua_pcall(m_lua_state, 0, 1, 0);
-  if (error) {
+  if (error != LUA_OK) {
 llvm::Error e = llvm::make_error(
 llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90557: [LLDB/Lua] call lua_close() on Lua dtor

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d7d6f276c6d: [LLDB/Lua] call lua_close() on Lua dtor 
(authored by tammela).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90557

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


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -33,7 +33,7 @@
 
   ~Lua() {
 assert(m_lua_state);
-luaL_openlibs(m_lua_state);
+lua_close(m_lua_state);
   }
 
   llvm::Error Run(llvm::StringRef buffer);


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -33,7 +33,7 @@
 
   ~Lua() {
 assert(m_lua_state);
-luaL_openlibs(m_lua_state);
+lua_close(m_lua_state);
   }
 
   llvm::Error Run(llvm::StringRef buffer);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90557: [LLDB/Lua] call lua_close() on Lua dtor

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
tammela requested review of this revision.
Herald added a subscriber: JDevlieghere.

This patch calls `lua_close()` on Lua dtor.

This guarantees that the Lua GC finalizers are honored, aside from the
usual internal clean up.

It also guarantees a call to the `__close` metamethod of any active
to-be-closed variable in Lua 5.4.

Since the previous `luaL_openlibs()` was a noop, because the standard
library is cached internally, I've removed it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90557

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


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -33,7 +33,7 @@
 
   ~Lua() {
 assert(m_lua_state);
-luaL_openlibs(m_lua_state);
+lua_close(m_lua_state);
   }
 
   llvm::Error Run(llvm::StringRef buffer);


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -33,7 +33,7 @@
 
   ~Lua() {
 assert(m_lua_state);
-luaL_openlibs(m_lua_state);
+lua_close(m_lua_state);
   }
 
   llvm::Error Run(llvm::StringRef buffer);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90556: [LLDB][NFC] treat Lua error codes in a more explicit manner

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
tammela requested review of this revision.
Herald added a subscriber: JDevlieghere.

This patch is a minor suggestion to not rely on the fact
that the `LUA_OK` macro is 0.

This assumption could change in future versions of the C API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90556

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


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -18,7 +18,7 @@
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
   lua_pcall(m_lua_state, 0, 0, 0);
-  if (!error)
+  if (error == LUA_OK)
 return llvm::Error::success();
 
   llvm::Error e = llvm::make_error(
@@ -44,7 +44,7 @@
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
   lua_pcall(m_lua_state, 0, 1, 0);
-  if (error) {
+  if (error != LUA_OK) {
 llvm::Error e = llvm::make_error(
 llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -18,7 +18,7 @@
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
   lua_pcall(m_lua_state, 0, 0, 0);
-  if (!error)
+  if (error == LUA_OK)
 return llvm::Error::success();
 
   llvm::Error e = llvm::make_error(
@@ -44,7 +44,7 @@
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
   lua_pcall(m_lua_state, 0, 1, 0);
-  if (error) {
+  if (error != LUA_OK) {
 llvm::Error e = llvm::make_error(
 llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits