From 7dd63c891f648f2e42a48607d25f861f5c084fa5 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 29 Aug 2024 17:59:30 +0800 Subject: [PATCH 1/2] scylla-gdb.py: lazy-evaluate the constants instead of evaluating the constants in-class, accessing them via a cached class property. it would be handy if we could source `scylla-gdb.py` in `.gdbinit`, but this script accesses some symbols which are not available with a file being debugged. so when gdb fails to load init script: ``` Traceback (most recent call last): File "/home/kefu/dev/scylladb/scylla-gdb.py", line 167, in class intrusive_slist: File "/home/kefu/dev/scylladb/scylla-gdb.py", line 168, in intrusive_slist size_t = gdb.lookup_type('size_t') ^^^^^^^^^^^^^^^^^^^^^^^^^ gdb.error: No type named size_t. ``` so we have to `file path/to/scylla` and *then* `source scylla-gdb.py` every time when we debug scylla or a seastar application, instead of loading `scylla-gdb.py` in `.gdbinit`. the reason is that the script access the debug symbols like `gdb.lookup_type('size_t')` in-class. so when the python interpreter reads the script, it evaluates this statement, but at that moment, the debug symbols are not loaded, so `source scylla-gdb.py` fails in `.gdbinit`. in this change, we transform all these class variables to cached property, so that they * are evaluated on-demand * are evaluated only once at most this addresses the pain at the expense of verbosity. Signed-off-by: Kefu Chai --- scylla-gdb.py | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/scylla-gdb.py b/scylla-gdb.py index 9c3a2e9c3c..96d6c3e62a 100755 --- a/scylla-gdb.py +++ b/scylla-gdb.py @@ -6,6 +6,7 @@ import gdb.printing import uuid import argparse import datetime +import functools import re from operator import attrgetter from collections import defaultdict @@ -118,7 +119,11 @@ def downcast_vptr(ptr): class intrusive_list: - size_t = gdb.lookup_type('size_t') + @classmethod + @property + @functools.cache + def size_t(cls): + return gdb.lookup_type('size_t') def __init__(self, list_ref, link=None): list_type = list_ref.type.strip_typedefs() @@ -160,7 +165,11 @@ class intrusive_list: class intrusive_slist: - size_t = gdb.lookup_type('size_t') + @classmethod + @property + @functools.cache + def size_t(cls): + return gdb.lookup_type('size_t') def __init__(self, list_ref, link=None): list_type = list_ref.type.strip_typedefs() @@ -240,7 +249,11 @@ class std_tuple: class intrusive_set: - size_t = gdb.lookup_type('size_t') + @classmethod + @property + @functools.cache + def size_t(cls): + return gdb.lookup_type('size_t') def __init__(self, ref, link=None): container_type = ref.type.strip_typedefs() @@ -428,7 +441,11 @@ class std_variant: class std_map: - size_t = gdb.lookup_type('size_t') + @classmethod + @property + @functools.cache + def size_t(cls): + return gdb.lookup_type('size_t') def __init__(self, ref): container_type = ref.type.strip_typedefs() @@ -3890,9 +3907,14 @@ class scylla_fiber(gdb.Command): Invoke `scylla fiber --help` for more information on usage. """ + @classmethod + @property + @functools.cache + def _vptr_type(cls): + return gdb.lookup_type('uintptr_t').pointer() + def __init__(self): gdb.Command.__init__(self, 'scylla fiber', gdb.COMMAND_USER, gdb.COMPLETE_NONE, True) - self._vptr_type = gdb.lookup_type('uintptr_t').pointer() self._task_symbol_matcher = task_symbol_matcher() self._thread_map = None @@ -4174,7 +4196,12 @@ class scylla_find(gdb.Command): thread 1, small (size <= 512), live (0x6000000f3800 +48) thread 1, small (size <= 56), live (0x6000008a1230 +32) """ - _vptr_type = gdb.lookup_type('uintptr_t').pointer() + @classmethod + @property + @functools.cache + def _vptr_type(cls): + return gdb.lookup_type('uintptr_t').pointer() + _size_char_to_size = { 'b': 8, 'h': 16, From 82fbe317ec8e81c70be9bb3b260145a063744538 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 30 Aug 2024 15:38:10 +0800 Subject: [PATCH 2/2] test/scylla_gdb: test the .gdb init use case before this change, we run all the tests in a single pytest session, with scylladb debug symbols loaded. but we want to test another use case, where the scylladb debug symbols are missing. in this change, * we do not check for the existence of debug symbols until necessary * add a mark named "without_scylla" * run the tests in two pytest sessions - one with "without_scylla" mark - one with "not without_scylla" mark * add a test which is marked with the "without_scylla" mark. the test verify that the scylla-gdb.py script can be loaded even without scylladb debug symbols. Signed-off-by: Kefu Chai --- test/scylla_gdb/conftest.py | 15 ++++++++------- test/scylla_gdb/pytest.ini | 2 ++ test/scylla_gdb/run | 30 ++++++++++++++++++++++-------- test/scylla_gdb/test_misc.py | 14 ++++++++++++++ 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/test/scylla_gdb/conftest.py b/test/scylla_gdb/conftest.py index 3eda3510b6..a2570365cf 100644 --- a/test/scylla_gdb/conftest.py +++ b/test/scylla_gdb/conftest.py @@ -11,6 +11,7 @@ import pytest import os +import sys try: import gdb as gdb_library @@ -18,12 +19,6 @@ except: print('This test must be run inside gdb. Run ./run instead.') exit(1) -try: - gdb_library.lookup_type('size_t') -except: - print(f'ERROR: Scylla executable was compiled without debugging information (-g)') - print(f'so cannot be used to test gdb. Please set SCYLLA environment variable.') - exit(1) def pytest_addoption(parser): parser.addoption('--scylla-pid', action='store', default=None, @@ -40,7 +35,6 @@ def pytest_addoption(parser): # object. @pytest.fixture(scope="session") def scylla_gdb(request): - import sys save_sys_path = sys.path sys.path.insert(1, sys.path[0] + '/../..') # Unfortunately, the file's name includes a dash which requires some @@ -59,6 +53,13 @@ def scylla_gdb(request): # subcommands are loaded into gdb. @pytest.fixture(scope="session") def gdb(request, scylla_gdb): + try: + gdb_library.lookup_type('size_t') + except: + print('ERROR: Scylla executable was compiled without debugging information (-g)') + print('so cannot be used to test gdb. Please set SCYLLA environment variable.') + sys.exit(1) + # The gdb tests are known to be broken on aarch64 (see # https://sourceware.org/bugzilla/show_bug.cgi?id=27886) and untested # on anything else. So skip them. diff --git a/test/scylla_gdb/pytest.ini b/test/scylla_gdb/pytest.ini index a979c80785..e391932780 100644 --- a/test/scylla_gdb/pytest.ini +++ b/test/scylla_gdb/pytest.ini @@ -2,3 +2,5 @@ # pytest will look for one in our ancestor directories, and may find # something irrelevant. So we should have one here, even if empty. [pytest] +markers = + without_scylla: run without attaching to a scylla process diff --git a/test/scylla_gdb/run b/test/scylla_gdb/run index d0d5ca65ba..a2d986c55b 100755 --- a/test/scylla_gdb/run +++ b/test/scylla_gdb/run @@ -32,7 +32,7 @@ run.wait_for_services(pid, [lambda: run.check_cql(ip)]) # run pytest inside gdb - and instead run gdb as a separate process and # pytest just sends commands to it. # TODO: think if we can avoid code duplication with run.run_ptest(). -def run_pytest_in_gdb(pytest_dir, additional_parameters): +def run_pytest_in_gdb(pytest_dir, executable, additional_parameters): sys.stdout.flush() sys.stderr.flush() pid = os.fork() @@ -43,13 +43,16 @@ def run_pytest_in_gdb(pytest_dir, additional_parameters): os.chdir(pytest_dir) pytest_args = ['-o', 'junit_family=xunit2'] + additional_parameters pytest_cmd = f'print("Starting pytest {" ".join(pytest_args)}"); import pytest; sys.argv[0]="pytest"; sys.exit(pytest.main({str(pytest_args)}))' - print(f'Starting gdb {run.scylla}') + print(f'Starting gdb {executable}') sys.stdout.flush() - os.execvp('gdb', ['gdb', - '-batch', '-n', '-se', run.scylla, - '-ex', 'set python print-stack full', - '-ex', 'python ' + pytest_cmd, - ]) + args = ['gdb', + '-batch', '-n', + '-ex', 'set python print-stack full', + '-ex', 'python ' + pytest_cmd, + ] + if executable: + args += ['-se', executable] + os.execvp('gdb', args) exit(1) # parent: run.run_pytest_pids.add(pid) @@ -65,7 +68,18 @@ def run_pytest_in_gdb(pytest_dir, additional_parameters): # command line, saves it as "initialpaths", and uses it to print what it # thinks are nice (but are really incorrect) relative paths for "nodes" (test # source files). -success = run_pytest_in_gdb(sys.path[0], ['--scylla-pid='+str(pid), '--scylla-tmp-dir='+run.pid_to_dir(pid)] + sys.argv[1:]) +success = True +for with_scylla in [True, False]: + if with_scylla: + args = ['--scylla-pid='+str(pid), + '--scylla-tmp-dir='+run.pid_to_dir(pid), + '-m', 'not without_scylla'] + executable = run.scylla + else: + args = ['-m', 'without_scylla'] + executable = '' + if not run_pytest_in_gdb(sys.path[0], executable, args + sys.argv[1:]): + success = False run.summary = 'Scylla GDB tests pass' if success else 'Scylla GDB tests failure' diff --git a/test/scylla_gdb/test_misc.py b/test/scylla_gdb/test_misc.py index 0b397ca70f..c673a7eeb0 100644 --- a/test/scylla_gdb/test_misc.py +++ b/test/scylla_gdb/test_misc.py @@ -190,4 +190,18 @@ def test_read_stats(gdb, sstable): def test_get_config_value(gdb): scylla(gdb, f'get-config-value compaction_static_shares') +@pytest.mark.without_scylla +def test_run_without_scylla(scylla_gdb): + # just try to load the scylla-gdb module without attaching to scylla. + # + # please note, if this test fails, there are good chances that scylla-gdb.py + # is unable to load without debug symbols. Calls to "gdb.lookup_type()" and + # similar functions that rely on debug symbols should be made within GDB + # commands themselves when they get exuecuted. To address potential + # failures, consider moving code that references debug symbols into a code + # path executed only when debug symbols is loaded. If the value of the + # symbol is a constant, consider caching it. using functools.cache + # decorator. + _ = scylla_gdb + # FIXME: need a simple test for lsa-segment