Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Maksim Kokryashkin <max.kokryashkin@gmail.com>,
	tarantool-patches@dev.tarantool.org, skaplun@tarantool.org,
	m.kokryashkin@tarantool.org, imun@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit v3 2/2] test: add tests for debugging extensions
Date: Wed, 22 Nov 2023 17:32:05 +0300	[thread overview]
Message-ID: <014cfe18-71eb-4d99-866f-056d6787526f@tarantool.org> (raw)
In-Reply-To: <20231110201612.81722-3-max.kokryashkin@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 16563 bytes --]

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

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

> +  # 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}
     )

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

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

[-- Attachment #2: Type: text/html, Size: 18251 bytes --]

  reply	other threads:[~2023-11-22 14:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-10 20:16 [Tarantool-patches] [PATCH luajit v3 0/2] debug: generalized extension Maksim Kokryashkin via Tarantool-patches
2023-11-10 20:16 ` [Tarantool-patches] [PATCH luajit v3 1/2] " Maksim Kokryashkin via Tarantool-patches
2023-11-22 14:46   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 13:58     ` Maxim Kokryashkin via Tarantool-patches
2023-11-10 20:16 ` [Tarantool-patches] [PATCH luajit v3 2/2] test: add tests for debugging extensions Maksim Kokryashkin via Tarantool-patches
2023-11-22 14:32   ` Sergey Bronnikov via Tarantool-patches [this message]
2023-12-06 14:03     ` Maxim Kokryashkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=014cfe18-71eb-4d99-866f-056d6787526f@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit v3 2/2] test: add tests for debugging extensions' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox