Hi, Sergey!
Thanks for the review!
 
Среда, 22 ноября 2023, 17:32 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
 

Hello, Max,

thanks for the tests!

See my comments.

 

First of all - TestLJStr always timed out:

======================================================================
ERROR: setUpClass (__main__.TestLJStr)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sergeyb/sources/MRG/tarantool/third_party/luajit/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py", line 88, in setUpClass
    cls.output = filter_debugger_output(execute_process(process_cmd))
  File "/home/sergeyb/sources/MRG/tarantool/third_party/luajit/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py", line 50, in execute_process
    process = subprocess.run(cmd, capture_output=True, timeout=TIMEOUT)
  File "/usr/lib/python3.10/subprocess.py", line 505, in run
    stdout, stderr = process.communicate(input, timeout=timeout)
  File "/usr/lib/python3.10/subprocess.py", line 1154, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "/usr/lib/python3.10/subprocess.py", line 2022, in _communicate
    self._check_timeout(endtime, orig_timeout, stdout, stderr)
  File "/usr/lib/python3.10/subprocess.py", line 1198, in _check_timeout
    raise TimeoutExpired(
subprocess.TimeoutExpired: Command '['/bin/gdb', '-x', '/tmp/tmpoo5cpysq', '--args', '/home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/src/luajit', '/tmp/tmpfupekxcq']' timed out after 10 seconds

Can’t reproduce regardless of lldb being installed or not.

make LuaJIT-debugger-lldb-tests works fine

 

On 11/10/23 23:16, Maksim Kokryashkin wrote:
This patch adds tests for LuaJIT debugging
extensions for lldb and gdb.
---
 .flake8rc                                     |   4 +
 test/CMakeLists.txt                           |   3 +
 .../CMakeLists.txt                            |  78 ++++++
 .../debug-extension-tests.py                  | 250 ++++++++++++++++++
 4 files changed, 335 insertions(+)
 create mode 100644 test/LuaJIT-debug-extensions-tests/CMakeLists.txt
 create mode 100644 test/LuaJIT-debug-extensions-tests/debug-extension-tests.py

diff --git a/.flake8rc b/.flake8rc
index 13e6178f..6766ed41 100644
--- a/.flake8rc
+++ b/.flake8rc
@@ -3,3 +3,7 @@ extend-ignore =
   # XXX: Suppress F821, since we have autogenerated names for
   # 'ptr' type complements in luajit_lldb.py.
   F821
+per-file-ignores =
+  # XXX: Flake considers regexp special characters to be
+  # escape sequences.
+  test/LuaJIT-debug-extensions-tests/debug-extension-tests.py:W605
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 58cba5ba..d7910ea4 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -78,6 +78,7 @@ add_subdirectory(PUC-Rio-Lua-5.1-tests)
 add_subdirectory(lua-Harness-tests)
 add_subdirectory(tarantool-c-tests)
 add_subdirectory(tarantool-tests)
+add_subdirectory(LuaJIT-debug-extensions-tests)
 
 add_custom_target(${PROJECT_NAME}-test DEPENDS
   LuaJIT-tests
@@ -85,6 +86,8 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS
   lua-Harness-tests
   tarantool-c-tests
   tarantool-tests
+  LuaJIT-lldb-extension-tests
+  LuaJIT-gdb-extension-tests
 )
 
 if(LUAJIT_USE_TEST)
diff --git a/test/LuaJIT-debug-extensions-tests/CMakeLists.txt b/test/LuaJIT-debug-extensions-tests/CMakeLists.txt
new file mode 100644
index 00000000..3b38201d
--- /dev/null
+++ b/test/LuaJIT-debug-extensions-tests/CMakeLists.txt
@@ -0,0 +1,78 @@
+add_custom_target(LuaJIT-gdb-extension-tests
+  DEPENDS ${LUAJIT_TEST_BINARY}
+)
+
+add_custom_target(LuaJIT-lldb-extension-tests
+  DEPENDS ${LUAJIT_TEST_BINARY}
+)
+
+# Debug info is required for testing of extensions.
+if(NOT (CMAKE_BUILD_TYPE MATCHES Debug))
+  message(WARNING
+    "not a DEBUG build, LuaJIT-lldb-extension-tests and "
+    "LuaJIT-gdb-extension-tests are dummy"
+  )
+  return()
+endif()
+
+# MacOS asks for permission to debug a process even when the
+# machine is set into development mode. To solve the issue,
+# it is required to add relevant users to the `_developer` user
+# group in MacOS. Disabled for now.
+if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND DEFINED ENV{CI})
+  message(WARNING
+    "Interactive debugging is unavailable for macOS CI builds,"
+    "LuaJIT-lldb-extension-tests is dummy"
+  )
+  return()
+endif()
+
+find_package(PythonInterp)
+if(NOT PYTHONINTERP_FOUND)
+  message(WARNING
+    "`python` is not found, LuaJIT-lldb-extension-tests and "
+    "LuaJIT-gdb-extension-tests are dummy"
+  )
+  return()
+endif()
+
+set(DEBUGGER_TEST_ENV
+  "LUAJIT_TEST_BINARY=${LUAJIT_TEST_BINARY}"

I suppose it should be something like this:

get_property(LUAJIT_TEST_BINARY TARGET ${LUAJIT_TEST_BINARY} PROPERTY LOCATION)

 

That variable comes from the parent scope, so there is no need to do that.
Ignoring.
+  # Suppresses __pycache__ generation.
+  "PYTHONDONTWRITEBYTECODE=1"
+  "DEBUGGER_EXTENSION_PATH=${PROJECT_SOURCE_DIR}/src/luajit_dbg.py"
+)
+
+set(TEST_SCRIPT_PATH
+  ${PROJECT_SOURCE_DIR}/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py
+)
+
+find_program(GDB gdb)
+if(GDB)
+  set(GDB_TEST_ENV ${DEBUGGER_TEST_ENV}
+    "DEBUGGER_COMMAND=${GDB}"
+  )
+  add_custom_command(TARGET LuaJIT-gdb-extension-tests
+    COMMENT "Running luajit_dbg.py tests with gdb"
+    COMMAND
+    ${GDB_TEST_ENV} ${PYTHON_EXECUTABLE} ${TEST_SCRIPT_PATH}
+    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+  )
+else()
+  message(WARNING "`gdb' is not found, so LuaJIT-gdb-extension-tests is dummy")
+endif()

I propose adding a message in dummy target, like we did in cmake/CodeCoverage.cmake:

    add_custom_target(${PROJECT_NAME}-coverage                                     
      COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${MSG}                    
    )    

Added, thanks!
+
+find_program(LLDB lldb)
+if(LLDB)
+  set(LLDB_TEST_ENV ${DEBUGGER_TEST_ENV}
+    "DEBUGGER_COMMAND=${LLDB}"
+  )
+  add_custom_command(TARGET LuaJIT-lldb-extension-tests
+    COMMENT "Running luajit_dbg.py tests with lldb"
+    COMMAND
+    ${LLDB_TEST_ENV} ${PYTHON_EXECUTABLE} ${TEST_SCRIPT_PATH}
+    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+  )
+else()
+  message(WARNING "`lldb' is not found, so LuaJIT-gdb-extension-tests is dummy")
typo: LuaJIT-lldb-extentsion-tests
Fixed, thanks!
+endif()
diff --git a/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py b/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py
new file mode 100644
index 00000000..6ef87473
--- /dev/null
+++ b/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py
@@ -0,0 +1,250 @@
+# This file provides tests for LuaJIT debug extensions for lldb and gdb.
+import os
+import re
+import subprocess
+import sys
+import tempfile
+import unittest
+
+from threading import Timer
+
+LEGACY = re.match(r'^2\.', sys.version)
+
+LUAJIT_BINARY = os.environ['LUAJIT_TEST_BINARY']
+EXTENSION = os.environ['DEBUGGER_EXTENSION_PATH']
+DEBUGGER = os.environ['DEBUGGER_COMMAND']
+LLDB = 'lldb' in DEBUGGER
+TIMEOUT = 10
skip or even fail all testcases if at least one required env variable is missed
Well, unittest is not TAP, so there is no easy way to skip all test cases. That’s not an issue anyway,
since if any variable is missing then the `KeyError` is raised and the suite fails. Ignoring.
+
+RUN_CMD_FILE = '-s' if LLDB else '-x'
+INFERIOR_ARGS = '--' if LLDB else '--args'
+PROCESS_RUN = 'process launch' if LLDB else 'r'
+LOAD_EXTENSION = (
+    'command script import {ext}' if LLDB else 'source {ext}'
+).format(ext=EXTENSION)
+
+
+def persist(data):
+    tmp = tempfile.NamedTemporaryFile(mode='w')
+    tmp.write(data)
+    tmp.flush()
+    return tmp
+
+
+def execute_process(cmd, timeout=TIMEOUT):
+    if LEGACY:
+        # XXX: The Python 2.7 version of `subprocess.Popen` doesn't have a
+        # timeout option, so the required functionality was implemented via
+        # `threading.Timer`.
+        process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
why stderr is ignored? non-empy stderr is a bad symptom
Fixed, thanks!
+        timer = Timer(TIMEOUT, process.kill)
+        timer.start()
+        stdout, _ = process.communicate()
+        timer.cancel()
+
+        # XXX: If the timeout is exceeded and the process is killed by the
+        # timer, then the return code is non-zero, and we are going to blow up.
+        assert process.returncode == 0
+        return stdout.decode('ascii')
+    else:
+        process = subprocess.run(cmd, capture_output=True, timeout=TIMEOUT)
+        return process.stdout.decode('ascii')
+
+
+def filter_debugger_output(output):
+    descriptor = '(lldb)' if LLDB else '(gdb)'
+    return ''.join(
+        filter(
+            lambda line: not line.startswith(descriptor),
+            output.splitlines(True),
+        ),
+    )
+
+
+class TestCaseBase(unittest.TestCase):
+    @classmethod
+    def construct_cmds(cls):
+        return '\n'.join([
+            'b {loc}'.format(loc=cls.location),
+            PROCESS_RUN,
+            'n',
+            LOAD_EXTENSION,
+            cls.extension_cmds.strip(),
+            'q',
+        ])
+
+    @classmethod
+    def setUpClass(cls):
+        cmd_file = persist(cls.construct_cmds())
+        script_file = persist(cls.lua_script)
+        process_cmd = [
+            DEBUGGER,
+            RUN_CMD_FILE,
+            cmd_file.name,
+            INFERIOR_ARGS,
+            LUAJIT_BINARY,
+            script_file.name,
+        ]
+        cls.output = filter_debugger_output(execute_process(process_cmd))
+        cmd_file.close()
+        script_file.close()
+
+    def check(self):
+        if LEGACY:
+            self.assertRegexpMatches(self.output, self.pattern.strip())
+        else:
+            self.assertRegex(self.output, self.pattern.strip())

