[lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

[JDevlieghere]

Summary

This implements a very elementary Lua script interpreter. It supports running a single command as well as running interactively. It uses editline if available. It's still missing a bunch of stuff though. Some things that I intentionally ingored for now are that I/O isn't properly hooked up (so every print goes to stdout) and the non-editline support which is not handling a bunch of corner cases. The latter is a matter of reusing existing code in the Python interpreter.

Discussion on the mailing list: http://lists.llvm.org/pipermail/lldb-dev/2019-December/015812.html

[xiaobai]

lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt

11

Nit: I would use target_include_directories here.

https://cmake.org/cmake/help/latest/command/target_include_directories.html

[labath]

It looks like this patch already includes some non-trivial functionality, so I think this is a good time to start figuring out how to test this. Another thing, which may not be needed straight away, but which I think we should consider pretty early is creating some sort of c++ wrappers, similar to the PythonDataObject we have for python. This is so that we don't have C error handling code everywhere, but have things wrapped in a nicer interface with llvm::Expecteds and everything.

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

39

How much of this class is boiler plate? Can it be shared with something?

49

TBH, I am not sure how useful this error really is. Presumably you will get some sort of an error from the lua interpreter if you just let this pass..

57

So what happens when command is not null terminated? I guess you ought to split this into luaL_loadbuffer && lua_pcall..

61

same here. You can just use AppendErrorWithFormatv..

[JDevlieghere]

Reuse IOHandlerEditline

[JDevlieghere]

• Rebase
• Hide C calls behind C++ abstraction
• Improve error handling
• Add unit test

[JDevlieghere]

• Rebase
• Add test case

[labath]

This is starting to look pretty good. I think that pretty soon you'll have to come up with some way of having a more persistent lua context (instead of creating a fresh one for each command), but that doesn't have to happen now.

Does the "multiline" script command do anything already? Is there anything to test with respect to that.

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

17–19

No longer needed?

23

it doesn't look like this is used (but if it is, it would be better off as a regular variable)

49

The constructor initialized this member, and the desctructor asserts it to be not null. It doesn't look like this assignment here is actually helping anything..

64

*GetOutputStreamFileSP() << llvm::toString(std::move(error))

68

delete

76

This if is not needed (new always succeeds)

lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

54

I'm not actually opposed to this (and it's nice to know that this could work if we need it), but why do this as a unit test? It seems like this could be tested equally well with a script command through the lldb driver. I was imagining the unit tests would be most useful for the lower level functionality -- roughly corresponding to the Lua class. Right now that class is pretty simple and doesn't really need a unit test, but I expect it will become more complex over time...

[JDevlieghere]

• Rebase
• Feedback Pavel

lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

54

Yes, I just wanted to show that it's possible :-) I've also included a test for the Lua class.

[labath]

lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt

15–16

I think you wouldn't need these if the other CMakeLists file would use PUBLIC keyword for headers, and INTERFACE for libraries.

lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

54

Ok, that's cool.

lldb/cmake/modules/LLDBConfig.cmake

else()
   # cache variable.
   set(LLDB_EXPORT_ALL_SYMBOLS 0)
 endif()
 
 if ((NOT MSVC) OR MSVC12)
   add_definitions( -DHAVE_ROUND )
 endif()
 
+if (LLDB_ENABLE_LUA)
+  find_package(Lua REQUIRED)
+endif()
 
 if (LLDB_ENABLE_LIBEDIT)
   find_package(LibEdit REQUIRED)
 
   # Check if we libedit capable of handling wide characters (built with
   # '--enable-widec').
   set(CMAKE_REQUIRED_LIBRARIES ${libedit_LIBRARIES})
   set(CMAKE_REQUIRED_INCLUDES ${libedit_INCLUDE_DIRS})

lldb/include/lldb/Core/IOHandler.h

enum class Type {
     CommandInterpreter,
     CommandList,
     Confirm,
     Curses,
     Expression,
     REPL,
     ProcessIO,
     PythonInterpreter,
+    LuaInterpreter,
     PythonCode,
     Other
   };
 
   IOHandler(Debugger &debugger, IOHandler::Type type);
 
   IOHandler(Debugger &debugger, IOHandler::Type type,
             const lldb::FileSP &input_sp, const lldb::StreamFileSP &output_sp,

lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt

 add_lldb_library(lldbPluginScriptInterpreterLua PLUGIN
+  Lua.cpp
   ScriptInterpreterLua.cpp
 
   LINK_LIBS
     lldbCore
     lldbInterpreter
-  )
+  )
+
+target_include_directories(lldbPluginScriptInterpreterLua PUBLIC ${LUA_INCLUDE_DIR})
+target_link_libraries(lldbPluginScriptInterpreterLua INTERFACE ${LUA_LIBRARIES})

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