message from assertRegex is totally unreadable:

======================================================================
FAIL: test (__main__.TestLoad)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sergeyb/sources/MRG/tarantool/third_party/luajit/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py", line 247, in <lambda>
    test_cls.test = lambda self: self.check()
  File "/home/sergeyb/sources/MRG/tarantool/third_party/luajit/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py", line 96, in check
    self.assertRegex(self.output, self.pattern.strip())
AssertionError: Regex didn't match: 'lj-tv command intialized\nlj-state command intialized\nlj-arch command intialized\nlj-gc command intialized\nlj-str command intialized\nlj-tab command intialized\nlj-stack command intialized\nLuaJIT debug extension is successfully loaded' not found in 'GNU gdb (Ubuntu 12.1-0ubuntu1~22.04) 12.1\nCopyright (C) 2022 Free Software Foundation, Inc.\nLicense GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>\nThis is free software: you are free to change and redistribute it.\nThere is NO WARRANTY, to the extent permitted by law.\nType "show copying" and "show warranty" for details.\nThis GDB was configured as "x86_64-linux-gnu".\nType "show configuration" for configuration details.\nFor bug reporting instructions, please see:\n<https://www.gnu.org/software/gdb/bugs/>.\nFind the GDB manual and other documentation resources online at:\n    <http://www.gnu.org/software/gdb/documentation/>.\n\nFor help, type "help".\nType "apropos word" to search for commands related to "word"...\nReading symbols from /home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/src/luajit...\nBreakpoint 1 at 0x2e640: file /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lib_base.c, line 496.\n[Thread debugging using libthread_db enabled]\nUsing host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".\n\nBreakpoint 1, lj_cf_print (L=0x55555555c8f0 <pmain>) at /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lib_base.c:496\n496\t{\n497\t  ptrdiff_t i, nargs = L->top - L->base;\n'

I’ve implemented my own check based on unittest’s public API, so now it looks nicer. Thanks!

 