+//===-- ScriptInterpreterLua.h ----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Lua_h_
+#define liblldb_Lua_h_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+#include "lua.hpp"
+
+namespace lldb_private {
+
+class Lua {
+public:
+  Lua() : m_lua_state(luaL_newstate()) {
+    assert(m_lua_state);
+    luaL_openlibs(m_lua_state);
+  }
+
+  ~Lua() {
+    assert(m_lua_state);
+    luaL_openlibs(m_lua_state);
+  }
+
+  llvm::Error Run(llvm::StringRef buffer);
+
+private:
+  lua_State *m_lua_state;
+};
+
+} // namespace lldb_private
+
+#endif // liblldb_Lua_h_

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

+//===-- Lua.cpp -----------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Lua.h"
+#include "llvm/Support/FormatVariadic.h"
+
+using namespace lldb_private;
+
+llvm::Error Lua::Run(llvm::StringRef buffer) {
+  int error =
+      luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
+      lua_pcall(m_lua_state, 0, 0, 0);
+  if (!error)
+    return llvm::Error::success();
+
+  llvm::Error e = llvm::make_error<llvm::StringError>(
+      llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
+      llvm::inconvertibleErrorCode());
+  // Pop error message from the stack.
+  lua_pop(m_lua_state, 1);
+  return e;
+}

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

 //===-- ScriptInterpreterLua.cpp --------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ScriptInterpreterLua.h"
+#include "Lua.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
-
-#include "llvm/Support/Threading.h"
-
-#include <mutex>
+#include "lldb/Utility/Timer.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
+class IOHandlerLuaInterpreter : public IOHandlerDelegate,
+                                public IOHandlerEditline {
+public:
+  IOHandlerLuaInterpreter(Debugger &debugger)
+      : IOHandlerEditline(debugger, IOHandler::Type::LuaInterpreter, "lua",
+                          ">>> ", "..> ", true, debugger.GetUseColor(), 0,
+                          *this, nullptr),
+        m_lua() {}
+
+  void IOHandlerInputComplete(IOHandler &io_handler,
+                              std::string &data) override {
+    if (llvm::Error error = m_lua.Run(data)) {
+      *GetOutputStreamFileSP() << llvm::toString(std::move(error));
+    }
+  }
+
+private:
+  Lua m_lua;
+};
+
 ScriptInterpreterLua::ScriptInterpreterLua(Debugger &debugger)
     : ScriptInterpreter(debugger, eScriptLanguageLua) {}
 
 ScriptInterpreterLua::~ScriptInterpreterLua() {}
 
 bool ScriptInterpreterLua::ExecuteOneLine(llvm::StringRef command,
-                                          CommandReturnObject *,
-                                          const ExecuteScriptOptions &) {
-  m_debugger.GetErrorStream().PutCString(
-      "error: the lua script interpreter is not yet implemented.\n");
+                                          CommandReturnObject *result,
+                                          const ExecuteScriptOptions &options) {
+  Lua l;
+  if (llvm::Error e = l.Run(command)) {
+    result->AppendErrorWithFormatv(
+        "lua failed attempting to evaluate '{0}': {1}\n", command,
+        llvm::toString(std::move(e)));
     return false;
   }
+  return true;
+}
 
 void ScriptInterpreterLua::ExecuteInterpreterLoop() {
-  m_debugger.GetErrorStream().PutCString(
-      "error: the lua script interpreter is not yet implemented.\n");
+  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+  Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
+
+  Debugger &debugger = m_debugger;
+
+  // At the moment, the only time the debugger does not have an input file
+  // handle is when this is called directly from lua, in which case it is
+  // both dangerous and unnecessary (not to mention confusing) to try to embed
+  // a running interpreter loop inside the already running lua interpreter
+  // loop, so we won't do it.
+
+  if (!debugger.GetInputFile().IsValid())
+    return;
+
+  IOHandlerSP io_handler_sp(new IOHandlerLuaInterpreter(debugger));
+  debugger.PushIOHandler(io_handler_sp);
 }
 
 void ScriptInterpreterLua::Initialize() {
   static llvm::once_flag g_once_flag;
 
   llvm::call_once(g_once_flag, []() {
     PluginManager::RegisterPlugin(GetPluginNameStatic(),
                                   GetPluginDescriptionStatic(),

lldb/test/CMakeLists.txt

 
 if (CMAKE_SIZEOF_VOID_P EQUAL 8)
   set(LLDB_IS_64_BITS 1)
 endif()
 
 # These values are not canonicalized within LLVM.
 llvm_canonicalize_cmake_booleans(
   LLDB_ENABLE_PYTHON
+  LLDB_ENABLE_LUA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_IS_64_BITS)
 
 # Configure the individual test suites.
 add_subdirectory(API)
 add_subdirectory(Shell)
 add_subdirectory(Unit)

lldb/test/Shell/ScriptInterpreter/Lua/lua.test

+# REQUIRES: lua
+# RUN: %lldb --script-language lua -o 'script print(1000+100+10+1)' 2>&1 | FileCheck %s
+# CHECK: 1111

lldb/test/Shell/lit.cfg.py

else:
             for x in out.split():
                 config.available_features.add('native-cpu-%s' % x)
         else:
             lit_config.warning("lit-cpuid failed: %s" % err)
 
 if config.lldb_enable_python:
     config.available_features.add('python')
 
+if config.lldb_enable_lua:
+    config.available_features.add('lua')
+
 if config.lldb_enable_lzma:
     config.available_features.add('lzma')
 
 if find_executable('xz') != None:
     config.available_features.add('xz')
 
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled

lldb/test/Shell/lit.site.cfg.py.in

 config.lldb_lit_tools_dir = r"@LLDB_LIT_TOOLS_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
+config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell")
 config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-shell")
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
 try:

lldb/unittests/ScriptInterpreter/CMakeLists.txt

 if (LLDB_ENABLE_PYTHON)
   add_subdirectory(Python)
 endif()
+if (LLDB_ENABLE_LUA)
+  add_subdirectory(Lua)
+endif()

lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt

+add_lldb_unittest(ScriptInterpreterLuaTests
+  LuaTests.cpp
+  ScriptInterpreterTests.cpp
+
+  LINK_LIBS
+    lldbHost
+    lldbPluginScriptInterpreterLua
+    lldbPluginPlatformLinux
+    LLVMTestingSupport
+  LINK_COMPONENTS
+    Support
+  )
 No newline at end of file

lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

+//===-- LuaTests.cpp ------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Plugins/ScriptInterpreter/Lua/Lua.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(LuaTest, RunValid) {
+  Lua lua;
+  llvm::Error error = lua.Run("foo = 1");
+  EXPECT_FALSE(static_cast<bool>(error));
+}
+
+TEST(LuaTest, RunInvalid) {
+  Lua lua;
+  llvm::Error error = lua.Run("nil = foo");
+  EXPECT_TRUE(static_cast<bool>(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+            "[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
+}

lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

+//===-- LuaTests.cpp ------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Utility/Reproducer.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+namespace {
+class ScriptInterpreterTest : public ::testing::Test {
+public:
+  void SetUp() override {
+    llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+    FileSystem::Initialize();
+    HostInfo::Initialize();
+
+    // Pretend Linux is the host platform.
+    platform_linux::PlatformLinux::Initialize();
+    ArchSpec arch("powerpc64-pc-linux");
+    Platform::SetHostPlatform(
+        platform_linux::PlatformLinux::CreateInstance(true, &arch));
+  }
+  void TearDown() override {
+    platform_linux::PlatformLinux::Terminate();
+    HostInfo::Terminate();
+    FileSystem::Terminate();
+    Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(ScriptInterpreterTest, Plugin) {
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
+  EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
+            "Lua script interpreter");
+}
+
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  ScriptInterpreterLua script_interpreter(*debugger_sp);
+  CommandReturnObject result;
+  EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", &result));
+  EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", &result));
+  EXPECT_TRUE(result.GetErrorData().startswith(
+      "error: lua failed attempting to evaluate 'nil = foo'"));
+}