+
+
+class TestLoad(TestCaseBase):
+    extension_cmds = ''
+    location = 'lj_cf_print'
+    lua_script = 'print(1)'
+    pattern = (
+        'lj-tv command intialized\n'
+        'lj-state command intialized\n'
+        'lj-arch command intialized\n'
+        'lj-gc command intialized\n'
+        'lj-str command intialized\n'
+        'lj-tab command intialized\n'
+        'lj-stack command intialized\n'
+        'LuaJIT debug extension is successfully loaded\n'
+    )
+
+
+class TestLJArch(TestCaseBase):
+    extension_cmds = 'lj-arch'
+    location = 'lj_cf_print'
+    lua_script = 'print(1)'
+    pattern = (
+        'LJ_64: (True|False), '
+        'LJ_GC64: (True|False), '
+        'LJ_DUALNUM: (True|False)'
+    )
+
+
+class TestLJState(TestCaseBase):
+    extension_cmds = 'lj-state'
+    location = 'lj_cf_print'
+    lua_script = 'print(1)'
+    pattern = (
+        'VM state: [A-Z]+\n'
+        'GC state: [A-Z]+\n'
+        'JIT state: [A-Z]+\n'
+    )
+
+
+class TestLJGC(TestCaseBase):
+    extension_cmds = 'lj-gc'
+    location = 'lj_cf_print'
+    lua_script = 'print(1)'
+    pattern = (
+        'GC stats: [A-Z]+\n'
+        '\ttotal: \d+\n'
+        '\tthreshold: \d+\n'
+        '\tdebt: \d+\n'
+        '\testimate: \d+\n'
+        '\tstepmul: \d+\n'
+        '\tpause: \d+\n'
+        '\tsweepstr: \d+/\d+\n'
+        '\troot: \d+ objects\n'
+        '\tgray: \d+ objects\n'
+        '\tgrayagain: \d+ objects\n'
+        '\tweak: \d+ objects\n'
+        '\tmmudata: \d+ objects\n'
+    )
+
+
+class TestLJStack(TestCaseBase):
+    extension_cmds = 'lj-stack'
+    location = 'lj_cf_print'
+    lua_script = 'print(1)'
+    pattern = (
+        '-+ Red zone:\s+\d+ slots -+\n'
+        '(0x[a-zA-Z0-9]+\s+\[(S|\s)(B|\s)(T|\s)(M|\s)\] VALUE: nil\n?)*\n'
+        '-+ Stack:\s+\d+ slots -+\n'
+        '(0x[A-Za-z0-9]+(:0x[A-Za-z0-9]+)?\s+'
+        '\[(S|\s)(B|\s)(T|\s)(M|\s)\].*\n?)+\n'
+    )
+
+
+class TestLJTV(TestCaseBase):
+    location = 'lj_cf_print'
+    lua_script = 'print(1)'
+    extension_cmds = (
+        'lj-tv L->base\n'
+        'lj-tv L->base + 1\n'
+        'lj-tv L->base + 2\n'
+        'lj-tv L->base + 3\n'
+        'lj-tv L->base + 4\n'
+        'lj-tv L->base + 5\n'
+        'lj-tv L->base + 6\n'
+        'lj-tv L->base + 7\n'
+        'lj-tv L->base + 8\n'
+        'lj-tv L->base + 9\n'
+        'lj-tv L->base + 10\n'
+        'lj-tv L->base + 11\n'
+    )
+
+    lua_script = (
+        'local ffi = require("ffi")\n'
+        'print(\n'
+        '  nil,\n'
+        '  false,\n'
+        '  true,\n'
+        '  "hello",\n'
+        '  {1},\n'
+        '  1,\n'
+        '  1.1,\n'
+        '  coroutine.create(function() end),\n'
+        '  ffi.new("int*"),\n'
+        '  function() end,\n'
+        '  print,\n'
+        '  require\n'
+        ')\n'
+    )
+
+    pattern = (
+        'nil\n'
+        'false\n'
+        'true\n'
+        'string \"hello\" @ 0x[a-zA-Z0-9]+\n'
+        'table @ 0x[a-zA-Z0-9]+ \(asize: \d+, hmask: 0x[a-zA-Z0-9]+\)\n'
+        '(number|integer) .*1.*\n'
+        'number 1.1\d+\n'
+        'thread @ 0x[a-zA-Z0-9]+\n'
+        'cdata @ 0x[a-zA-Z0-9]+\n'
+        'Lua function @ 0x[a-zA-Z0-9]+, [0-9]+ upvalues, .+:[0-9]+\n'
+        'fast function #[0-9]+\n'
+        'C function @ 0x[a-zA-Z0-9]+\n'
+    )
+
+
+class TestLJStr(TestCaseBase):
+    extension_cmds = 'lj-str fname'
+    location = 'lj_cf_dofile'
+    lua_script = 'pcall(dofile("name"))'
+    pattern = 'String: .* \[\d+ bytes\] with hash 0x[a-zA-Z0-9]+'
+
+
+class TestLJTab(TestCaseBase):
+    extension_cmds = 'lj-tab t'
+    location = 'lj_cf_unpack'
+    lua_script = 'unpack({1; a = 1})'
+    pattern = (
+        'Array part: 3 slots\n'
+        '0x[a-zA-Z0-9]+: \[0\]: nil\n'
+        '0x[a-zA-Z0-9]+: \[1\]: .+ 1\n'
+        '0x[a-zA-Z0-9]+: \[2\]: nil\n'
+        'Hash part: 2 nodes\n'
+        '0x[a-zA-Z0-9]+: { string "a" @ 0x[a-zA-Z0-9]+ } => '
+        '{ .+ 1 }; next = 0x0\n'
+        '0x[a-zA-Z0-9]+: { nil } => { nil }; next = 0x0\n'
+    )
+
+
+for test_cls in TestCaseBase.__subclasses__():
+    test_cls.test = lambda self: self.check()
+
+if __name__ == '__main__':
+    unittest.main(verbosity=2)