From d7c2a8f12e95f4e25b0c6c960d0c1578b0e7fd1f Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 28 Jul 2022 10:13:30 -0400 Subject: [PATCH 01/26] pylintrc: disable too-many-lines --- tests/pylintrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pylintrc b/tests/pylintrc index ba13178f0f4..b4358b2fb7e 100644 --- a/tests/pylintrc +++ b/tests/pylintrc @@ -65,7 +65,7 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -disable=missing-docstring,too-few-public-methods,invalid-name,too-many-locals,too-many-instance-attributes,too-many-public-methods,fixme,no-self-use,too-many-branches,too-many-statements,too-many-arguments,too-many-boolean-expressions,line-too-long,duplicate-code,bad-option-value +disable=missing-docstring,too-few-public-methods,invalid-name,too-many-locals,too-many-instance-attributes,too-many-public-methods,fixme,no-self-use,too-many-branches,too-many-statements,too-many-arguments,too-many-boolean-expressions,line-too-long,duplicate-code,bad-option-value,too-many-lines [REPORTS] From 86ef52e16dac57488faf6dae9c547eeb474e769a Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 11:10:16 -0400 Subject: [PATCH 02/26] crash.sh: always recreate gdbinit file gdbinit was always appended when used out of the git tree, which would lead to odd failures Signed-off-by: Jeff Mahoney --- crash.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crash.sh b/crash.sh index ed258f47748..abe0f9d3531 100755 --- a/crash.sh +++ b/crash.sh @@ -163,6 +163,8 @@ if [ -z "$GDB" ]; then exit 1 fi +:> $GDBINIT + # If we're using crash.sh from the git repo, use the modules from the git repo DIR="$(dirname $0)" if [ -e "$DIR/setup.py" ]; then @@ -183,7 +185,6 @@ if [ -e "$DIR/setup.py" ]; then done else export CRASH_PYTHON_HELP="/usr/share/crash-python/help" - :> $GDBINIT TEST_GDBINIT="/usr/share/crash-python/test-gdb-compatibility.gdbinit" fi From d669cd17a5def48d4274e997c90faddcd4d6f86a Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 11:13:34 -0400 Subject: [PATCH 03/26] crash.sh: Allow specifying gdb command line via $GDB_CMDLINE environment var When developing gdb extensions, it's helpful to be able to use a gdb that hasn't been fully installed. This means that the --data-directory option must be passed as well. Signed-off-by: Jeff Mahoney --- crash.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crash.sh b/crash.sh index abe0f9d3531..8276c928ac4 100755 --- a/crash.sh +++ b/crash.sh @@ -188,7 +188,7 @@ else TEST_GDBINIT="/usr/share/crash-python/test-gdb-compatibility.gdbinit" fi -if ! $GDB -nx -batch -x $GDBINIT -x $TEST_GDBINIT; then +if ! $GDB $GDB_CMDLINE -nx -batch -x $GDBINIT -x $TEST_GDBINIT; then echo "fatal: crash-python cannot initialize" >&2 exit 1 fi @@ -296,12 +296,12 @@ EOF # This is how we debug gdb problems when running crash if [ "$DEBUGMODE" = "gdb" ]; then - RUN="run -nx -q -x $GDBINIT" + RUN="run $GDB_CMDLINE -nx -q -x $GDBINIT" echo $RUN > $TMPDIR/gdbinit-debug gdb $GDB -nx -q -x $TMPDIR/gdbinit-debug elif [ "$DEBUGMODE" = "valgrind" ]; then valgrind --keep-stacktraces=alloc-and-free $GDB -nh -q -x $GDBINIT else - $GDB -nx -q -x $GDBINIT + $GDB $GDB_CMDLINE -nx -q -x $GDBINIT fi From 3bb193efec2881ac04a7b796f2ba495f7ff98f39 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 16:03:42 -0400 Subject: [PATCH 04/26] tests: allow use of custom gdb and command line Similar to running pycrash with a gdb under development, being able to use it to run the tests is helpful as well. Signed-off-by: Jeff Mahoney --- tests/run-gdb.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/run-gdb.sh b/tests/run-gdb.sh index b09f53b7df4..26f8b8e3af2 100755 --- a/tests/run-gdb.sh +++ b/tests/run-gdb.sh @@ -1,5 +1,10 @@ #!/bin/bash DIR=$(dirname "$0") + +if test -z "$GDB"; then + GDB=crash-python-gdb +fi + echo "Starting gdb" -exec crash-python-gdb -nx -batch -x $DIR/gdbinit-boilerplate "$@" +exec $GDB $GDB_CMDLINE -nx -batch -x $DIR/gdbinit-boilerplate "$@" From d059a7a994efba4d9068f8018cf4dd3b1c9523a0 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 15:57:44 -0400 Subject: [PATCH 05/26] lockless_ringbuffer: add type annotations Add missing type annotations for is_finalized and is_reusable. Signed-off-by: Jeff Mahoney --- crash/subsystem/printk/lockless_ringbuffer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crash/subsystem/printk/lockless_ringbuffer.py b/crash/subsystem/printk/lockless_ringbuffer.py index 736281d3d33..355e7a40fe9 100644 --- a/crash/subsystem/printk/lockless_ringbuffer.py +++ b/crash/subsystem/printk/lockless_ringbuffer.py @@ -92,11 +92,11 @@ def desc_state(self) -> int: ''' Return state of the descriptor ''' return (self.state_var & self.sv_mask) >> self.sv_shift - def is_finalized(self): + def is_finalized(self) -> bool: ''' Finalized desriptor points to a valid (deta) message ''' return self.desc_state() == 0x2 - def is_reusable(self): + def is_reusable(self) -> bool: ''' Reusable descriptor still has a valid sequence number but the data are gone. From cf0f1156f2e129009359f368a44a1e29067bb370 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Wed, 27 Jul 2022 23:08:08 -0400 Subject: [PATCH 06/26] lockless_ringbuffer: fix lint issues --- crash/subsystem/printk/lockless_ringbuffer.py | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/crash/subsystem/printk/lockless_ringbuffer.py b/crash/subsystem/printk/lockless_ringbuffer.py index 355e7a40fe9..bb6362c0675 100644 --- a/crash/subsystem/printk/lockless_ringbuffer.py +++ b/crash/subsystem/printk/lockless_ringbuffer.py @@ -1,15 +1,13 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -from typing import Dict, Iterable, Any - import argparse -import sys + import gdb from crash.util.symbols import Types, Symvals from crash.exceptions import DelayedAttributeError -from crash.subsystem.printk import LogTypeException, LogInvalidOption +from crash.subsystem.printk import LogTypeException types = Types(['struct printk_info *', 'struct prb_desc *', @@ -131,10 +129,10 @@ def get_data_block(self, blk_lpos: PrbDataBlkLPos) -> PrbDataBlock: blk_p = self.data.cast(types.char_p_type) + begin_idx return PrbDataBlock(blk_p.cast(types.prb_data_block_p_type)) - def get_text(self, blk_lpos: PrbDataBlkLPos, len: int) -> str: + def get_text(self, blk_lpos: PrbDataBlkLPos, _len: int) -> str: ''' return string stored at the given blk_lpos ''' data_block = self.get_data_block(blk_lpos) - return data_block.data.cast(types.char_p_type).string(length=len) + return data_block.data.cast(types.char_p_type).string(length=_len) class PrbDescRing: @@ -154,20 +152,20 @@ def __init__(self, dr: gdb.Value) -> None: self.tail_id = atomic_long_read(dr['tail_id']) self.mask_id = (1 << self.count_bits) - 1 - def get_idx(self, id: int) -> int: + def get_idx(self, _id: int) -> int: ''' Return index to the desc ring for the given id ''' - return id & self.mask_id + return _id & self.mask_id - def get_desc(self, id: int) -> PrbDesc: + def get_desc(self, _id: int) -> PrbDesc: ''' Return prb_desc structure for the given id ''' - idx = self.get_idx(id) + idx = self.get_idx(_id) desc_p = (self.descs.cast(types.char_p_type) + types.prb_desc_p_type.target().sizeof * idx) return PrbDesc(desc_p.cast(types.prb_desc_p_type)) - def get_info(self, id: int) -> PrintkInfo: + def get_info(self, _id: int) -> PrintkInfo: ''' return printk_info structure for the given id ''' - idx = self.get_idx(id) + idx = self.get_idx(_id) info_p = (self.infos.cast(types.char_p_type) + types.printk_info_p_type.target().sizeof * idx) return PrintkInfo(info_p.cast(types.printk_info_p_type)) @@ -184,10 +182,10 @@ def __init__(self, prb: gdb.Value) -> None: def is_valid_desc(self, desc: PrbDesc, info: PrintkInfo, seq: int) -> bool: ''' Does the descritor constains consistent values? ''' - if (not (desc.is_finalized() or desc.is_reusable())): + if not (desc.is_finalized() or desc.is_reusable()): return False # Must match the expected seq number. Otherwise is being updated. - return (info.seq == seq) + return info.seq == seq def first_seq(self) -> int: ''' @@ -202,11 +200,11 @@ def first_seq(self) -> int: # As a result, the valid sequence number should be either in tail_id # or tail_id + 1 entry. for i in range(0, 1): - id = self.desc_ring.tail_id + i - desc = self.desc_ring.get_desc(id) + _id = self.desc_ring.tail_id + i + desc = self.desc_ring.get_desc(_id) - if (desc.is_finalized() or desc.is_reusable()): - info = self.desc_ring.get_info(id) + if desc.is_finalized() or desc.is_reusable(): + info = self.desc_ring.get_info(_id) return info.seq # Something went wrong. Do not continue with an invalid sequence number. @@ -230,13 +228,13 @@ def show_msg(self, desc: PrbDesc, info: PrintkInfo, level = '<{:d}>'.format(info.level) text = self.data_ring.get_text(desc.text_blk_lpos, info.text_len) - print('{}{}{}'.format(level,timestamp,text)) + print('{}{}{}'.format(level, timestamp, text)) - if (args.d): + if args.d: # Only two dev_info values are supported at the moment - if (len(info.dev_info.subsystem)): + if info.dev_info.subsystem: print(' SUBSYSTEM={}'.format(info.dev_info.subsystem)) - if (len(info.dev_info.device)): + if info.dev_info.device: print(' DEVICE={}'.format(info.dev_info.device)) def show_log(self, args: argparse.Namespace) -> None: @@ -247,7 +245,7 @@ def show_log(self, args: argparse.Namespace) -> None: while True: desc = self.desc_ring.get_desc(seq) info = self.desc_ring.get_info(seq) - if (not self.is_valid_desc(desc, info, seq)): + if not self.is_valid_desc(desc, info, seq): break seq += 1 @@ -255,7 +253,7 @@ def show_log(self, args: argparse.Namespace) -> None: # Sequence numbers are stored in separate ring buffer. # The descriptor ring might include valid sequence numbers # but the data might already be replaced. - if (desc.is_reusable()): + if desc.is_reusable(): continue self.show_msg(desc, info, args) @@ -273,10 +271,8 @@ def lockless_rb_show(args: argparse.Namespace) -> None: """ try: - test = symvals.prb + prb = PrbRingBuffer(symvals.prb) except DelayedAttributeError: raise LogTypeException('not lockless log') from None - prb = PrbRingBuffer(symvals.prb) - prb.show_log(args) From 47a18e4b64f407023c81b47ce2c3697faaf75a00 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 28 Jul 2022 00:31:45 -0400 Subject: [PATCH 07/26] structured_ringbuffer: fix lint warnings --- crash/subsystem/printk/structured_ringbuffer.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crash/subsystem/printk/structured_ringbuffer.py b/crash/subsystem/printk/structured_ringbuffer.py index 456672c10e4..2ce233d388f 100644 --- a/crash/subsystem/printk/structured_ringbuffer.py +++ b/crash/subsystem/printk/structured_ringbuffer.py @@ -31,7 +31,7 @@ def log_from_idx(logbuf: gdb.Value, idx: int) -> Dict: dictval = (msg.cast(types.char_p_type) + types.printk_log_p_type.target().sizeof + textlen) - dict = dictval.string(length=dictlen) + msgdict = dictval.string(length=dictlen) msglen = int(msg['len']) @@ -41,16 +41,14 @@ def log_from_idx(logbuf: gdb.Value, idx: int) -> Dict: else: nextidx = idx + msglen - msgdict = { + return { 'text' : text[0:textlen], 'timestamp' : int(msg['ts_nsec']), 'level' : int(msg['level']), 'next' : nextidx, - 'dict' : dict[0:dictlen], + 'dict' : msgdict[0:dictlen], } - return msgdict - def get_log_msgs() -> Iterable[Dict[str, Any]]: try: idx = symvals.log_first_idx @@ -88,5 +86,5 @@ def structured_rb_show(args: argparse.Namespace) -> None: print('{}{}{}'.format(level, timestamp, line)) if (args.d and msg['dict']): - for dict in msg['dict'].split('\0'): - print(' {}'.format(dict)) + for entry in msg['dict'].split('\0'): + print(' {}'.format(entry)) From d6b8bb20311a492527f5f15492bd15296aa7f47c Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Wed, 27 Jul 2022 23:32:33 -0400 Subject: [PATCH 08/26] lint: fix whitespace This commit only modifies whitespace to make pylint happy. Signed-off-by: Jeff Mahoney --- crash/arch/x86_64.py | 2 +- crash/commands/dev.py | 4 ++-- crash/subsystem/storage/__init__.py | 3 +-- crash/subsystem/storage/blockmq.py | 2 +- crash/types/slab.py | 8 ++++---- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/crash/arch/x86_64.py b/crash/arch/x86_64.py index 58c6be880c8..73cc415e28a 100644 --- a/crash/arch/x86_64.py +++ b/crash/arch/x86_64.py @@ -106,7 +106,7 @@ class x86_64Architecture(CrashArchitecture): ident = "i386:x86-64" aliases = ["x86_64"] - _frame_offset : Optional[int] = None + _frame_offset: Optional[int] = None def __init__(self) -> None: super(x86_64Architecture, self).__init__() diff --git a/crash/commands/dev.py b/crash/commands/dev.py index 5a6ebdc1031..d388eaf5687 100644 --- a/crash/commands/dev.py +++ b/crash/commands/dev.py @@ -17,14 +17,14 @@ class DevCommand(Command): """display character and block devices""" - def __init__(self, name : str) -> None: + def __init__(self, name: str) -> None: parser = ArgumentParser(prog=name) parser.add_argument('-d', action='store_true', default=False) super().__init__(name, parser) - def execute(self, args : argparse.Namespace) -> None: + def execute(self, args: argparse.Namespace) -> None: if args.d: print("{:^5} {:^16} {:^10} {:^16} {:^5} {:^5} {:^5} {:^5}" .format("MAJOR", "GENDISK", "NAME", "REQUEST_QUEUE", diff --git a/crash/subsystem/storage/__init__.py b/crash/subsystem/storage/__init__.py index 1ed6ec90b4c..0651522894d 100644 --- a/crash/subsystem/storage/__init__.py +++ b/crash/subsystem/storage/__init__.py @@ -367,7 +367,7 @@ def _rq_in_flight(request: gdb.Value) -> bool: elif struct_has_member(request_s, 'atomic_flags'): def _rq_in_flight(request: gdb.Value) -> bool: return (request['atomic_flags'] & - (1 << int(types.enum_rq_atomic_flags_type['REQ_ATOM_STARTED'].enumval)) != 0) + (1 << int(types.enum_rq_atomic_flags_type['REQ_ATOM_STARTED'].enumval)) != 0) else: def _rq_in_flight(request: gdb.Value) -> bool: return request['cmd_flags'] & REQ_STARTED != 0 # type: ignore @@ -377,4 +377,3 @@ def _rq_in_flight(request: gdb.Value) -> bool: type_cbs = TypeCallbacks([('struct device_type', _check_types), ('enum req_flag_bits', _export_req_flags), ('struct request', _check_struct_request)]) - diff --git a/crash/subsystem/storage/blockmq.py b/crash/subsystem/storage/blockmq.py index 407a3bd7903..b48455228f8 100644 --- a/crash/subsystem/storage/blockmq.py +++ b/crash/subsystem/storage/blockmq.py @@ -14,7 +14,7 @@ class NoQueueError(RuntimeError): pass types = Types(['struct request', 'struct request_queue', - 'struct sbitmap_queue', 'struct blk_mq_hw_ctx' ]) + 'struct sbitmap_queue', 'struct blk_mq_hw_ctx']) def _check_queue_type(queue: gdb.Value) -> None: if not queue_is_mq(queue): diff --git a/crash/types/slab.py b/crash/types/slab.py index 2ca38a6e059..af50e1eae44 100644 --- a/crash/types/slab.py +++ b/crash/types/slab.py @@ -213,7 +213,7 @@ class SlabSLAB(Slab): BUFCTL_END = ~0 & 0xffffffff - kmem_cache : 'KmemCacheSLAB' + kmem_cache: 'KmemCacheSLAB' def __init__(self, gdb_obj: gdb.Value, kmem_cache: 'KmemCacheSLAB', error: bool = False) -> None: @@ -501,7 +501,7 @@ def check(self, slabtype: int, nid: int) -> int: class SlabSLUB(Slab): - kmem_cache : 'KmemCacheSLUB' + kmem_cache: 'KmemCacheSLUB' def __init__(self, gdb_obj: gdb.Value, kmem_cache: 'KmemCacheSLUB') -> None: super().__init__(gdb_obj, kmem_cache) @@ -770,7 +770,7 @@ class KmemCacheSLAB(KmemCache): slab_list_name = {0: "partial", 1: "full", 2: "free"} slab_list_fullname = {0: "slabs_partial", 1: "slabs_full", 2: "slabs_free"} - buffer_size : int + buffer_size: int def __init__(self, name: str, gdb_obj: gdb.Value) -> None: super().__init__(name, gdb_obj) @@ -1012,7 +1012,7 @@ def ___check_slabs(self, node: gdb.Value, slabtype: int, nid: int, count = errors['num_ok'] if (count and errors['first_ok'] is not None and - errors['last_ok'] is not None): + errors['last_ok'] is not None): print(f"{errors['num_ok']} slab objects were ok between " f"0x{errors['first_ok']:x} and 0x{errors['last_ok']:x}") From 3170fa428f0cba4d6d75e05c22a99e886327858e Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 28 Jul 2022 16:24:49 -0400 Subject: [PATCH 09/26] crash.arch.x86_64: add typing for _scheduled_rip --- crash/arch/x86_64.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crash/arch/x86_64.py b/crash/arch/x86_64.py index 73cc415e28a..ea494422161 100644 --- a/crash/arch/x86_64.py +++ b/crash/arch/x86_64.py @@ -114,6 +114,8 @@ def __init__(self) -> None: # Stop stack traces with addresses below this self.filter = KernelFrameFilter(0xffff000000000000) + self._scheduled_rip: int + def setup_thread_info(self, thread: gdb.InferiorThread) -> None: task = thread.info.task_struct thread_info = task['stack'].cast(types.thread_info_p_type) @@ -159,7 +161,7 @@ def adjust_scheduled_frame_offset(self, rsp: gdb.Value) -> gdb.Value: return rsp + self._frame_offset return rsp - def get_scheduled_rip(self) -> None: + def get_scheduled_rip(self) -> int: return self._scheduled_rip @classmethod From 4951bea471e86c8e445cb2b049d5e9ab484bc6af Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 28 Jul 2022 00:22:21 -0400 Subject: [PATCH 10/26] lint: clean up imports --- crash/addrxlat.py | 4 ++-- crash/arch/x86_64.py | 4 ++-- crash/commands/dmesg.py | 5 ----- crash/subsystem/printk/plain_ringbuffer.py | 4 +--- crash/subsystem/printk/structured_ringbuffer.py | 2 +- crash/subsystem/storage/block.py | 1 - crash/types/sbitmap.py | 1 - 7 files changed, 6 insertions(+), 15 deletions(-) diff --git a/crash/addrxlat.py b/crash/addrxlat.py index 0638e7da532..568bf4cd0a5 100644 --- a/crash/addrxlat.py +++ b/crash/addrxlat.py @@ -1,9 +1,9 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -import gdb - import addrxlat + +import gdb import crash from crash.cache.syscache import utsname diff --git a/crash/arch/x86_64.py b/crash/arch/x86_64.py index ea494422161..40427cb41f2 100644 --- a/crash/arch/x86_64.py +++ b/crash/arch/x86_64.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -import gdb +from typing import Optional import re -from typing import Optional +import gdb from crash.arch import CrashArchitecture, KernelFrameFilter, register_arch from crash.arch import FetchRegistersCallback diff --git a/crash/commands/dmesg.py b/crash/commands/dmesg.py index 75348bb0437..c2d63868d2e 100644 --- a/crash/commands/dmesg.py +++ b/crash/commands/dmesg.py @@ -139,14 +139,9 @@ ... """ -from typing import Dict, Iterable, Any - import argparse -import gdb - from crash.commands import Command, ArgumentParser, CommandError -from crash.exceptions import DelayedAttributeError from crash.subsystem.printk import LogTypeException, LogInvalidOption from crash.subsystem.printk.lockless_ringbuffer import lockless_rb_show from crash.subsystem.printk.structured_ringbuffer import structured_rb_show diff --git a/crash/subsystem/printk/plain_ringbuffer.py b/crash/subsystem/printk/plain_ringbuffer.py index fd4174f3732..c8c8fc99528 100644 --- a/crash/subsystem/printk/plain_ringbuffer.py +++ b/crash/subsystem/printk/plain_ringbuffer.py @@ -4,10 +4,8 @@ import argparse import re -import gdb - from crash.util.symbols import Types, Symvals -from crash.subsystem.printk import LogTypeException, LogInvalidOption +from crash.subsystem.printk import LogInvalidOption types = Types(['char *']) symvals = Symvals(['log_buf', 'log_buf_len']) diff --git a/crash/subsystem/printk/structured_ringbuffer.py b/crash/subsystem/printk/structured_ringbuffer.py index 2ce233d388f..bf389f68193 100644 --- a/crash/subsystem/printk/structured_ringbuffer.py +++ b/crash/subsystem/printk/structured_ringbuffer.py @@ -9,7 +9,7 @@ from crash.util.symbols import Types, Symvals from crash.exceptions import DelayedAttributeError -from crash.subsystem.printk import LogTypeException, LogInvalidOption +from crash.subsystem.printk import LogTypeException types = Types(['struct printk_log *', 'char *']) symvals = Symvals(['log_buf', 'log_buf_len', 'log_first_idx', 'log_next_idx', diff --git a/crash/subsystem/storage/block.py b/crash/subsystem/storage/block.py index 2427a17a441..62abdf33ca2 100644 --- a/crash/subsystem/storage/block.py +++ b/crash/subsystem/storage/block.py @@ -5,7 +5,6 @@ import gdb -from crash.util.symbols import Types from crash.subsystem.storage import queue_is_mq from crash.subsystem.storage.blocksq import sq_for_each_request_in_queue, \ sq_requests_in_flight, sq_requests_queued diff --git a/crash/types/sbitmap.py b/crash/types/sbitmap.py index 33f0ad439af..dacef415aeb 100644 --- a/crash/types/sbitmap.py +++ b/crash/types/sbitmap.py @@ -10,7 +10,6 @@ import gdb -from crash.exceptions import InvalidArgumentError from crash.util.symbols import Types from crash.util import struct_has_member From 7f60d6d8b776c67905731bee640a55676a51678f Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 11:17:16 -0400 Subject: [PATCH 11/26] crash: fix requirements checking The crash module imported the kdump target, which depends on the functionality in GDB that the requirements checker is supposed to discover. Move the target check to crash.kernel instead. Signed-off-by: Jeff Mahoney --- crash/__init__.py | 13 ++----------- crash/addrxlat.py | 2 +- crash/arch/__init__.py | 2 +- crash/kernel.py | 17 ++++++++++++++++- crash/types/node.py | 2 +- crash/types/page.py | 2 +- 6 files changed, 22 insertions(+), 16 deletions(-) diff --git a/crash/__init__.py b/crash/__init__.py index 19ed28db6fe..e7e966536a9 100644 --- a/crash/__init__.py +++ b/crash/__init__.py @@ -3,14 +3,5 @@ import gdb -import kdump.target - -def current_target() -> kdump.target.Target: - target = gdb.current_target() - if target is None: - raise ValueError("No current target") - - if not isinstance(target, kdump.target.Target): - raise ValueError(f"Current target {type(target)} is not supported") - - return target +def archname() -> str: + return gdb.selected_inferior().architecture().name() diff --git a/crash/addrxlat.py b/crash/addrxlat.py index 568bf4cd0a5..75044124543 100644 --- a/crash/addrxlat.py +++ b/crash/addrxlat.py @@ -49,7 +49,7 @@ def cb_read64(self, faddr: addrxlat.FullAddress) -> int: class CrashAddressTranslation: def __init__(self) -> None: try: - target = crash.current_target() + target = gdb.current_target() self.context = target.kdump.get_addrxlat_ctx() self.system = target.kdump.get_addrxlat_sys() except AttributeError: diff --git a/crash/arch/__init__.py b/crash/arch/__init__.py index 3046ea881a1..f38ac75d122 100644 --- a/crash/arch/__init__.py +++ b/crash/arch/__init__.py @@ -41,7 +41,7 @@ class CrashArchitecture: _fetch_registers: Type[FetchRegistersCallback] def __init__(self) -> None: - target = crash.current_target() + target = gdb.current_target() try: target.set_fetch_registers(self._fetch_registers()) except AttributeError: diff --git a/crash/kernel.py b/crash/kernel.py index b5f1ada38a3..dfa9a039e36 100644 --- a/crash/kernel.py +++ b/crash/kernel.py @@ -11,6 +11,8 @@ from elftools.elf.elffile import ELFFile import gdb +import kdump.target + import crash import crash.arch import crash.arch.x86_64 @@ -128,12 +130,26 @@ class CrashKernel: symvals = Symvals(['init_task']) symbols = Symbols(['runqueues']) + def check_target(self) -> kdump.target.Target: + target = gdb.current_target() + + if target is None: + raise ValueError("No current target") + + if not isinstance(target, kdump.target.Target): + raise ValueError(f"Current target {type(target)} is not supported") + + return target + # pylint: disable=unused-argument def __init__(self, roots: PathSpecifier = None, vmlinux_debuginfo: PathSpecifier = None, module_path: PathSpecifier = None, module_debuginfo_path: PathSpecifier = None, verbose: bool = False, debug: bool = False) -> None: + + self.target = self.check_target() + self.findmap: Dict[str, Dict[Any, Any]] = dict() self.modules_order: Dict[str, Dict[str, str]] = dict() obj = gdb.objfiles()[0] @@ -179,7 +195,6 @@ def __init__(self, roots: PathSpecifier = None, self.arch = archclass() - self.target = crash.current_target() self.vmcore = self.target.kdump self.crashing_thread: Optional[gdb.InferiorThread] = None diff --git a/crash/types/node.py b/crash/types/node.py index cc4fc71609b..7658f26f5aa 100644 --- a/crash/types/node.py +++ b/crash/types/node.py @@ -28,7 +28,7 @@ def numa_node_id(cpu: int) -> int: Returns: :obj:`int`: The NUMA node ID for the specified CPU. """ - if crash.current_target().arch.name() == "powerpc:common64": + if crash.archname() == "powerpc:common64": return int(symvals.numa_cpu_lookup_table[cpu]) return int(get_percpu_var(symbols.numa_node, cpu)) diff --git a/crash/types/page.py b/crash/types/page.py index 8648615fe92..a487975da55 100644 --- a/crash/types/page.py +++ b/crash/types/page.py @@ -60,7 +60,7 @@ class Page: def setup_page_type(cls, gdbtype: gdb.Type) -> None: # TODO: should check config, but that failed to work on ppc64, hardcode # 64k for now - if crash.current_target().arch.name() == "powerpc:common64": + if crash.archname() == "powerpc:common64": cls.PAGE_SHIFT = 16 # also a config cls.directmap_base = 0xc000000000000000 From 7d87c3ac7d1d6d3c6d8a62f785c4e05aea7bbf2d Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Wed, 27 Jul 2022 23:03:59 -0400 Subject: [PATCH 12/26] crash.util.symbols: add facility to delay until the target is initialized This commit removes the complexity of ordering import, initialization, and delayed symbol/type resolution that made some kinds of changes fragile. By default, the callbacks won't occur until a target based on gdb.LinuxKernelTarget is on top. This implies that the kernel has been properly relocated and we don't need to do things like flush the symbol cache or manually order imports and initializations. We can also now pause and unpause callbacks around module loading so that the module loading process drops from about 5 seconds per module to 5 seconds total. Some types are needed during early initialization and the waiting behavior can be overridden by specifying wait_for_target=False. Signed-off-by: Jeff Mahoney --- crash/infra/callback.py | 160 +++++++++++++++++++++----------- crash/infra/lookup.py | 91 ++++++++---------- crash/kernel.py | 8 +- crash/util/symbols.py | 46 ++++----- tests/test_infra_lookup.py | 26 ++++-- tests/test_list.py | 4 + tests/test_objfile_callbacks.py | 63 ++++++++++++- tests/test_percpu.py | 2 + tests/test_rbtree.py | 2 + tests/test_syscache.py | 7 +- tests/test_syscmd.py | 2 + tests/test_types_bitmap.py | 2 + tests/test_util.py | 3 + tests/test_util_symbols.py | 40 +++++--- 14 files changed, 293 insertions(+), 163 deletions(-) diff --git a/crash/infra/callback.py b/crash/infra/callback.py index ddee8d5a5f2..ede7ec1daa0 100644 --- a/crash/infra/callback.py +++ b/crash/infra/callback.py @@ -1,7 +1,9 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -from typing import Callable, Any, Union, TypeVar, Optional +from typing import Any, Callable, List, Optional, TypeVar, Union + +import abc import gdb @@ -16,7 +18,7 @@ def __init__(self, callback_obj: 'ObjfileEventCallback') -> None: super().__init__(msg) self.callback_obj = callback_obj -class ObjfileEventCallback: +class ObjfileEventCallback(metaclass=abc.ABCMeta): """ A generic objfile callback class @@ -30,11 +32,61 @@ class ObjfileEventCallback: Consumers of this interface must also call :meth:`connect_callback` to connect the object to the callback infrastructure. """ - def __init__(self) -> None: + + _target_waitlist: List['ObjfileEventCallback'] = list() + _pending_list: List['ObjfileEventCallback'] = list() + _paused: bool = False + _connected_to_objfile_callback: bool = False + + def check_target(self) -> bool: + return isinstance(gdb.current_target(), gdb.LinuxKernelTarget) + + def __init__(self, wait_for_target: bool = True) -> None: self.completed = False self.connected = False + self._waiting_for_target = wait_for_target and not self.check_target() + + if not self._connected_to_objfile_callback: + # pylint: disable=no-member + gdb.events.new_objfile.connect(self._new_objfile_callback) + self._connected_to_objfile_callback = True + + # pylint: disable=unused-argument + @classmethod + def _new_objfile_callback(cls, event: gdb.NewObjFileEvent) -> None: + cls.evaluate_all() + + @classmethod + def target_ready(cls) -> None: + for callback in cls._target_waitlist: + callback.complete_wait_for_target() + + cls._target_waitlist[:] = list() + cls._update_pending() + + @classmethod + def evaluate_all(cls) -> None: + if not cls._paused: + for callback in cls._pending_list: + callback.evaluate(False) + cls._update_pending() - self._setup_symbol_cache_flush_callback() + @classmethod + def pause(cls) -> None: + cls._paused = True + + @classmethod + def unpause(cls) -> None: + cls._paused = False + cls.evaluate_all() + @classmethod + def dump_lists(cls) -> None: + print(f"Pending list: {[str(x) for x in ObjfileEventCallback._pending_list]}") + print(f"Target waitlist: {[str(x) for x in ObjfileEventCallback._target_waitlist]}") + + def complete_wait_for_target(self) -> None: + self._waiting_for_target = False + self.evaluate(False) def connect_callback(self) -> bool: """ @@ -49,27 +101,26 @@ def connect_callback(self) -> bool: if self.connected: return False - self.connected = True - - # We don't want to do lookups immediately if we don't have - # an objfile. It'll fail for any custom types but it can - # also return builtin types that are eventually changed. - objfiles = gdb.objfiles() - if objfiles: - result = self.check_ready() - if not (result is None or result is False): - completed = self.callback(result) - if completed is None: - completed = True - self.completed = completed + if not self._waiting_for_target: + # We don't want to do lookups immediately if we don't have + # an objfile. It'll fail for any custom types but it can + # also return builtin types that are eventually changed. + if gdb.objfiles(): + self.evaluate() + else: + self._target_waitlist.append(self) if self.completed is False: - # pylint: disable=no-member - gdb.events.new_objfile.connect(self._new_objfile_callback) + self.connected = True + self._pending_list.append(self) return self.completed - def complete(self) -> None: + @classmethod + def _update_pending(cls) -> None: + cls._pending_list[:] = [x for x in cls._pending_list if x.connected] + + def complete(self, update_now: bool = True) -> None: """ Complete and disconnect this callback from the event system. @@ -77,43 +128,26 @@ def complete(self) -> None: :obj:`CallbackCompleted`: This callback has already been completed. """ if not self.completed: - # pylint: disable=no-member - gdb.events.new_objfile.disconnect(self._new_objfile_callback) self.completed = True - self.connected = False + if self.connected: + self.connected = False + if update_now: + self._update_pending() else: raise CallbackCompleted(self) - _symbol_cache_flush_setup = False - @classmethod - def _setup_symbol_cache_flush_callback(cls) -> None: - if not cls._symbol_cache_flush_setup: - # pylint: disable=no-member - gdb.events.new_objfile.connect(cls._flush_symbol_cache_callback) - cls._symbol_cache_flush_setup = True - - - # GDB does this itself, but Python is initialized ahead of the - # symtab code. The symtab observer is behind the python observers - # in the execution queue so the cache flush executes /after/ us. - @classmethod - # pylint: disable=unused-argument - def _flush_symbol_cache_callback(cls, event: gdb.NewObjFileEvent) -> None: - gdb.execute("maint flush-symbol-cache") - - # pylint: disable=unused-argument - def _new_objfile_callback(self, event: gdb.NewObjFileEvent) -> None: - # GDB purposely copies the event list prior to calling the callbacks - # If we remove an event from another handler, it will still be sent - if self.completed: - return - - result = self.check_ready() - if not (result is None or result is False): - completed = self.callback(result) - if completed is True or completed is None: - self.complete() - + def evaluate(self, update_now: bool = True) -> None: + if not self._waiting_for_target: + try: + result = self.check_ready() + if not (result is None or result is False): + completed = self.callback(result) + if completed is True or completed is None: + self.complete(update_now) + except gdb.error: + pass + + @abc.abstractmethod def check_ready(self) -> Any: """ The method that derived classes implement for detecting when the @@ -124,8 +158,9 @@ def check_ready(self) -> Any: be passed untouched to :meth:`callback` if the result is anything other than :obj:`None` or :obj:`False`. """ - raise NotImplementedError("check_ready must be implemented by derived class.") + pass + @abc.abstractmethod def callback(self, result: Any) -> Optional[bool]: """ The callback that derived classes implement for handling the @@ -139,4 +174,19 @@ def callback(self, result: Any) -> Optional[bool]: the callback succeeded and will be completed and removed. Otherwise, the callback will stay connected for future completion. """ - raise NotImplementedError("callback must be implemented by derived class.") + pass + +def target_ready() -> None: + ObjfileEventCallback.target_ready() + +def evaluate_all() -> None: + ObjfileEventCallback.evaluate_all() + +def pause_objfile_callbacks() -> None: + ObjfileEventCallback.pause() + +def unpause_objfile_callbacks() -> None: + ObjfileEventCallback.unpause() + +def dump_lists() -> None: + ObjfileEventCallback.dump_lists() diff --git a/crash/infra/lookup.py b/crash/infra/lookup.py index ad50cdc0b0e..7d84ad47d77 100644 --- a/crash/infra/lookup.py +++ b/crash/infra/lookup.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -from typing import Tuple, Any, Union, Optional +from typing import Any, Optional, Tuple, Type, Union import gdb @@ -9,6 +9,7 @@ from crash.infra.callback import Callback from crash.exceptions import DelayedAttributeError +# pylint: disable=abstract-method class NamedCallback(ObjfileEventCallback): """ A base class for Callbacks with names @@ -28,31 +29,14 @@ class NamedCallback(ObjfileEventCallback): attrname (:obj:`str`): The name of symbol or type being resolved translated for use as an attribute name. """ - def __init__(self, name: str, callback: Callback, - attrname: str = None) -> None: - super().__init__() + def __init__(self, name: str, callback: Callback, wait_for_target: bool = True, **kwargs: Any) -> None: + super().__init__(wait_for_target) self.name = name - self.attrname = self.name - - if attrname is not None: - self.attrname = attrname + self.attrname = kwargs.get('attrname', self.name) self._callback = callback - # This is silly but it avoids pylint abstract-method warnings - def check_ready(self) -> Any: - """ - The method that derived classes implement for detecting when the - conditions required to call the callback have been met. - - Returns: - :obj:`object`: This method can return an arbitrary object. It will - be passed untouched to :meth:`callback` if the result is anything - other than :obj:`None` or :obj:`False`. - """ - raise NotImplementedError("check_ready must be implemented by derived class.") - def callback(self, result: Any) -> Union[None, bool]: """ The callback for handling the sucessful result of :meth:`check_ready`. @@ -82,9 +66,9 @@ class MinimalSymbolCallback(NamedCallback): callback: The callback to execute when the minimal symbol is discovered symbol_file (optional): Name of the symbol file to use """ - def __init__(self, name: str, callback: Callback, + def __init__(self, name: str, callback: Callback, wait_for_target: bool = True, symbol_file: str = None) -> None: - super().__init__(name, callback) + super().__init__(name, callback, wait_for_target) self.symbol_file = symbol_file @@ -120,9 +104,9 @@ class SymbolCallback(NamedCallback): is assumed to be one of the value associated with :obj:`gdb.Symbol` constant, i.e. SYMBOL_*_DOMAIN. """ - def __init__(self, name: str, callback: Callback, + def __init__(self, name: str, callback: Callback, wait_for_target: bool = True, domain: int = gdb.SYMBOL_VAR_DOMAIN) -> None: - super().__init__(name, callback) + super().__init__(name, callback, wait_for_target) self.domain = domain @@ -183,11 +167,11 @@ class TypeCallback(NamedCallback): block (optional): The :obj:`gdb.Block` to search for the symbol """ - def __init__(self, name: str, callback: Callback, - block: gdb.Block = None) -> None: + def __init__(self, name: str, callback: Callback, wait_for_target: bool = True, + block: gdb.Block = None, **kwargs: Any) -> None: (name, attrname, self.pointer) = self.resolve_type(name) - super().__init__(name, callback, attrname) + super().__init__(name, callback, wait_for_target, attrname=attrname) self.block = block @@ -264,21 +248,23 @@ class DelayedValue: A generic class for making class attributes available that describe to-be-loaded symbols, minimal symbols, and types. """ - def __init__(self, name: str, attrname: str = None) -> None: + def __init__(self, name: str, wait_for_target: bool = True, **kwargs: Any) -> None: if name is None or not isinstance(name, str): raise ValueError("Name must be a valid string") self.name = name - - if attrname is None: - self.attrname = name - else: - self.attrname = attrname + self.wait_for_target = wait_for_target + self.attrname = kwargs.get('attrname', self.name) assert self.attrname is not None + self.cb: NamedCallback + self.value: Any = None + def attach_callback(self, cbcls: Type[NamedCallback], **kwargs: Any) -> None: + self.cb = cbcls(self.name, self.callback, self.wait_for_target, **kwargs) + def get(self) -> Any: if self.value is None: raise DelayedAttributeError(self.name) @@ -288,6 +274,13 @@ def callback(self, value: Any) -> None: if self.value is not None: return self.value = value + try: + del self.cb + except AttributeError: + pass + + def __str__(self) -> str: + return "{} attached with {}".format(self.__class__, str(self.cb)) class DelayedMinimalSymbol(DelayedValue): """ @@ -296,12 +289,9 @@ class DelayedMinimalSymbol(DelayedValue): Args: name: The name of the minimal symbol """ - def __init__(self, name: str) -> None: - super().__init__(name) - self.cb = MinimalSymbolCallback(name, self.callback) - - def __str__(self) -> str: - return "{} attached with {}".format(self.__class__, str(self.cb)) + def __init__(self, name: str, wait_for_target: bool = True) -> None: + super().__init__(name, wait_for_target) + self.attach_callback(MinimalSymbolCallback) class DelayedSymbol(DelayedValue): """ @@ -310,12 +300,9 @@ class DelayedSymbol(DelayedValue): Args: name: The name of the symbol """ - def __init__(self, name: str) -> None: - super().__init__(name) - self.cb = SymbolCallback(name, self.callback) - - def __str__(self) -> str: - return "{} attached with {}".format(self.__class__, str(self.cb)) + def __init__(self, name: str, wait_for_target: bool = True) -> None: + super().__init__(name, wait_for_target) + self.attach_callback(SymbolCallback) class DelayedType(DelayedValue): """ @@ -324,10 +311,11 @@ class DelayedType(DelayedValue): Args: name: The name of the type. """ - def __init__(self, name: str) -> None: + def __init__(self, name: str, wait_for_target: bool = True, + block: gdb.Block = None) -> None: (name, attrname, self.pointer) = TypeCallback.resolve_type(name) - super().__init__(name, attrname) - self.cb = TypeCallback(name, self.callback) + super().__init__(name, wait_for_target, attrname=attrname) + self.attach_callback(TypeCallback, block=block) def __str__(self) -> str: return "{} attached with {}".format(self.__class__, str(self.callback)) @@ -352,7 +340,7 @@ def callback(self, value: gdb.Symbol) -> None: self.value = symval def __str__(self) -> str: - return "{} attached with {}".format(self.__class__, str(self.cb)) + return "{} attached with {}".format(self.__class__, str(self.callback)) class DelayedMinimalSymval(DelayedMinimalSymbol): """ @@ -364,6 +352,3 @@ class DelayedMinimalSymval(DelayedMinimalSymbol): """ def callback(self, value: gdb.MinSymbol) -> None: self.value = int(value.value().address) - - def __str__(self) -> str: - return "{} attached with {}".format(self.__class__, str(self.cb)) diff --git a/crash/kernel.py b/crash/kernel.py index dfa9a039e36..44121256e7c 100644 --- a/crash/kernel.py +++ b/crash/kernel.py @@ -21,6 +21,8 @@ from crash.util import get_symbol_value from crash.util.symbols import Types, Symvals, Symbols from crash.exceptions import MissingSymbolError, InvalidArgumentError +from crash.infra.callback import pause_objfile_callbacks, unpause_objfile_callbacks +from crash.cache.syscache import utsname class CrashKernelError(RuntimeError): """Raised when an error occurs while initializing the debugging session""" @@ -450,13 +452,12 @@ def load_modules(self, verbose: bool = False, debug: bool = False) -> None: This does not include a failure to locate a module or its debuginfo. """ - import crash.cache.syscache # pylint: disable=redefined-outer-name - version = crash.cache.syscache.utsname.release - print("Loading modules for {}".format(version), end='') + print("Loading modules for {}".format(utsname.release), end='') if verbose: print(":", flush=True) failed = 0 loaded = 0 + pause_objfile_callbacks() for module in for_each_module(): modname = "{}".format(module['name'].string()) modfname = "{}.ko".format(modname) @@ -535,6 +536,7 @@ def load_modules(self, verbose: bool = False, debug: bool = False) -> None: # We shouldn't need this again, so why keep it around? del self.findmap self.findmap = {} + unpause_objfile_callbacks() def _normalize_modname(self, mod: str) -> str: return mod.replace('-', '_') diff --git a/crash/util/symbols.py b/crash/util/symbols.py index 0c11a56bc33..075338cf626 100644 --- a/crash/util/symbols.py +++ b/crash/util/symbols.py @@ -48,14 +48,14 @@ class DelayedCollection: the container object *or* the contained object if it has been overridden via :meth:`override`. """ - def __init__(self, cls: Type[DelayedValue], names: Names) -> None: + def __init__(self, cls: Type[DelayedValue], names: Names, wait_for_target: bool) -> None: self.attrs: Dict[str, DelayedValue] = {} if isinstance(names, str): names = [names] for name in names: - t = cls(name) + t = cls(name, wait_for_target=wait_for_target) self.attrs[t.attrname] = t self.attrs[t.name] = t @@ -129,8 +129,8 @@ class Types(DelayedCollection): names: A :obj:`str` or :obj:`list` of :obj:`str` containing the names of the types to resolve. """ - def __init__(self, names: Names) -> None: - super(Types, self).__init__(DelayedType, names) + def __init__(self, names: Names, wait_for_target: bool = True) -> None: + super(Types, self).__init__(DelayedType, names, wait_for_target) def override(self, name: str, value: gdb.Type) -> None: # type: ignore """ @@ -171,8 +171,8 @@ class Symbols(DelayedCollection): names: A :obj:`str` or :obj:`list` of :obj:`str` containing the names of the symbols to resolve. """ - def __init__(self, names: Names) -> None: - super(Symbols, self).__init__(DelayedSymbol, names) + def __init__(self, names: Names, wait_for_target: bool = True) -> None: + super(Symbols, self).__init__(DelayedSymbol, names, wait_for_target) class Symvals(DelayedCollection): """ @@ -205,8 +205,8 @@ class Symvals(DelayedCollection): names: A :obj:`str` or :obj:`list` of :obj:`str` containing the names of the symbols to resolve. """ - def __init__(self, names: Names) -> None: - super(Symvals, self).__init__(DelayedSymval, names) + def __init__(self, names: Names, wait_for_target: bool = True) -> None: + super(Symvals, self).__init__(DelayedSymval, names, wait_for_target) class MinimalSymbols(DelayedCollection): """ @@ -239,8 +239,8 @@ class MinimalSymbols(DelayedCollection): names: A :obj:`str` or :obj:`list` of :obj:`str` containing the names of the minimal symbols to resolve. """ - def __init__(self, names: Names) -> None: - super(MinimalSymbols, self).__init__(DelayedMinimalSymbol, names) + def __init__(self, names: Names, wait_for_target: bool = True) -> None: + super().__init__(DelayedMinimalSymbol, names, wait_for_target) class MinimalSymvals(DelayedCollection): """ @@ -268,8 +268,8 @@ class MinimalSymvals(DelayedCollection): names: A :obj:`str` or :obj:`list` of :obj:`str` containing the names of the minimal symbols to resolve. """ - def __init__(self, names: Names) -> None: - super(MinimalSymvals, self).__init__(DelayedMinimalSymval, names) + def __init__(self, names: Names, wait_for_target: bool = True) -> None: + super().__init__(DelayedMinimalSymval, names, wait_for_target) class DelayedValues(DelayedCollection): """ @@ -303,30 +303,30 @@ class DelayedValues(DelayedCollection): Args: names: The names to use for the :obj:`.DelayedValue` objects. """ - def __init__(self, names: Names) -> None: - super(DelayedValues, self).__init__(DelayedValue, names) + def __init__(self, names: Names, wait_for_target: bool = True) -> None: + super().__init__(DelayedValue, names, wait_for_target) CallbackSpecifier = Tuple[str, Callable] CallbackSpecifiers = Union[List[CallbackSpecifier], CallbackSpecifier] class CallbackCollection: - def __init__(self, cls: Type[NamedCallback], - cbs: CallbackSpecifiers) -> None: + def __init__(self, cls: Type[NamedCallback], cbs: CallbackSpecifiers, + wait_for_target: bool) -> None: if isinstance(cbs, tuple): cbs = [cbs] for cb in cbs: - t = cls(cb[0], cb[1]) + t = cls(cb[0], cb[1], wait_for_target=wait_for_target) setattr(self, t.attrname, t) class TypeCallbacks(CallbackCollection): - def __init__(self, cbs: CallbackSpecifiers) -> None: - super().__init__(TypeCallback, cbs) + def __init__(self, cbs: CallbackSpecifiers, wait_for_target: bool = True) -> None: + super().__init__(TypeCallback, cbs, wait_for_target) class SymbolCallbacks(CallbackCollection): - def __init__(self, cbs: CallbackSpecifiers) -> None: - super().__init__(SymbolCallback, cbs) + def __init__(self, cbs: CallbackSpecifiers, wait_for_target: bool = True) -> None: + super().__init__(SymbolCallback, cbs, wait_for_target) class MinimalSymbolCallbacks(CallbackCollection): - def __init__(self, cbs: CallbackSpecifiers) -> None: - super().__init__(MinimalSymbolCallback, cbs) + def __init__(self, cbs: CallbackSpecifiers, wait_for_target: bool = True) -> None: + super().__init__(MinimalSymbolCallback, cbs, wait_for_target) diff --git a/tests/test_infra_lookup.py b/tests/test_infra_lookup.py index 8f79ccdd7e8..ca180fbea08 100644 --- a/tests/test_infra_lookup.py +++ b/tests/test_infra_lookup.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: import unittest +from unittest.mock import patch import gdb from crash.exceptions import DelayedAttributeError -from crash.infra.callback import ObjfileEventCallback from crash.infra.lookup import SymbolCallback, TypeCallback from crash.infra.lookup import MinimalSymbolCallback from crash.infra.lookup import DelayedType, DelayedSymbol, DelayedSymval @@ -74,14 +74,18 @@ def setUp(self): def tearDown(self): gdb.execute("file") - def load_file(self): + def load_util_file(self): gdb.execute("file tests/test-util") + def load_list_file(self): + gdb.execute("file tests/test-list") + def get_test_class(self): class test_class(object): def __init__(self): self.found = False - cb = MinimalSymbolCallback('test_struct', self.callback) + with patch.object(MinimalSymbolCallback, 'check_target', return_value=True): + cb = MinimalSymbolCallback('test_struct', self.callback) def callback(self, result): self.found = True @@ -93,12 +97,12 @@ def test_minsymbol_no_symbol_found(self): test_class = self.get_test_class() x = test_class() self.assertFalse(x.found) - gdb.execute("file tests/test-list") + self.load_list_file() self.assertFalse(x.found) def test_minsymbol_found_immediately(self): test_class = self.get_test_class() - self.load_file() + self.load_util_file() x = test_class() self.assertTrue(x.found) self.assertTrue(isinstance(x.result, gdb.MinSymbol)) @@ -107,7 +111,7 @@ def test_minsymbol_found_after_load(self): test_class = self.get_test_class() x = test_class() self.assertFalse(x.found) - self.load_file() + self.load_util_file() self.assertTrue(x.found) self.assertTrue(isinstance(x.result, gdb.MinSymbol)) @@ -115,9 +119,9 @@ def test_minsymbol_not_found_in_early_load_then_found_after_load(self): test_class = self.get_test_class() x = test_class() self.assertFalse(x.found) - gdb.execute("file tests/test-list") + self.load_list_file() self.assertFalse(x.found) - self.load_file() + self.load_util_file() self.assertTrue(x.found) self.assertTrue(isinstance(x.result, gdb.MinSymbol)) @@ -132,7 +136,8 @@ def get_test_class(self): class test_class(object): def __init__(self): self.found = False - cb = SymbolCallback('test_struct', self.callback) + with patch.object(SymbolCallback, 'check_target', return_value=True): + cb = SymbolCallback('test_struct', self.callback) def callback(self, result): self.found = True @@ -183,7 +188,8 @@ def get_test_class(self): class test_class(object): def __init__(self): self.found = False - cb = TypeCallback('struct test', self.callback) + with patch.object(TypeCallback, 'check_target', return_value=True): + cb = TypeCallback('struct test', self.callback) def callback(self, result): self.found = True diff --git a/tests/test_list.py b/tests/test_list.py index 4a8dec2beab..51216083bba 100644 --- a/tests/test_list.py +++ b/tests/test_list.py @@ -2,8 +2,11 @@ # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: import unittest +from unittest.mock import patch import gdb +import crash.infra.callback + from crash.exceptions import ArgumentTypeError, UnexpectedGDBTypeError from crash.exceptions import InvalidArgumentError from crash.types.list import list_for_each, list_for_each_entry @@ -15,6 +18,7 @@ def get_symbol(name): class TestList(unittest.TestCase): def setUp(self): gdb.execute("file tests/test-list") + crash.infra.callback.target_ready() self.list_head = gdb.lookup_type("struct list_head") def tearDown(self): diff --git a/tests/test_objfile_callbacks.py b/tests/test_objfile_callbacks.py index ae1906e3dc8..2cba0c63d14 100644 --- a/tests/test_objfile_callbacks.py +++ b/tests/test_objfile_callbacks.py @@ -2,6 +2,7 @@ # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: import unittest +from unittest.mock import patch import gdb from crash.util import safe_get_symbol_value @@ -17,12 +18,13 @@ def tearDown(self): def load_file(self): gdb.execute("file tests/test-util") - def test_registering(self): + def get_test_class(self): class test_class(ObjfileEventCallback): - def __init__(self): + def __init__(self, *args, **kwargs): self.called = False self.checked = False - super(test_class, self).__init__() + self.result = None + super(test_class, self).__init__(*args, **kwargs) self.connect_callback() @@ -34,12 +36,67 @@ def callback(self, result): self.called = True self.result = result + return test_class + + def test_registering(self): + test_class = self.get_test_class() + with patch.object(test_class, 'check_target', return_value=True): + x = test_class() + + self.assertFalse(x.called) + self.assertFalse(x.completed) + self.assertFalse(x.checked) + self.assertTrue(x.result is None) + + self.load_file() + self.assertTrue(x.checked) + self.assertTrue(x.called) + self.assertTrue(x.completed) + + self.assertTrue(isinstance(x.result, gdb.Value)) + + def test_early_callback_with_target_wait(self): + test_class = self.get_test_class() + x = test_class() + self.assertFalse(x.called) self.assertFalse(x.completed) self.assertFalse(x.checked) + self.assertTrue(x.result is None) + self.load_file() + self.assertFalse(x.called) + self.assertFalse(x.completed) + self.assertFalse(x.checked) + self.assertTrue(x.result is None) + + x.target_ready() + self.assertTrue(x.checked) + self.assertTrue(x.called) + self.assertTrue(x.completed) + + self.assertTrue(isinstance(x.result, gdb.Value)) + + def test_early_callback_without_target_wait(self): + test_class = self.get_test_class() + + x = test_class(False) + + self.assertFalse(x.called) + self.assertFalse(x.completed) + self.assertFalse(x.checked) + self.assertTrue(x.result is None) + + self.load_file() + self.assertTrue(x.called) + self.assertTrue(x.completed) + self.assertTrue(x.checked) + self.assertTrue(isinstance(x.result, gdb.Value)) + + x.target_ready() self.assertTrue(x.checked) self.assertTrue(x.called) self.assertTrue(x.completed) + self.assertTrue(isinstance(x.result, gdb.Value)) diff --git a/tests/test_percpu.py b/tests/test_percpu.py index 9087fccb6c5..7e88038ea06 100644 --- a/tests/test_percpu.py +++ b/tests/test_percpu.py @@ -5,11 +5,13 @@ import gdb import crash +import crash.infra.callback import crash.types.percpu as percpu class TestPerCPU(unittest.TestCase): def setUp(self): gdb.execute("file tests/test-percpu", to_string=True) + crash.infra.callback.target_ready() try: print() diff --git a/tests/test_rbtree.py b/tests/test_rbtree.py index 5346ac7900a..5f0f17791d2 100644 --- a/tests/test_rbtree.py +++ b/tests/test_rbtree.py @@ -4,6 +4,7 @@ import unittest import gdb +import crash.infra.callback from crash.types.rbtree import rbtree_postorder_for_each, rbtree_postorder_for_each_entry def get_symbol(name): @@ -12,6 +13,7 @@ def get_symbol(name): class TestRbtree(unittest.TestCase): def setUp(self): gdb.execute("file tests/test-rbtree", to_string=True) + crash.infra.callback.target_ready() try: print() print("--- Unsuppressable gdb output ---", end='') diff --git a/tests/test_syscache.py b/tests/test_syscache.py index 1ee1089bf69..f3cdfc97f9e 100644 --- a/tests/test_syscache.py +++ b/tests/test_syscache.py @@ -2,10 +2,14 @@ # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: import unittest +from unittest.mock import patch import gdb import sys from importlib import reload +import crash.infra.callback +from crash.infra.callback import ObjfileEventCallback + from crash.exceptions import DelayedAttributeError fake_config = ( """ @@ -35,6 +39,7 @@ def cycle_namespace(self): self.utsname = crash.cache.syscache.utsname self.kernel = crash.cache.syscache.kernel self.config = crash.cache.syscache.config + crash.infra.callback.target_ready() def clear_namespace(self): gdb.execute("file") @@ -50,7 +55,7 @@ def _decompress_config_buffer(self): def test_utsname_no_sym(self): gdb.execute("file") - gdb.execute("maint flush-symbol-cache") + gdb.execute("maint flush symbol-cache") self.cycle_namespace() utsname = self.CrashUtsnameCache() with self.assertRaises(DelayedAttributeError): diff --git a/tests/test_syscmd.py b/tests/test_syscmd.py index 7387d18600c..d5a7d002df6 100644 --- a/tests/test_syscmd.py +++ b/tests/test_syscmd.py @@ -7,12 +7,14 @@ from io import StringIO from crash.exceptions import MissingSymbolError +import crash.infra.callback from crash.commands import CommandLineError from crash.commands.syscmd import SysCommand class TestSysCmd(unittest.TestCase): def setUp(self): gdb.execute("file tests/test-syscache", to_string=True) + crash.infra.callback.target_ready() self.cmd = SysCommand("pysys") def tearDown(self): diff --git a/tests/test_types_bitmap.py b/tests/test_types_bitmap.py index b11cf2d4f93..c5ffda9fd4c 100644 --- a/tests/test_types_bitmap.py +++ b/tests/test_types_bitmap.py @@ -4,6 +4,7 @@ import unittest import sys +import crash.infra.callback import crash.types.bitmap as bm import gdb @@ -11,6 +12,7 @@ class TestBitmap(unittest.TestCase): def setUp(self): gdb.execute("file tests/test-percpu") + crash.infra.callback.target_ready() ulong = gdb.lookup_type('unsigned long') ulong_array = ulong.array(0) diff --git a/tests/test_util.py b/tests/test_util.py index dd3fdf2ae32..797a31ca09f 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -3,6 +3,7 @@ import unittest import gdb +import crash.infra.callback from crash.exceptions import MissingTypeError, MissingSymbolError from crash.util import offsetof, container_of, resolve_type from crash.util import get_symbol_value, safe_get_symbol_value @@ -10,12 +11,14 @@ from crash.exceptions import NotStructOrUnionError from crash.util import InvalidComponentError + def getsym(sym): return gdb.lookup_symbol(sym, None)[0].value() class TestUtil(unittest.TestCase): def setUp(self): gdb.execute("file tests/test-util") + crash.infra.callback.target_ready() self.ulong = gdb.lookup_type('unsigned long') self.ulongsize = self.ulong.sizeof self.test_struct = gdb.lookup_type("struct test") diff --git a/tests/test_util_symbols.py b/tests/test_util_symbols.py index e32c6728e5f..4a2f10dce5c 100644 --- a/tests/test_util_symbols.py +++ b/tests/test_util_symbols.py @@ -1,11 +1,13 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: import unittest +from unittest.mock import patch import platform import gdb from crash.exceptions import DelayedAttributeError +from crash.infra.lookup import NamedCallback from crash.util.symbols import MinimalSymbols, Symbols, Symvals, Types from crash.util.symbols import TypeCallbacks, SymbolCallbacks from crash.util.symbols import MinimalSymbolCallbacks @@ -19,7 +21,8 @@ def load_file(self): def msymbol_test(self): class Test(object): - msymbols = MinimalSymbols([ 'test_struct' ]) + with patch.object(NamedCallback, 'check_target', return_value=True): + msymbols = MinimalSymbols([ 'test_struct' ]) return Test def test_bad_msymbol_name(self): @@ -51,7 +54,8 @@ def test_msymbol_available_at_start(self): def symbol_test(self): class Test(object): - symbols = Symbols([ 'test_struct' ]) + with patch.object(NamedCallback, 'check_target', return_value=True): + symbols = Symbols([ 'test_struct' ]) return Test def test_bad_symbol_name(self): @@ -83,7 +87,8 @@ def test_symbol_available_at_start(self): def symval_test(self): class Test(object): - symvals = Symvals( [ 'test_struct' ] ) + with patch.object(NamedCallback, 'check_target', return_value=True): + symvals = Symvals( [ 'test_struct' ] ) return Test def test_bad_symval_name(self): @@ -115,7 +120,8 @@ def test_symval_available_at_start(self): def type_test(self): class Test(object): - types = Types( [ 'struct test' ] ) + with patch.object(NamedCallback, 'check_target', return_value=True): + types = Types( [ 'struct test' ] ) return Test def test_bad_type_name(self): @@ -149,7 +155,8 @@ def test_type_available_at_start(self): def ptype_test(self): class Test(object): - types = Types( [ 'struct test *' ]) + with patch.object(NamedCallback, 'check_target', return_value=True): + types = Types( [ 'struct test *' ]) return Test def test_bad_ptype_name(self): @@ -190,8 +197,9 @@ class nested(object): def check_ulong(cls, gdbtype): cls.ulong_valid = True - type_cbs = TypeCallbacks( [ ('unsigned long', - nested.check_ulong) ] ) + with patch.object(NamedCallback, 'check_target', return_value=True): + type_cbs = TypeCallbacks( [ ('unsigned long', + nested.check_ulong) ] ) return Test def test_type_callback_nofile(self): @@ -211,17 +219,19 @@ def test_type_callback(self): def type_callback_test_multi(self): class Test(object): - class nested(object): - types = Types( [ 'unsigned long' ] ) + with patch.object(NamedCallback, 'check_target', return_value=True): + class nested(object): + types = Types( [ 'unsigned long' ] ) - ulong_valid = False + ulong_valid = False - @classmethod - def check_ulong(cls, gdbtype): - cls.ulong_valid = True + @classmethod + def check_ulong(cls, gdbtype): + cls.ulong_valid = True - type_cbs = TypeCallbacks( [ ('unsigned long', - nested.check_ulong) ] ) + with patch.object(NamedCallback, 'check_target', return_value=True): + type_cbs = TypeCallbacks( [ ('unsigned long', + nested.check_ulong) ] ) return Test From 67a32426dbd31e7989b58f8b89fb6db780ec4b7e Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 28 Jul 2022 13:27:46 -0400 Subject: [PATCH 13/26] crash.addrxlat: align with newest version The type=enum argument has been replaced with an os_type=str argument. Signed-off-by: Jeff Mahoney --- crash/addrxlat.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crash/addrxlat.py b/crash/addrxlat.py index 75044124543..e2b1322cc38 100644 --- a/crash/addrxlat.py +++ b/crash/addrxlat.py @@ -57,7 +57,7 @@ def __init__(self) -> None: self.system = addrxlat.System() self.system.os_init(self.context, arch=utsname.machine, - type=addrxlat.OS_LINUX) + os_type="linux") self.is_non_auto = False xlatmap = self.system.get_map(addrxlat.SYS_MAP_MACHPHYS_KPHYS) diff --git a/setup.py b/setup.py index 3a8db1515ce..4575cd0aaeb 100644 --- a/setup.py +++ b/setup.py @@ -14,7 +14,7 @@ }, python_requires='>=3.6', - install_requires = [ 'pyelftools' ], + install_requires = [ 'pyelftools', 'addrxlat' ], author = "Jeff Mahoney", author_email = "jeffm@suse.com", From e3462b7a90a9a862558ab17c491b1a49f01c4f39 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 11:38:59 -0400 Subject: [PATCH 14/26] crash-python: Update to use gdb-12.1 with kdumpfile target The crash-python GDB implementation has been updated to use gdb 12.1, which integrates libkdumpfile into libbfd and offers a small semantic target to improve performance. As part of the update, the registers interface has changed, we populate the thread list from the target implementation in C, and use the core file interface. Signed-off-by: Jeff Mahoney --- README.rst | 4 +- crash.sh | 7 ++- crash/addrxlat.py | 7 ++- crash/arch/__init__.py | 27 ++++---- crash/arch/x86_64.py | 85 +++++++++++++------------ crash/kernel.py | 39 +++++------- crash/requirements/__init__.py | 14 ++--- crash/requirements/test_target.py | 10 +-- doc-source/mock/gdb/__init__.py | 16 ++++- kdump/target.py | 100 ++++-------------------------- test-gdb-compatibility.gdbinit | 12 ---- tests/stubs/_gdb.pyi | 34 +++++----- 12 files changed, 141 insertions(+), 214 deletions(-) diff --git a/README.rst b/README.rst index 987c7a894cd..2f0325ed6e5 100644 --- a/README.rst +++ b/README.rst @@ -91,8 +91,8 @@ It requires the following components to work successfully: - `Python `_ 3.6 or newer - `pyelftools `_ -- `libkdumpfile `_ -- `GDB `_ with python extensions and built with Python 3.6 or newer. +- `libkdumpfile `_ with the kdumpfile_object_from_native API +- `GDB `_ with python extensions and built with Python 3.6 or newer. If you are using a SUSE or openSUSE release, pre-built packages are available on the `Open Build Service `_. diff --git a/crash.sh b/crash.sh index 8276c928ac4..2404be90fdb 100755 --- a/crash.sh +++ b/crash.sh @@ -233,12 +233,15 @@ set prompt py-crash> set height 0 set print pretty on +file $KERNEL +core $VMCORE + python from kdump.target import Target target = Target(debug=False) end -target kdumpfile $KERNEL $VMCORE +target kdumpfile python import sys @@ -290,8 +293,6 @@ except RuntimeError as e: traceback.print_exc() sys.exit(1) -target.unregister() -del target EOF # This is how we debug gdb problems when running crash diff --git a/crash/addrxlat.py b/crash/addrxlat.py index e2b1322cc38..a73308f6eb2 100644 --- a/crash/addrxlat.py +++ b/crash/addrxlat.py @@ -5,6 +5,7 @@ import gdb import crash +import kdump.target from crash.cache.syscache import utsname from crash.util import offsetof @@ -50,8 +51,10 @@ class CrashAddressTranslation: def __init__(self) -> None: try: target = gdb.current_target() - self.context = target.kdump.get_addrxlat_ctx() - self.system = target.kdump.get_addrxlat_sys() + if not isinstance(target, kdump.target.Target): + raise TypeError("Not using kdump target") + self.context = target.kdumpfile.get_addrxlat_ctx() + self.system = target.kdumpfile.get_addrxlat_sys() except AttributeError: self.context = TranslationContext() self.system = addrxlat.System() diff --git a/crash/arch/__init__.py b/crash/arch/__init__.py index f38ac75d122..633bc710dfd 100644 --- a/crash/arch/__init__.py +++ b/crash/arch/__init__.py @@ -4,6 +4,8 @@ from typing import List, Iterator, Any, Optional, Type import gdb +from gdb import RegisterNameType, RegisterCollectionType +from gdb import FetchRegistersCallbackType from gdb.FrameDecorator import FrameDecorator import crash @@ -15,41 +17,42 @@ class FetchRegistersCallback: The architecture code must implement the :meth:`fetch_active` and :meth:`fetch_scheduled` methods. """ - def fetch_active(self, thread: gdb.InferiorThread, register: int) -> None: + def fetch_active(self, thread: gdb.InferiorThread, + register: RegisterNameType) -> RegisterCollectionType: raise NotImplementedError("Target has no fetch_active callback") def fetch_scheduled(self, thread: gdb.InferiorThread, - register: int) -> None: + register: RegisterNameType) -> RegisterCollectionType: raise NotImplementedError("Target has no fetch_scheduled callback") def __call__(self, thread: gdb.InferiorThread, - register: gdb.Register) -> None: - if register is None: - regnum = -1 - else: - regnum = register.regnum + register: RegisterNameType) -> RegisterCollectionType: if thread.info.active: - return self.fetch_active(thread, regnum) + return self.fetch_active(thread, register) - return self.fetch_scheduled(thread, regnum) + return self.fetch_scheduled(thread, register) class CrashArchitecture: ident = "base-class" aliases: List[str] = list() - _fetch_registers: Type[FetchRegistersCallback] + _fetch_registers: FetchRegistersCallbackType def __init__(self) -> None: target = gdb.current_target() + if target is None: + raise ValueError("No target loaded") from None + if not isinstance(target, gdb.LinuxKernelTarget): + raise ValueError("Incorrect target loaded") from None + try: target.set_fetch_registers(self._fetch_registers()) except AttributeError: raise NotImplementedError("No fetch_registers callback defined") from None @classmethod - def set_fetch_registers(cls, - callback: Type[FetchRegistersCallback]) -> None: + def set_fetch_registers(cls, callback: FetchRegistersCallbackType) -> None: """ Set a fetch_regisers callback for the Target to use. diff --git a/crash/arch/x86_64.py b/crash/arch/x86_64.py index 40427cb41f2..03de4342614 100644 --- a/crash/arch/x86_64.py +++ b/crash/arch/x86_64.py @@ -3,6 +3,7 @@ from typing import Optional import re +import sys import gdb @@ -17,63 +18,63 @@ # pylint: disable=abstract-method class _FetchRegistersBase(FetchRegistersCallback): - def fetch_active(self, thread: gdb.InferiorThread, register: int) -> None: + def fetch_active(self, thread: gdb.InferiorThread, + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: + regmap = { + "rflags" : "eflags" + } + registers = {} task = thread.info for reg in task.regs: - if reg == "rip" and register not in (16, -1): + if (reg == "rip" and register is not None and + register.name != "rip"): continue try: - thread.registers[reg].value = task.regs[reg] + # vmcore uses rflags, gdb uses eflags + if reg in regmap: + reg = regmap[reg] + registers[reg] = task.regs[reg] except KeyError: pass - def fetch_scheduled(self, thread: gdb.InferiorThread, - register: int) -> None: - pass + return registers # pylint: disable=abstract-method class _FRC_inactive_task_frame(_FetchRegistersBase): def fetch_scheduled(self, thread: gdb.InferiorThread, - register: int) -> None: + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: + registers: gdb.RegisterCollectionType = {} task = thread.info.task_struct rsp = task['thread']['sp'].cast(types.unsigned_long_p_type) rsp = thread.arch.adjust_scheduled_frame_offset(rsp) - thread.registers['rsp'].value = rsp + registers['rsp'] = rsp frame = rsp.cast(types.inactive_task_frame_p_type).dereference() - # Only write rip when requested; It resets the frame cache - if register in (16, -1): - thread.registers['rip'].value = thread.arch.get_scheduled_rip() - if register == 16: - return - - thread.registers['rbp'].value = frame['bp'] - thread.registers['rbx'].value = frame['bx'] - thread.registers['r12'].value = frame['r12'] - thread.registers['r13'].value = frame['r13'] - thread.registers['r14'].value = frame['r14'] - thread.registers['r15'].value = frame['r15'] - thread.registers['cs'].value = 2*8 - thread.registers['ss'].value = 3*8 + registers['rip'] = thread.arch.get_scheduled_rip() + registers['rbp'] = frame['bp'] + registers['rbx'] = frame['bx'] + registers['r12'] = frame['r12'] + registers['r13'] = frame['r13'] + registers['r14'] = frame['r14'] + registers['r15'] = frame['r15'] + registers['cs'] = 2*8 + registers['ss'] = 3*8 thread.info.stack_pointer = rsp thread.info.valid_stack = True + return registers + class _FRC_thread_return(_FetchRegistersBase): def fetch_scheduled(self, thread: gdb.InferiorThread, - register: int) -> None: + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: + registers: gdb.RegisterCollectionType = {} task = thread.info.task_struct - # Only write rip when requested; It resets the frame cache - if register in (16, -1): - thread.registers['rip'].value = msymvals.thread_return - if register == 16: - return - rsp = task['thread']['sp'].cast(types.unsigned_long_p_type) rbp = rsp.dereference().cast(types.unsigned_long_p_type) rbx = (rbp - 1).dereference() @@ -89,19 +90,22 @@ def fetch_scheduled(self, thread: gdb.InferiorThread, # if ex: # print("EXCEPTION STACK: pid {:d}".format(task['pid'])) - thread.registers['rsp'].value = rsp - thread.registers['rbp'].value = rbp - thread.registers['rbx'].value = rbx - thread.registers['r12'].value = r12 - thread.registers['r13'].value = r13 - thread.registers['r14'].value = r14 - thread.registers['r15'].value = r15 - thread.registers['cs'].value = 2*8 - thread.registers['ss'].value = 3*8 + registers['rip'] = msymvals.thread_return + registers['rsp'] = rsp + registers['rbp'] = rbp + registers['rbx'] = rbx + registers['r12'] = r12 + registers['r13'] = r13 + registers['r14'] = r14 + registers['r15'] = r15 + registers['cs'] = 2*8 + registers['ss'] = 3*8 thread.info.stack_pointer = rsp thread.info.valid_stack = True + return registers + class x86_64Architecture(CrashArchitecture): ident = "i386:x86-64" aliases = ["x86_64"] @@ -130,7 +134,7 @@ def setup_scheduled_frame_offset(self, task: gdb.Value) -> None: return top = int(task['stack']) + 16*1024 - callq = re.compile("callq.*<(\w+)>") + callq = re.compile("callq?.*<(\w+)>") orig_rsp = rsp = task['thread']['sp'].cast(types.unsigned_long_p_type) @@ -145,6 +149,9 @@ def setup_scheduled_frame_offset(self, task: gdb.Value) -> None: count += 1 continue + if not insn: + continue + m = callq.search(insn) if m and m.group(1) == "__switch_to_asm": self._frame_offset = rsp - orig_rsp + 1 diff --git a/crash/kernel.py b/crash/kernel.py index 44121256e7c..bb959009203 100644 --- a/crash/kernel.py +++ b/crash/kernel.py @@ -18,7 +18,7 @@ import crash.arch.x86_64 import crash.arch.ppc64 from crash.types.module import for_each_module, for_each_module_section -from crash.util import get_symbol_value +from crash.util import get_symbol_value, get_typed_pointer from crash.util.symbols import Types, Symvals, Symbols from crash.exceptions import MissingSymbolError, InvalidArgumentError from crash.infra.callback import pause_objfile_callbacks, unpause_objfile_callbacks @@ -189,7 +189,7 @@ def __init__(self, roots: PathSpecifier = None, self.vermagic = self.extract_vermagic() - archname = obj.architecture.name() + archname = crash.archname() try: archclass = crash.arch.get_architecture(archname) except RuntimeError as e: @@ -197,8 +197,6 @@ def __init__(self, roots: PathSpecifier = None, self.arch = archclass() - self.vmcore = self.target.kdump - self.crashing_thread: Optional[gdb.InferiorThread] = None def _setup_roots(self, roots: PathSpecifier = None, @@ -687,7 +685,7 @@ def setup_tasks(self) -> None: populated, which allows symbolic stack traces to be made available. """ from crash.types.percpu import get_percpu_vars - from crash.types.task import LinuxTask, for_each_all_tasks + from crash.types.task import LinuxTask, types as task_types import crash.cache.tasks # pylint: disable=redefined-outer-name gdb.execute('set print thread-events 0') @@ -703,27 +701,26 @@ def setup_tasks(self) -> None: except MissingSymbolError: crashing_cpu = -1 - for task in for_each_all_tasks(): - ltask = LinuxTask(task) + kdumpfile = self.target.kdumpfile + task_struct_p_type = task_types.task_struct_type.pointer() + + for thread in gdb.selected_inferior().threads(): + task_address = thread.ptid[2] + + task = get_typed_pointer(task_address, task_struct_p_type) + + ltask = LinuxTask(task.dereference()) - active = int(task.address) in rqscurrs + active = task_address in rqscurrs if active: - cpu = rqscurrs[int(task.address)] - regs = self.vmcore.attr.cpu[cpu].reg + cpu = rqscurrs[task_address] + regs = kdumpfile.attr.cpu[cpu].reg ltask.set_active(cpu, regs) else: self.arch.setup_scheduled_frame_offset(task) - ptid = (LINUX_KERNEL_PID, task['pid'], 0) - - try: - thread = gdb.selected_inferior().new_thread(ptid) - thread.info = ltask - thread.arch = self.arch - except gdb.error: - print("Failed to setup task @{:#x}".format(int(task.address))) - continue - thread.name = task['comm'].string() + thread.info = ltask + thread.arch = self.arch if active and cpu == crashing_cpu: self.crashing_thread = thread @@ -738,5 +735,3 @@ def setup_tasks(self) -> None: print(".", end='') sys.stdout.flush() print(" done. ({} tasks total)".format(task_count)) - - gdb.selected_inferior().executing = False diff --git a/crash/requirements/__init__.py b/crash/requirements/__init__.py index 5e1b217e31d..1468f05e0de 100644 --- a/crash/requirements/__init__.py +++ b/crash/requirements/__init__.py @@ -25,19 +25,13 @@ raise IncompatibleGDBError("gdb.MinSymbol") from e try: - x4 = gdb.Register + x4 = gdb.RegisterDescriptor del x4 except AttributeError as e: raise IncompatibleGDBError("gdb.Register") from e try: - x6 = gdb.Inferior.new_thread - del x6 + x5 = gdb.LinuxKernelTarget + del x5 except AttributeError as e: - raise IncompatibleGDBError("gdb.Inferior.new_thread") from e - -try: - x7 = gdb.Objfile.architecture - del x7 -except AttributeError as e: - raise IncompatibleGDBError("gdb.Objfile.architecture") from e + raise IncompatibleGDBError("gdb.LinuxKernelTarget") from e diff --git a/crash/requirements/test_target.py b/crash/requirements/test_target.py index 06e2c5e5acc..38bfd157777 100644 --- a/crash/requirements/test_target.py +++ b/crash/requirements/test_target.py @@ -1,12 +1,12 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -from typing import Tuple +from typing import Optional, Tuple import gdb PTID = Tuple[int, int, int] -class TestTarget(gdb.Target): +class TestTarget(gdb.LinuxKernelTarget): def __init__(self) -> None: super().__init__() @@ -21,13 +21,9 @@ def close(self) -> None: pass def fetch_registers(self, thread: gdb.InferiorThread, - register: gdb.Register) -> None: + register: Optional[gdb.RegisterDescriptor]) -> Optional[gdb.RegisterCollectionType]: pass # pylint: disable=unused-argument def thread_alive(self, ptid: PTID) -> bool: return True - - def setup_task(self) -> None: - ptid = (1, 1, 0) - gdb.selected_inferior().new_thread(ptid, self) diff --git a/doc-source/mock/gdb/__init__.py b/doc-source/mock/gdb/__init__.py index 525b50a2b1c..ab0b877815c 100644 --- a/doc-source/mock/gdb/__init__.py +++ b/doc-source/mock/gdb/__init__.py @@ -1,9 +1,13 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: +from typing import Dict, Optional, Union + class Target(object): - class kdump(object): - pass + pass + +class LinuxKernelTarget(Target): + class kdumpfile(object): def get_addrxlat_ctx(): pass class get_addrxlat_sys(): @@ -76,5 +80,13 @@ class NewObjFileEvent(object): class Frame(object): pass +class RegisterDescriptor: + pass + +RegisterNameType = Union[RegisterDescriptor, str] +RegisterValueType = Optional[Union[int, bytearray]] +RegisterCollectionType = Dict[RegisterNameType, RegisterValueType] + + SYMBOL_VAR_DOMAIN = 0 COMMAND_USER = 0 diff --git a/kdump/target.py b/kdump/target.py index 0949d21551b..5092b7ed1cf 100644 --- a/kdump/target.py +++ b/kdump/target.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -from typing import Tuple, Callable +from typing import Tuple, Callable, Optional import sys import shlex @@ -13,106 +13,32 @@ import gdb -TargetFetchRegisters = Callable[[gdb.InferiorThread, gdb.Register], None] +FetchRegistersCallbackType = Callable[[gdb.InferiorThread, Optional[gdb.RegisterDescriptor]], + gdb.RegisterCollectionType] +StoreRegistersCallbackType = Callable[[gdb.InferiorThread, gdb.RegisterCollectionType], None] PTID = Tuple[int, int, int] -class Target(gdb.Target): +class Target(gdb.LinuxKernelTarget): - _fetch_registers: TargetFetchRegisters + _fetch_registers: FetchRegistersCallbackType def __init__(self, debug: bool = False) -> None: super().__init__() self.debug = debug self.shortname = "kdumpfile" self.longname = "Use a Linux kernel kdump file as a target" - self.kdump: kdumpfile - self.base_offset = 0 self.register() - # pylint: disable=unused-argument - def open(self, args: str, from_tty: bool) -> None: - argv = shlex.split(args) - if len(argv) < 2: - raise gdb.GdbError("kdumpfile target requires kernel image and vmcore") - - vmlinux = argv[0] - filename = argv[1] - - try: - self.kdump = kdumpfile(file=filename) - except Exception as e: - raise gdb.GdbError("Failed to open `{}': {}" - .format(filename, str(e))) - - # pylint: disable=unsupported-assignment-operation - self.kdump.attr['addrxlat.ostype'] = 'linux' - - KERNELOFFSET = "linux.vmcoreinfo.lines.KERNELOFFSET" - try: - attr = self.kdump.attr.get(KERNELOFFSET, "0") # pylint: disable=no-member - self.base_offset = int(attr, base=16) - except (TypeError, ValueError): - pass - - # Load the kernel at the relocated address - # Unfortunately, the percpu section has an offset of 0 and - # ends up getting placed at the offset base. This is easy - # enough to handle in the percpu code. - result = gdb.execute("symbol-file {} -o {:#x}" - .format(vmlinux, self.base_offset), - to_string=True) - - if self.debug: - print(result) - - # We don't have an exec-file so we need to set the architecture - # explicitly. - arch = gdb.objfiles()[0].architecture.name() - result = gdb.execute("set architecture {}".format(arch), to_string=True) - if self.debug: - print(result) - + def open(self, name: str, from_tty: bool) -> None: + print("Opened kdump.Target") def close(self) -> None: try: self.unregister() except RuntimeError: pass - del self.kdump - - @classmethod - def report_error(cls, addr: int, length: int, error: Exception) -> None: - print("Error while reading {:d} bytes from {:#x}: {}" - .format(length, addr, str(error)), - file=sys.stderr) - - # pylint: disable=unused-argument - def xfer_partial(self, obj: int, annex: str, readbuf: bytearray, - writebuf: bytearray, offset: int, ln: int) -> int: - ret = -1 - if obj == self.TARGET_OBJECT_MEMORY: - try: - r = self.kdump.read(KDUMP_KVADDR, offset, ln) - readbuf[:] = r - ret = ln - except EOFException as e: - if self.debug: - self.report_error(offset, ln, e) - raise gdb.TargetXferEOF(str(e)) - # pylint: disable=no-member - except (NoDataException, addrxlat.exceptions.NoDataError) as e: - if self.debug: - self.report_error(offset, ln, e) - raise gdb.TargetXferUnavailable(str(e)) - except AddressTranslationException as e: - if self.debug: - self.report_error(offset, ln, e) - raise gdb.TargetXferUnavailable(str(e)) - else: - raise IOError("Unknown obj type") - return ret # pylint: disable=unused-argument def thread_alive(self, ptid: PTID) -> bool: @@ -121,15 +47,15 @@ def thread_alive(self, ptid: PTID) -> bool: def pid_to_str(self, ptid: PTID) -> str: return "pid {:d}".format(ptid[1]) - def set_fetch_registers(self, callback: TargetFetchRegisters) -> None: + def set_fetch_registers(self, callback: FetchRegistersCallbackType) -> None: self._fetch_registers = callback # type: ignore def fetch_registers(self, thread: gdb.InferiorThread, - register: gdb.Register) -> None: + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: try: return self._fetch_registers(thread, register) # type: ignore - except AttributeError: - raise NotImplementedError("Target did not define fetch_registers callback") from None + except AttributeError as e: + raise NotImplementedError(f"Target did not define fetch_registers callback: {e}") from e def prepare_to_store(self, thread: gdb.InferiorThread) -> None: pass @@ -137,7 +63,7 @@ def prepare_to_store(self, thread: gdb.InferiorThread) -> None: # We don't need to store anything; The regcache is already written. # pylint: disable=unused-argument def store_registers(self, thread: gdb.InferiorThread, - register: gdb.Register) -> None: + register: gdb.RegisterCollectionType) -> None: pass # pylint: disable=unused-argument diff --git a/test-gdb-compatibility.gdbinit b/test-gdb-compatibility.gdbinit index 1bef4e030e6..0a5c64c8bca 100644 --- a/test-gdb-compatibility.gdbinit +++ b/test-gdb-compatibility.gdbinit @@ -16,15 +16,3 @@ except IncompatibleGDBError as e: end target testtarget foo - -python -try: - gdb.execute('set print thread-events 0') - target.setup_task() - gdb.execute("thread 1", to_string=True) - sys.exit(0) -except gdb.error as e: - print(e) - print("This version of gdb is not compatible with crash-python") - sys.exit(1) -end diff --git a/tests/stubs/_gdb.pyi b/tests/stubs/_gdb.pyi index 375a5e2ef7c..07829021b81 100644 --- a/tests/stubs/_gdb.pyi +++ b/tests/stubs/_gdb.pyi @@ -2,6 +2,8 @@ # # NOTE: This dynamically typed stub was automatically generated by stubgen. +from kdumpfile import kdumpfile as kdumpfile_type + from typing import Any, Tuple, List, Optional, Dict, Iterator, Callable from typing import Union, Iterable, Sequence, NewType from typing import TypeVar, Generic @@ -258,7 +260,9 @@ class EventRegistry(Generic[EventType]): class ExitedEvent(Event): ... -class Field: ... +class Field: + enumval: int = ... + bitpos: int = ... class FinishBreakpoint(Breakpoint): return_value: Optional[Value] = ... @@ -291,17 +295,13 @@ class GdbError(Exception): ... IntValue = Union[Value, int] class Inferior: - executing: bool = ... num: int = ... pid: bool = ... progspace: Progspace = ... was_attached: bool = ... def appeared(self, pid: int) -> None: ... def architecture(self) -> Architecture: ... - def delete_thread(self, ptid: Tuple[int, int, int]) -> None: ... def is_valid(self) -> bool: ... - def new_thread(self, ptid: Tuple[int, int, int], - priv: Optional[Any] = ...) -> InferiorThread: ... def read_memory(self, address: IntValue, length: IntValue) -> Membuf: ... def search_memory(self, address: IntValue, length: IntValue, pattern: Buffer) -> int: ... @@ -318,14 +318,14 @@ class InferiorCallPreEvent(Event): ... class InferiorDeletedEvent(Event): ... class InferiorThread: - executing: bool = ... + arch: Any = ... + details: str = ... global_num: int = ... inferior: Inferior = ... info: Any = ... name: str = ... num: int = ... ptid: Tuple[int, int, int] = ... - registers: Dict[str, Register] = ... def handle(self) -> bytes: ... def is_exited(self) -> bool: ... def is_running(self) -> bool: ... @@ -418,12 +418,12 @@ class Progspace: def objfiles(self) -> List[Objfile]: ... def solib_name(self, name: int) -> Optional[str]: ... -class Register: - name: Optional[str] = ... - regnum: int = ... - size: int = ... - type: Type = ... - value: Union[Value, int] = ... +class RegisterDescriptor: + name: str = ... + +RegisterNameType = Union[str, RegisterDescriptor] +RegisterValueType = Optional[Union[int, bytearray]] +RegisterCollectionType = Dict[RegisterNameType, RegisterValueType] class RegisterChangedEvent(Event): ... @@ -505,7 +505,6 @@ class Target: def register(self) -> Any: ... def unregister(self) -> Any: ... - def stacked_target(self) -> bool: ... def open(self, argstring: str, from_tty: bool) -> None: ... def close(self) -> None: ... def info(self, thread: InferiorThread) -> str: ... @@ -516,12 +515,15 @@ class Target: def thread_alive(self, ptid: Tuple[int, int, int]) -> bool: ... def pid_to_str(self, ptid: Tuple[int, int,int]) -> str: ... def fetch_registers(self, thread: InferiorThread, - register: Register) -> None: ... + register: Optional[RegisterDescriptor]) -> Optional[RegisterCollectionType]: ... def prepare_to_store(self, thread: InferiorThread) -> None: ... def store_registers(self, thread: InferiorThread, - register: Register) -> None: ... + registers: RegisterCollectionType) -> None: ... def has_execution(self, ptid: Tuple[int, int, int]) -> bool: ... +class LinuxKernelTarget(Target): + kdumpfile: kdumpfile_type = ... + class TargetXferEOF(EOFError): ... class TargetXferUnavailable(LookupError): ... From ce0e6ef865c44f4df80a1cfbf1f78bdff206a3d7 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 11:44:21 -0400 Subject: [PATCH 15/26] crash.types.task: Update for Linux 5.14 Linux 5.14 repurposed the task_struct::state field. As part of the change, the field was renamed to __state. Signed-off-by: Jeff Mahoney --- crash/exceptions.py | 3 +++ crash/types/task.py | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/crash/exceptions.py b/crash/exceptions.py index d0c4d3b3cde..fe7e9d57ea3 100644 --- a/crash/exceptions.py +++ b/crash/exceptions.py @@ -17,6 +17,9 @@ class MissingSymbolError(RuntimeError): class MissingTypeError(RuntimeError): """The requested type cannot be located.""" +class MissingFieldError(RuntimeError): + """The requested field cannot be located.""" + class CorruptedError(RuntimeError): """A corrupted data structure has been encountered.""" diff --git a/crash/types/task.py b/crash/types/task.py index a0be1a6cdeb..aca75e19a1d 100644 --- a/crash/types/task.py +++ b/crash/types/task.py @@ -6,7 +6,7 @@ import gdb from crash.exceptions import InvalidArgumentError, ArgumentTypeError -from crash.exceptions import UnexpectedGDBTypeError +from crash.exceptions import UnexpectedGDBTypeError, MissingFieldError from crash.util import array_size, struct_has_member from crash.util.symbols import Types, Symvals, SymbolCallbacks from crash.types.list import list_for_each_entry @@ -51,6 +51,8 @@ class TaskStateFlags: TASK_NEW: int = TASK_FLAG_UNINITIALIZED TASK_IDLE: int = TASK_FLAG_UNINITIALIZED + _state_field: str = 'state' + def __init__(self) -> None: raise NotImplementedError("This class is not meant to be instantiated") @@ -225,6 +227,8 @@ class LinuxTask: _get_rss: Callable[['LinuxTask'], int] _get_last_run: Callable[['LinuxTask'], int] + _state_field: str + def __init__(self, task_struct: gdb.Value) -> None: self._init_task_types(task_struct) @@ -263,8 +267,15 @@ def _init_task_types(cls, task: gdb.Value) -> None: # within gdb. Equality requires a deep comparison rather than # a simple pointer comparison. types.override('struct task_struct', task.type) - fields = types.task_struct_type.fields() + fields = [x.name for x in types.task_struct_type.fields()] cls._task_state_has_exit_state = 'exit_state' in fields + if 'state' in fields: + cls._state_field = 'state' + elif '__state' in fields: + cls._state_field = '__state' + else: + raise MissingFieldError("No way to resolve task_struct.state") + cls._pick_get_rss() cls._pick_last_run() cls._valid = True @@ -348,7 +359,7 @@ def task_state(self) -> int: Returns: :obj:`int`: The state flags for this task. """ - state = int(self.task_struct['state']) + state = int(self.task_struct[self._state_field]) if self._task_state_has_exit_state: state |= int(self.task_struct['exit_state']) return state From c2f314b45d82271042528a51a878117e13845a3f Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 11:46:47 -0400 Subject: [PATCH 16/26] crash.kernel: Add support for zstd compressed modules Modules on SUSE products are now shipped compressed with zstd and need special handling. This commit splits up the module loading code so that we can handle these more easily by decompressing the module files into a temporary director for loading and then cleaning up the mess afterward. Signed-off-by: Jeff Mahoney --- crash/kernel.py | 183 ++++++++++++++++++++++++++++-------------------- setup.py | 2 +- 2 files changed, 109 insertions(+), 76 deletions(-) diff --git a/crash/kernel.py b/crash/kernel.py index bb959009203..d2fd9eb98f7 100644 --- a/crash/kernel.py +++ b/crash/kernel.py @@ -1,18 +1,22 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -from typing import Pattern, Union, List, Dict, Any, Optional +from typing import Pattern, Union, List, Dict, Any, Optional, BinaryIO import sys import re import fnmatch import os.path +import tempfile from elftools.elf.elffile import ELFFile import gdb -import kdump.target +# This is from the C extension and published via __all__; pylint bug? +# pylint: disable=no-name-in-module +from zstd import decompress as zstd_decompress +import kdump.target import crash import crash.arch import crash.arch.x86_64 @@ -382,20 +386,19 @@ def extract_vermagic(self) -> str: return self._get_minsymbol_as_string('vermagic') - def extract_modinfo_from_module(self, modpath: str) -> Dict[str, str]: + def extract_modinfo_from_module(self, modfile: BinaryIO) -> Dict[str, str]: """ Returns the modinfo from a module file Args: - modpath: The path to the module file. + modpath: An open module file Returns: dict: A dictionary containing the names and values of the modinfo variables. """ - f = open(modpath, 'rb') - elf = ELFFile(f) + elf = ELFFile(modfile) modinfo = elf.get_section_by_name('.modinfo') d = {} @@ -406,7 +409,6 @@ def extract_modinfo_from_module(self, modpath: str) -> Dict[str, str]: d[val[0:eq]] = val[eq + 1:] del elf - f.close() return d def _get_module_sections(self, module: gdb.Value) -> str: @@ -415,8 +417,9 @@ def _get_module_sections(self, module: gdb.Value) -> str: out.append("-s {} {:#x}".format(name, addr)) return " ".join(out) - def _check_module_version(self, modpath: str, module: gdb.Value) -> None: - modinfo = self.extract_modinfo_from_module(modpath) + def _check_module_version(self, modfile: BinaryIO, module: gdb.Value) -> None: + modinfo = self.extract_modinfo_from_module(modfile) + modpath = modfile.name vermagic = modinfo.get('vermagic', None) @@ -433,6 +436,56 @@ def _check_module_version(self, modpath: str, module: gdb.Value) -> None: raise _ModSourceVersionMismatchError(modpath, mi_srcversion, mod_srcversion) + def _try_load_module(self, modname: str, module: gdb.Value, modfile: BinaryIO, + verbose: bool = False, debug: bool = False) -> gdb.Objfile: + self._check_module_version(modfile, module) + + modpath = modfile.name + + if 'module_core' in module.type: + addr = int(module['module_core']) + else: + addr = int(module['core_layout']['base']) + + if debug: + print(f"Loading {modpath} at {addr:#x} from {modname}") + elif verbose: + print(f"Loading {modname} at {addr:#x}") + else: + print(".", end='') + sys.stdout.flush() + + sections = self._get_module_sections(module) + + percpu = int(module['percpu']) + if percpu > 0: + sections += " -s .data..percpu {:#x}".format(percpu) + + try: + result = gdb.execute("add-symbol-file {} {:#x} {}" + .format(modpath, addr, sections), + to_string=True) + except gdb.error as e: + raise CrashKernelError("Error while loading module `{}': {}" + .format(modname, str(e))) from e + if debug: + print(result) + + return gdb.lookup_objfile(modpath) + + def try_load_module(self, modname: str, module: gdb.Value, modpath: str, + tmpdirname: str, + verbose: bool = False, debug: bool = False) -> gdb.Objfile: + if modpath.endswith(".zst"): + with open(modpath, 'rb') as cmodfile: + with open(os.path.join(tmpdirname, modname + ".ko"), 'w+b') as modfile: + modfile.write(zstd_decompress(cmodfile.read())) + return self._try_load_module(modname, module, modfile, debug) + else: + with open(modpath, 'rb') as modfile: + return self._try_load_module(modname, module, modfile, debug) + + def load_modules(self, verbose: bool = False, debug: bool = False) -> None: """ Load modules (including debuginfo) into the crash session. @@ -450,81 +503,57 @@ def load_modules(self, verbose: bool = False, debug: bool = False) -> None: This does not include a failure to locate a module or its debuginfo. """ - print("Loading modules for {}".format(utsname.release), end='') + count = 0 + for module in for_each_module(): + count += 1 + print(f"Loading {count} modules for {utsname.release}", end='') if verbose: print(":", flush=True) + else: + print(".", end='', flush=True) failed = 0 loaded = 0 - pause_objfile_callbacks() - for module in for_each_module(): - modname = "{}".format(module['name'].string()) - modfname = "{}.ko".format(modname) - found = False - for path in self.module_path: - try: - modpath = self._find_module_file(modfname, path) - except _NoMatchingFileError: - continue - - try: - self._check_module_version(modpath, module) - except _ModinfoMismatchError as e: - if verbose: - print(str(e)) - continue - - found = True + pause_objfile_callbacks() + with tempfile.TemporaryDirectory() as tmpdirname: + for module in for_each_module(): + modname = module['name'].string() + modfname = f"{modname}.ko" + objfile = None + for path in self.module_path: + + try: + modpath = self._find_module_file(modfname, path) + except _NoMatchingFileError: + continue + + try: + objfile = self.try_load_module(modname, module, modpath, + tmpdirname, verbose, debug) + except (_ModinfoMismatchError, OSError) as e: + if verbose: + print(f"Module open failed: {str(e)}") + continue + + if not objfile.has_symbols(): + self._load_module_debuginfo(objfile, modpath, verbose) + elif debug: + print(" + has debug symbols") + break - if 'module_core' in module.type: - addr = int(module['module_core']) + if objfile: + if not objfile.has_symbols(): + print("Couldn't find debuginfo for {}".format(modname)) + loaded += 1 else: - addr = int(module['core_layout']['base']) + if failed == 0: + print() + print("Couldn't find module file for {}".format(modname)) + failed += 1 - if debug: - print("Loading {} at {:#x}".format(modpath, addr)) - elif verbose: - print("Loading {} at {:#x}".format(modname, addr)) - else: + if (loaded + failed) % 10 == 10: print(".", end='') sys.stdout.flush() - - sections = self._get_module_sections(module) - - percpu = int(module['percpu']) - if percpu > 0: - sections += " -s .data..percpu {:#x}".format(percpu) - - try: - result = gdb.execute("add-symbol-file {} {:#x} {}" - .format(modpath, addr, sections), - to_string=True) - except gdb.error as e: - raise CrashKernelError("Error while loading module `{}': {}" - .format(modname, str(e))) from e - if debug: - print(result) - - objfile = gdb.lookup_objfile(modpath) - if not objfile.has_symbols(): - self._load_module_debuginfo(objfile, modpath, verbose) - elif debug: - print(" + has debug symbols") - - break - - if not found: - if failed == 0: - print() - print("Couldn't find module file for {}".format(modname)) - failed += 1 - else: - if not objfile.has_symbols(): - print("Couldn't find debuginfo for {}".format(modname)) - loaded += 1 - if (loaded + failed) % 10 == 10: - print(".", end='') - sys.stdout.flush() print(" done. ({} loaded".format(loaded), end='') if failed: print(", {} failed)".format(failed)) @@ -552,6 +581,8 @@ def _cache_modules_order(self, path: str) -> None: modpath = os.path.join(path, modpath) if os.path.exists(modpath): self.modules_order[path][modname] = modpath + if os.path.exists(modpath + ".zst"): + self.modules_order[path][modname] = modpath + ".zst" f.close() except OSError: pass @@ -656,7 +687,9 @@ def _load_module_debuginfo(self, objfile: gdb.Objfile, if modpath is None: raise RuntimeError("loaded objfile has no filename???") if ".gz" in modpath: - modpath = modpath.replace(".gz", "") + modpath = modpath[:-3] + elif ".zst" in modpath: + modpath = modpath[:-4] filename = "{}.debug".format(os.path.basename(modpath)) build_id_path = self.build_id_path(objfile) diff --git a/setup.py b/setup.py index 4575cd0aaeb..467adc843c7 100644 --- a/setup.py +++ b/setup.py @@ -14,7 +14,7 @@ }, python_requires='>=3.6', - install_requires = [ 'pyelftools', 'addrxlat' ], + install_requires = [ 'pyelftools', 'addrxlat', 'zstd' ], author = "Jeff Mahoney", author_email = "jeffm@suse.com", From 4985a315defeb7c68e8ec7af50ca558eb55ea876 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 22 Jul 2022 11:49:13 -0400 Subject: [PATCH 17/26] crash.arch.x86_64: fix stack setup on kernels using __switch_to_asm The frame offset being calculated was incorrect. Signed-off-by: Jeff Mahoney --- crash/arch/x86_64.py | 1 - 1 file changed, 1 deletion(-) diff --git a/crash/arch/x86_64.py b/crash/arch/x86_64.py index 03de4342614..e689e513bbc 100644 --- a/crash/arch/x86_64.py +++ b/crash/arch/x86_64.py @@ -154,7 +154,6 @@ def setup_scheduled_frame_offset(self, task: gdb.Value) -> None: m = callq.search(insn) if m and m.group(1) == "__switch_to_asm": - self._frame_offset = rsp - orig_rsp + 1 self._scheduled_rip = val return From cc75e2b95280fc716f26d5d4cd8e14ccf34d923d Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 28 Jul 2022 00:43:43 -0400 Subject: [PATCH 18/26] crash: use gdb.LinuxKernelTarget This commit removes crash.arch in favor of a new crash.target which is derived from gdb.LinuxKernelTarget. It simplifies startup and also uses the native implementation of libkdumpfile which improves read performance substantially. Signed-off-by: Jeff Mahoney --- Makefile | 6 +- crash.sh | 12 +- crash/addrxlat.py | 7 +- crash/arch/__init__.py | 113 -------------- crash/arch/ppc64.py | 31 ---- crash/kernel.py | 95 +----------- crash/session.py | 33 ++++- crash/target/__init__.py | 243 +++++++++++++++++++++++++++++++ crash/target/ppc64.py | 59 ++++++++ crash/{arch => target}/x86_64.py | 155 ++++++++++---------- crash/types/task.py | 51 ++++--- doc-source/conf.py | 5 - doc-source/development.rst | 1 - doc-source/testing.rst | 3 +- kdump/__init__.py | 0 kdump/target.py | 71 --------- tests/gen-import-tests.sh | 2 +- tests/run-mypy.py | 8 +- tests/test_target.py | 34 ----- 19 files changed, 446 insertions(+), 483 deletions(-) delete mode 100644 crash/arch/__init__.py delete mode 100644 crash/arch/ppc64.py create mode 100644 crash/target/__init__.py create mode 100644 crash/target/ppc64.py rename crash/{arch => target}/x86_64.py (63%) delete mode 100644 kdump/__init__.py delete mode 100644 kdump/target.py delete mode 100644 tests/test_target.py diff --git a/Makefile b/Makefile index 343208ea2e0..b729e85ce2d 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ endif all: clean build doc test doc-source-clean: - rm -f doc-source/crash/*.rst doc-source/kdump/*.rst + rm -f doc-source/crash/*.rst rm -f doc-source/commands/*.rst doc-clean: doc-source-clean @@ -54,8 +54,6 @@ textdir=$(docdir)/text doc-text-install: doc-help install -m 755 -d $(DESTDIR)$(textdir)/crash install -m 644 -t $(DESTDIR)$(textdir)/crash docs/text/crash/*.txt - install -m 755 -d $(DESTDIR)$(textdir)/kdump - install -m 644 -t $(DESTDIR)$(textdir)/kdump docs/text/kdump/*.txt install -m 644 -t $(DESTDIR)$(textdir) docs/text/*.txt htmldir=$(docdir)/html @@ -68,7 +66,7 @@ unit-tests: force-rebuild sh tests/run-tests.sh lint: force-rebuild - sh tests/run-pylint.sh $(PYLINT_ARGS) crash kdump + sh tests/run-pylint.sh $(PYLINT_ARGS) crash static-check: force-rebuild sh tests/run-static-checks.sh diff --git a/crash.sh b/crash.sh index 2404be90fdb..09277b7ab75 100755 --- a/crash.sh +++ b/crash.sh @@ -236,19 +236,11 @@ set print pretty on file $KERNEL core $VMCORE -python -from kdump.target import Target -target = Target(debug=False) -end - -target kdumpfile - python import sys import traceback try: import crash.session - from crash.kernel import CrashKernel except RuntimeError as e: print("crash-python: {}, exiting".format(str(e)), file=sys.stderr) traceback.print_exc() @@ -278,10 +270,8 @@ if len(s) > 0: module_debuginfo_path = s.split(" ") try: - kernel = CrashKernel(roots, vmlinux_debuginfo, module_path, + x = crash.session.Session(roots, vmlinux_debuginfo, module_path, module_debuginfo_path, verbose, debug) - - x = crash.session.Session(kernel, verbose=verbose, debug=debug) print("The 'pyhelp' command will list the command extensions.") except gdb.error as e: print("crash-python: {}, exiting".format(str(e)), file=sys.stderr) diff --git a/crash/addrxlat.py b/crash/addrxlat.py index a73308f6eb2..31a8867c5fa 100644 --- a/crash/addrxlat.py +++ b/crash/addrxlat.py @@ -4,8 +4,9 @@ import addrxlat import gdb + import crash -import kdump.target +import crash.target from crash.cache.syscache import utsname from crash.util import offsetof @@ -50,9 +51,7 @@ def cb_read64(self, faddr: addrxlat.FullAddress) -> int: class CrashAddressTranslation: def __init__(self) -> None: try: - target = gdb.current_target() - if not isinstance(target, kdump.target.Target): - raise TypeError("Not using kdump target") + target = crash.target.check_target() self.context = target.kdumpfile.get_addrxlat_ctx() self.system = target.kdumpfile.get_addrxlat_sys() except AttributeError: diff --git a/crash/arch/__init__.py b/crash/arch/__init__.py deleted file mode 100644 index 633bc710dfd..00000000000 --- a/crash/arch/__init__.py +++ /dev/null @@ -1,113 +0,0 @@ -# -*- coding: utf-8 -*- -# vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: - -from typing import List, Iterator, Any, Optional, Type - -import gdb -from gdb import RegisterNameType, RegisterCollectionType -from gdb import FetchRegistersCallbackType -from gdb.FrameDecorator import FrameDecorator - -import crash - -class FetchRegistersCallback: - """ - The base class from which to implement the fetch_registers callback. - - The architecture code must implement the :meth:`fetch_active` and - :meth:`fetch_scheduled` methods. - """ - def fetch_active(self, thread: gdb.InferiorThread, - register: RegisterNameType) -> RegisterCollectionType: - raise NotImplementedError("Target has no fetch_active callback") - - def fetch_scheduled(self, thread: gdb.InferiorThread, - register: RegisterNameType) -> RegisterCollectionType: - raise NotImplementedError("Target has no fetch_scheduled callback") - - def __call__(self, thread: gdb.InferiorThread, - register: RegisterNameType) -> RegisterCollectionType: - - if thread.info.active: - return self.fetch_active(thread, register) - - return self.fetch_scheduled(thread, register) - -class CrashArchitecture: - ident = "base-class" - aliases: List[str] = list() - - _fetch_registers: FetchRegistersCallbackType - - def __init__(self) -> None: - target = gdb.current_target() - if target is None: - raise ValueError("No target loaded") from None - if not isinstance(target, gdb.LinuxKernelTarget): - raise ValueError("Incorrect target loaded") from None - - try: - target.set_fetch_registers(self._fetch_registers()) - except AttributeError: - raise NotImplementedError("No fetch_registers callback defined") from None - - @classmethod - def set_fetch_registers(cls, callback: FetchRegistersCallbackType) -> None: - """ - Set a fetch_regisers callback for the Target to use. - - Args: - callback: A Callable that accepts a :obj:`gdb.InferiorThread` and - :obj:`gdb.Register` and populates the requested registers for - the specified thread. A register with the seemingly invalid - register number of -1 is a request to populate all registers. - """ - cls._fetch_registers = callback - - def setup_thread_info(self, thread: gdb.InferiorThread) -> None: - raise NotImplementedError("setup_thread_info not implemented") - - def get_stack_pointer(self, thread_struct: gdb.Value) -> int: - raise NotImplementedError("get_stack_pointer is not implemented") - - def setup_scheduled_frame_offset(self, task: gdb.Value) -> None: - pass - -# This keeps stack traces from continuing into userspace and causing problems. -class KernelFrameFilter: - def __init__(self, address: int) -> None: - self.name = "KernelFrameFilter" - self.priority = 100 - self.enabled = True - self.address = address - gdb.frame_filters[self.name] = self - - def filter(self, frame_iter: Iterator[Any]) -> Any: - return KernelAddressIterator(frame_iter, self.address) - -class KernelAddressIterator: - def __init__(self, ii: Iterator, address: int) -> None: - self.input_iterator = ii - self.address = address - - def __iter__(self) -> Any: - return self - - def __next__(self) -> Any: - frame = next(self.input_iterator) - - if frame.inferior_frame().pc() < self.address: - raise StopIteration - - return frame - -architectures = {} -def register_arch(arch: Type[CrashArchitecture]) -> None: - architectures[arch.ident] = arch - for ident in arch.aliases: - architectures[ident] = arch - -def get_architecture(archname: str) -> Type[CrashArchitecture]: - if archname in architectures: - return architectures[archname] - raise RuntimeError(f"Couldn't locate helpers for arch: {archname}") diff --git a/crash/arch/ppc64.py b/crash/arch/ppc64.py deleted file mode 100644 index 07da586cad7..00000000000 --- a/crash/arch/ppc64.py +++ /dev/null @@ -1,31 +0,0 @@ -# -*- coding: utf-8 -*- -# vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: - -import gdb - -from crash.arch import CrashArchitecture, KernelFrameFilter, register_arch -from crash.arch import FetchRegistersCallback - -class FR_Placeholder(FetchRegistersCallback): # pylint: disable=abstract-method - pass - -class Powerpc64Architecture(CrashArchitecture): - ident = "powerpc:common64" - aliases = ["ppc64", "elf64-powerpc"] - - _fetch_registers = FR_Placeholder - - def __init__(self) -> None: - super(Powerpc64Architecture, self).__init__() - # Stop stack traces with addresses below this - self.filter = KernelFrameFilter(0xffff000000000000) - - def setup_thread_info(self, thread: gdb.InferiorThread) -> None: - task = thread.info.task_struct - thread.info.set_thread_info(task['thread_info'].address) - - @classmethod - def get_stack_pointer(cls, thread_struct: gdb.Value) -> int: - return int(thread_struct['ksp']) - -register_arch(Powerpc64Architecture) diff --git a/crash/kernel.py b/crash/kernel.py index d2fd9eb98f7..fd1661e4b0f 100644 --- a/crash/kernel.py +++ b/crash/kernel.py @@ -16,14 +16,11 @@ # pylint: disable=no-name-in-module from zstd import decompress as zstd_decompress -import kdump.target import crash -import crash.arch -import crash.arch.x86_64 -import crash.arch.ppc64 +import crash.target from crash.types.module import for_each_module, for_each_module_section -from crash.util import get_symbol_value, get_typed_pointer -from crash.util.symbols import Types, Symvals, Symbols +from crash.util import get_symbol_value +from crash.util.symbols import Types from crash.exceptions import MissingSymbolError, InvalidArgumentError from crash.infra.callback import pause_objfile_callbacks, unpause_objfile_callbacks from crash.cache.syscache import utsname @@ -133,19 +130,6 @@ class CrashKernel: """ types = Types(['char *']) - symvals = Symvals(['init_task']) - symbols = Symbols(['runqueues']) - - def check_target(self) -> kdump.target.Target: - target = gdb.current_target() - - if target is None: - raise ValueError("No current target") - - if not isinstance(target, kdump.target.Target): - raise ValueError(f"Current target {type(target)} is not supported") - - return target # pylint: disable=unused-argument def __init__(self, roots: PathSpecifier = None, @@ -154,7 +138,7 @@ def __init__(self, roots: PathSpecifier = None, module_debuginfo_path: PathSpecifier = None, verbose: bool = False, debug: bool = False) -> None: - self.target = self.check_target() + self.target = crash.target.check_target() self.findmap: Dict[str, Dict[Any, Any]] = dict() self.modules_order: Dict[str, Dict[str, str]] = dict() @@ -193,16 +177,6 @@ def __init__(self, roots: PathSpecifier = None, self.vermagic = self.extract_vermagic() - archname = crash.archname() - try: - archclass = crash.arch.get_architecture(archname) - except RuntimeError as e: - raise CrashKernelError(str(e)) from e - - self.arch = archclass() - - self.crashing_thread: Optional[gdb.InferiorThread] = None - def _setup_roots(self, roots: PathSpecifier = None, verbose: bool = False) -> None: if roots is None: @@ -707,64 +681,3 @@ def _load_module_debuginfo(self, objfile: gdb.Objfile, if self._try_load_debuginfo(objfile, filepath, verbose): break - - def setup_tasks(self) -> None: - """ - Populate GDB's thread list using the kernel's task lists - - This method will iterate over the kernel's task lists, create a - LinuxTask object, and create a gdb thread for each one. The - threads will be built so that the registers are ready to be - populated, which allows symbolic stack traces to be made available. - """ - from crash.types.percpu import get_percpu_vars - from crash.types.task import LinuxTask, types as task_types - import crash.cache.tasks # pylint: disable=redefined-outer-name - gdb.execute('set print thread-events 0') - - rqs = get_percpu_vars(self.symbols.runqueues) - rqscurrs = {int(x["curr"]) : k for (k, x) in rqs.items()} - - print("Loading tasks...", end='') - sys.stdout.flush() - - task_count = 0 - try: - crashing_cpu = int(get_symbol_value('crashing_cpu')) - except MissingSymbolError: - crashing_cpu = -1 - - kdumpfile = self.target.kdumpfile - task_struct_p_type = task_types.task_struct_type.pointer() - - for thread in gdb.selected_inferior().threads(): - task_address = thread.ptid[2] - - task = get_typed_pointer(task_address, task_struct_p_type) - - ltask = LinuxTask(task.dereference()) - - active = task_address in rqscurrs - if active: - cpu = rqscurrs[task_address] - regs = kdumpfile.attr.cpu[cpu].reg - ltask.set_active(cpu, regs) - else: - self.arch.setup_scheduled_frame_offset(task) - - thread.info = ltask - thread.arch = self.arch - if active and cpu == crashing_cpu: - self.crashing_thread = thread - - self.arch.setup_thread_info(thread) - ltask.attach_thread(thread) - ltask.set_get_stack_pointer(self.arch.get_stack_pointer) - - crash.cache.tasks.cache_task(ltask) - - task_count += 1 - if task_count % 100 == 0: - print(".", end='') - sys.stdout.flush() - print(" done. ({} tasks total)".format(task_count)) diff --git a/crash/session.py b/crash/session.py index 9f912e8b847..e47a8cd7a0e 100644 --- a/crash/session.py +++ b/crash/session.py @@ -1,10 +1,16 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: +from typing import List, Union + import gdb from crash.infra import autoload_submodules -from crash.kernel import CrashKernel, CrashKernelError +import crash.target +import crash.target.ppc64 +import crash.target.x86_64 + +PathSpecifier = Union[List[str], str] class Session: """ @@ -20,27 +26,38 @@ class Session: debug (optional, default=False): Whether to enable verbose debugging output """ - def __init__(self, kernel: CrashKernel, verbose: bool = False, - debug: bool = False) -> None: + def __init__(self, roots: PathSpecifier = None, + vmlinux_debuginfo: PathSpecifier = None, + module_path: PathSpecifier = None, + module_debuginfo_path: PathSpecifier = None, + verbose: bool = False, debug: bool = False) -> None: print("crash-python initializing...") - self.kernel = kernel + + self.debug = debug + self.verbose = verbose + + target = crash.target.setup_target() + from crash.kernel import CrashKernel, CrashKernelError + + self.kernel = CrashKernel(roots, vmlinux_debuginfo, module_path, + module_debuginfo_path, verbose, debug) autoload_submodules('crash.cache') autoload_submodules('crash.subsystem') autoload_submodules('crash.commands') try: - self.kernel.setup_tasks() + print("Loading modules") self.kernel.load_modules(verbose=verbose, debug=debug) except CrashKernelError as e: print(str(e)) print("Further debugging may not be possible.") return - if self.kernel.crashing_thread: + if target.crashing_thread: try: result = gdb.execute("thread {}" - .format(self.kernel.crashing_thread.num), + .format(target.crashing_thread.num), to_string=True) if debug: print(result) @@ -51,5 +68,5 @@ def __init__(self, kernel: CrashKernel, verbose: bool = False, return print("Backtrace from crashing task (PID {:d}):" - .format(self.kernel.crashing_thread.ptid[1])) + .format(target.crashing_thread.ptid[1])) gdb.execute("where") diff --git a/crash/target/__init__.py b/crash/target/__init__.py new file mode 100644 index 00000000000..320d002e7de --- /dev/null +++ b/crash/target/__init__.py @@ -0,0 +1,243 @@ +# -*- coding: utf-8 -*- +# vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: + +from typing import Any, Iterator, List, Optional, Tuple, Type + +import abc +import sys + +import gdb + +from crash.exceptions import MissingSymbolError +import crash.infra.callback + +from crash.types.percpu import get_percpu_vars +from crash.util.symbols import Symbols, Symvals +from crash.util import get_typed_pointer + +symbols = Symbols(['runqueues']) +symvals = Symvals(['crashing_cpu']) + +class IncorrectTargetError(ValueError): + """Incorrect target implementation for this kernel""" + pass + +PTID = Tuple[int, int, int] + +# This keeps stack traces from continuing into userspace and causing problems. +class KernelFrameFilter: + def __init__(self, address: int) -> None: + self.name = "KernelFrameFilter" + self.priority = 100 + self.enabled = True + self.address = address + gdb.frame_filters[self.name] = self + + def filter(self, frame_iter: Iterator[Any]) -> Any: + return KernelAddressIterator(frame_iter, self.address) + +class KernelAddressIterator: + def __init__(self, ii: Iterator, address: int) -> None: + self.input_iterator = ii + self.address = address + + def __iter__(self) -> Any: + return self + + def __next__(self) -> Any: + frame = next(self.input_iterator) + + if frame.inferior_frame().pc() < self.address: + raise StopIteration + + return frame + +# A working target will be a mixin composed of a class derived from +# TargetBase and TargetFetchRegistersBase + +class TargetBase(gdb.LinuxKernelTarget, metaclass=abc.ABCMeta): + def __init__(self, debug: int = 0) -> None: + super().__init__() + + self.debug = debug + self.shortname = "Crash-Python Linux Target" + self.longname = "Use a Core file as a Linux Kernel Target" + self.ready = False + + self.crashing_thread: Optional[gdb.InferiorThread] = None + + def open(self, name: str, from_tty: bool) -> None: + if not self.fetch_registers_usable(): + raise IncorrectTargetError("Not usable") + + if not gdb.objfiles()[0].has_symbols(): + raise ValueError("Cannot debug kernel without symbol table") + + super().open(name, from_tty) + + crash.infra.callback.target_ready() + + self.setup_tasks() + + def setup_tasks(self) -> None: + # pylint complains about this. It's ugly but putting the import within + # setup_tasks breaks the cycle. + # pylint: disable=cyclic-import + from crash.types.task import LinuxTask, types as task_types + import crash.cache.tasks # pylint: disable=redefined-outer-name + print("Loading tasks...", end="") + sys.stdout.flush() + + rqs = get_percpu_vars(symbols.runqueues) + rqscurrs = {int(x["curr"]) : k for (k, x) in rqs.items()} + + task_count = 0 + try: + crashing_cpu = symvals.crashing_cpu + except MissingSymbolError: + crashing_cpu = -1 + + task_struct_p_type = task_types.task_struct_type.pointer() + for thread in gdb.selected_inferior().threads(): + task_address = thread.ptid[2] + + task = get_typed_pointer(task_address, task_struct_p_type) + ltask = LinuxTask(task.dereference()) + + active = task_address in rqscurrs + if active: + cpu = rqscurrs[task_address] + regs = self.kdumpfile.attr.cpu[cpu].reg + ltask.set_active(cpu, regs) + + thread.info = ltask + if active and cpu == crashing_cpu: + self.crashing_thread = thread + + self.arch_setup_thread(thread) + ltask.attach_thread(thread) + + crash.cache.tasks.cache_task(ltask) + + task_count += 1 + if task_count % 100 == 0: + print(".", end='') + sys.stdout.flush() + print(" done. ({} tasks total)".format(task_count)) + + def close(self) -> None: + pass + + # pylint: disable=unused-argument + def thread_alive(self, ptid: PTID) -> bool: + return True + + # pylint: disable=unused-argument + def prepare_to_store(self, thread: gdb.InferiorThread) -> None: + pass + + @abc.abstractmethod + def fetch_registers_usable(self) -> bool: + pass + + @abc.abstractmethod + def fetch_registers(self, thread: gdb.InferiorThread, + register: Optional[gdb.RegisterDescriptor]) -> Optional[gdb.RegisterCollectionType]: + pass + + # pylint: disable=unused-argument + def store_registers(self, thread: gdb.InferiorThread, registers: gdb.RegisterCollectionType) -> None: + raise TypeError("This target is read-only.") + + # pylint: disable=unused-argument + def has_execution(self, ptid: PTID) -> bool: + return False + + @abc.abstractmethod + def arch_setup_thread(self, thread: gdb.InferiorThread) -> None: + pass + + @abc.abstractmethod + def get_stack_pointer(self, thread: gdb.InferiorThread) -> int: + pass + +class TargetFetchRegistersBase(metaclass=abc.ABCMeta): + """ + The base class from which to implement the fetch_registers callback. + + The architecture code must implement the :meth:`fetch_active` and + :meth:`fetch_scheduled` methods. + """ + _enabled: bool = False + + def __init__(self) -> None: + super().__init__() + self.fetching: bool = False + + # pylint: disable=unused-argument + @classmethod + def enable(cls, unused: Optional[gdb.Type] = None) -> None: + cls._enabled = True + + @classmethod + def fetch_registers_usable(cls) -> bool: + return cls._enabled + + @abc.abstractmethod + def fetch_active(self, thread: gdb.InferiorThread, + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: + pass + + @abc.abstractmethod + def fetch_scheduled(self, thread: gdb.InferiorThread, + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: + pass + + def fetch_registers(self, thread: gdb.InferiorThread, + register: Optional[gdb.RegisterDescriptor]) -> Optional[gdb.RegisterCollectionType]: + ret: Optional[gdb.RegisterCollectionType] = None + + # Don't recurse, but don't fail either + if self.fetching: + return None + + self.fetching = True + try: + if thread.info.active: + ret = self.fetch_active(thread, register) + else: + ret = self.fetch_scheduled(thread, register) + except AttributeError: + # We still want to be able to list the threads even if we haven't + # setup tasks. + ret = None + + self.fetching = False + return ret + +_targets: List[Type[TargetBase]] = [] +def register_target(new_target: Type[TargetBase]) -> None: + _targets.append(new_target) + +def setup_target() -> TargetBase: + for target in _targets: + t = None + try: + t = target() + t.open("", False) + return t + except IncorrectTargetError: + del t + + raise IncorrectTargetError("Could not identify target implementation for this kernel") + +def check_target() -> TargetBase: + target = gdb.current_target() + + if target is None: + raise ValueError("No current target") + + if not isinstance(target, TargetBase): + raise ValueError(f"Current target {type(target)} is not supported") + + return target diff --git a/crash/target/ppc64.py b/crash/target/ppc64.py new file mode 100644 index 00000000000..ae54f2930d1 --- /dev/null +++ b/crash/target/ppc64.py @@ -0,0 +1,59 @@ +# -*- coding: utf-8 -*- +# vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: + +from typing import Optional + +import gdb + +import crash.target +from crash.target import register_target +from crash.target import KernelFrameFilter + +class _FetchRegistersBase(crash.target.TargetFetchRegistersBase): + def __init__(self) -> None: + super().__init__() + self.filter: KernelFrameFilter + + def fetch_active(self, thread: gdb.InferiorThread, + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: + registers = {} + task = thread.info + for reg in task.regs: + if (reg == "pc" and register is not None and + register.name != "pc"): + continue + try: + registers[reg] = task.regs[reg] + except KeyError: + pass + + return registers + + def fetch_scheduled(self, thread: gdb.InferiorThread, + register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: + registers: gdb.RegisterCollectionType = {} + return registers + +# pylint: disable=abstract-method +class PPC64TargetBase(crash.target.TargetBase): + ident = "powerpc:common64" + aliases = ["ppc64", "elf64-powerpc"] + + def __init__(self) -> None: + super().__init__() + + # Stop stack traces with addresses below this + self.filter = KernelFrameFilter(0xffff000000000000) + + def arch_setup_thread(self, thread: gdb.InferiorThread) -> None: + task = thread.info.task_struct + thread.info.set_thread_info(task['thread_info'].address) + thread.info.set_thread_struct(task['thread']) + + def get_stack_pointer(self, thread: gdb.InferiorThread) -> int: + return int(thread.info.thread_struct['ksp']) + +class PPC64Target(_FetchRegistersBase, PPC64TargetBase): + pass + +register_target(PPC64Target) diff --git a/crash/arch/x86_64.py b/crash/target/x86_64.py similarity index 63% rename from crash/arch/x86_64.py rename to crash/target/x86_64.py index e689e513bbc..9a6dc8eb100 100644 --- a/crash/arch/x86_64.py +++ b/crash/target/x86_64.py @@ -3,12 +3,11 @@ from typing import Optional import re -import sys import gdb - -from crash.arch import CrashArchitecture, KernelFrameFilter, register_arch -from crash.arch import FetchRegistersCallback +import crash.target +from crash.target import IncorrectTargetError, register_target +from crash.target import KernelFrameFilter from crash.util.symbols import Types, MinimalSymvals from crash.util.symbols import TypeCallbacks, MinimalSymbolCallbacks @@ -17,17 +16,21 @@ msymvals = MinimalSymvals(['thread_return']) # pylint: disable=abstract-method -class _FetchRegistersBase(FetchRegistersCallback): +class _FetchRegistersBase(crash.target.TargetFetchRegistersBase): + def __init__(self) -> None: + super().__init__() + self.filter: KernelFrameFilter + def fetch_active(self, thread: gdb.InferiorThread, register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: regmap = { - "rflags" : "eflags" + "rflags" : "eflags" } registers = {} task = thread.info for reg in task.regs: if (reg == "rip" and register is not None and - register.name != "rip"): + register.name != "rip"): continue try: # vmcore uses rflags, gdb uses eflags @@ -39,22 +42,62 @@ def fetch_active(self, thread: gdb.InferiorThread, return registers -# pylint: disable=abstract-method -class _FRC_inactive_task_frame(_FetchRegistersBase): +class _FetchRegistersInactiveFrame(_FetchRegistersBase): + def __init__(self) -> None: + super().__init__() + + self._scheduled_rip: int = 0 + if not self._enabled: + raise IncorrectTargetError("Missing struct inactive_task_frame type") + + # We don't have CFI for __switch_to_asm but we do know what it looks like. + # We push 6 registers and then swap rsp, so we can just rewind back + # to __switch_to_asm getting called and then populate the registers that + # were saved on the stack. + def find_scheduled_rip(self, task: gdb.Value) -> None: + top = int(task['stack']) + 16*1024 + callq = re.compile(r"callq?.*<(\w+)>") + + rsp = task['thread']['sp'].cast(types.unsigned_long_p_type) + + count = 0 + while int(rsp) < top: + val = int(rsp.dereference()) - 5 + if val > self.filter.address: + try: + insn = gdb.execute(f"x/i {val:#x}", to_string=True) + except gdb.error: + insn = None + + if insn: + m = callq.search(insn) + if m and m.group(1) == "__switch_to_asm": + print("Set scheduled RIP") + self._scheduled_rip = val + return + + rsp += 1 + count += 1 + + raise RuntimeError("Cannot locate stack frame offset for __schedule") + + def get_scheduled_rip(self, task: gdb.Value) -> int: + if self._scheduled_rip == 0: + self.find_scheduled_rip(task) + + return self._scheduled_rip + def fetch_scheduled(self, thread: gdb.InferiorThread, register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: registers: gdb.RegisterCollectionType = {} task = thread.info.task_struct rsp = task['thread']['sp'].cast(types.unsigned_long_p_type) - - rsp = thread.arch.adjust_scheduled_frame_offset(rsp) - registers['rsp'] = rsp frame = rsp.cast(types.inactive_task_frame_p_type).dereference() - registers['rip'] = thread.arch.get_scheduled_rip() + registers['rip'] = self.get_scheduled_rip(task) registers['rbp'] = frame['bp'] registers['rbx'] = frame['bx'] registers['r12'] = frame['r12'] @@ -69,7 +112,7 @@ def fetch_scheduled(self, thread: gdb.InferiorThread, return registers -class _FRC_thread_return(_FetchRegistersBase): +class _FetchRegistersThreadReturn(_FetchRegistersBase): def fetch_scheduled(self, thread: gdb.InferiorThread, register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: registers: gdb.RegisterCollectionType = {} @@ -106,87 +149,35 @@ def fetch_scheduled(self, thread: gdb.InferiorThread, return registers -class x86_64Architecture(CrashArchitecture): +class X8664TargetBase(crash.target.TargetBase): ident = "i386:x86-64" aliases = ["x86_64"] - _frame_offset: Optional[int] = None - def __init__(self) -> None: - super(x86_64Architecture, self).__init__() + super().__init__() # Stop stack traces with addresses below this self.filter = KernelFrameFilter(0xffff000000000000) - self._scheduled_rip: int - - def setup_thread_info(self, thread: gdb.InferiorThread) -> None: + def arch_setup_thread(self, thread: gdb.InferiorThread) -> None: task = thread.info.task_struct thread_info = task['stack'].cast(types.thread_info_p_type) thread.info.set_thread_info(thread_info) + thread.info.set_thread_struct(task['thread']) - # We don't have CFI for __switch_to_asm but we do know what it looks like. - # We push 6 registers and then swap rsp, so we can just rewind back - # to __switch_to_asm getting called and then populate the registers that - # were saved on the stack. - def setup_scheduled_frame_offset(self, task: gdb.Value) -> None: - if self._frame_offset: - return - - top = int(task['stack']) + 16*1024 - callq = re.compile("callq?.*<(\w+)>") - - orig_rsp = rsp = task['thread']['sp'].cast(types.unsigned_long_p_type) - - count = 0 - while int(rsp) < top: - val = int(rsp.dereference()) - 5 - if val > self.filter.address: - try: - insn = gdb.execute(f"x/i {val:#x}", to_string=True) - except Exception as e: - rsp += 1 - count += 1 - continue - - if not insn: - continue - - m = callq.search(insn) - if m and m.group(1) == "__switch_to_asm": - self._scheduled_rip = val - return - - rsp += 1 - count += 1 - - raise RuntimeError("Cannot locate stack frame offset for __schedule") - - def adjust_scheduled_frame_offset(self, rsp: gdb.Value) -> gdb.Value: - if self._frame_offset: - return rsp + self._frame_offset - return rsp - - def get_scheduled_rip(self) -> int: - return self._scheduled_rip - - @classmethod - # pylint: disable=unused-argument - def setup_inactive_task_frame_handler(cls, inactive: gdb.Type) -> None: - cls.set_fetch_registers(_FRC_inactive_task_frame) + def get_stack_pointer(self, thread: gdb.InferiorThread) -> int: + return int(thread.info.thread_struct['sp']) - @classmethod - # pylint: disable=unused-argument - def setup_thread_return_handler(cls, inactive: gdb.Type) -> None: - cls.set_fetch_registers(_FRC_thread_return) +class X8664ThreadReturnTarget(_FetchRegistersThreadReturn, X8664TargetBase): + pass - @classmethod - def get_stack_pointer(cls, thread_struct: gdb.Value) -> int: - return int(thread_struct['sp']) +class X8664InactiveFrameTarget(_FetchRegistersInactiveFrame, X8664TargetBase): + pass -type_cbs = TypeCallbacks([('struct inactive_task_frame', - x86_64Architecture.setup_inactive_task_frame_handler)]) -msymbol_cbs = MinimalSymbolCallbacks([('thread_return', - x86_64Architecture.setup_thread_return_handler)]) +type_cbs = TypeCallbacks([('struct inactive_task_frame', _FetchRegistersInactiveFrame.enable)], + wait_for_target=False) +msymbol_cbs = MinimalSymbolCallbacks([('thread_return', _FetchRegistersThreadReturn.enable)], + wait_for_target=False) -register_arch(x86_64Architecture) +register_target(X8664ThreadReturnTarget) +register_target(X8664InactiveFrameTarget) diff --git a/crash/types/task.py b/crash/types/task.py index aca75e19a1d..6018cf7b4e2 100644 --- a/crash/types/task.py +++ b/crash/types/task.py @@ -5,6 +5,7 @@ import gdb +from crash.target import check_target from crash.exceptions import InvalidArgumentError, ArgumentTypeError from crash.exceptions import UnexpectedGDBTypeError, MissingFieldError from crash.util import array_size, struct_has_member @@ -245,6 +246,7 @@ def __init__(self, task_struct: gdb.Value) -> None: self.cpu = -1 self.regs: Dict[str, int] = dict() + self.thread_struct: gdb.Value self.thread_info: gdb.Value self.thread: gdb.InferiorThread @@ -309,6 +311,33 @@ def attach_thread(self, thread: gdb.InferiorThread) -> None: raise TypeError("Expected gdb.InferiorThread") self.thread = thread + def set_thread_struct(self, thread_struct: gdb.Value) -> None: + """ + Set the thread struct for this task + + The thread struct structure is architecture specific. This method + allows the architecture code to assign its thread struct structure + to this task. + + Args: + thread_struct: The ``struct thread_struct`` to be associated with + this task. The value must be of type ``struct thread_struct``. + """ + self.thread_struct = thread_struct + + def get_thread_struct(self) -> gdb.Value: + """ + Get the thread struct for this task + + The thread struct structure is architecture specific and so this + method abstracts its retreival. + + Returns: + :obj:`gdb.Value`: The struct thread_struct associated with this + task. The type of the value is ``struct thread_struct``. + """ + return self.thread_struct + def set_thread_info(self, thread_info: gdb.Value) -> None: """ Set the thread info for this task @@ -500,20 +529,6 @@ def is_kernel_task(self) -> bool: return False - @classmethod - def set_get_stack_pointer(cls, fn: Callable[[gdb.Value], int]) -> None: - """ - Set the stack pointer callback for this architecture - - The callback must accept a :obj:`gdb.Value` of type - ``struct thread`` and return a :obj:`int` containing the address - of the stack pointer. - - Args: - fn: The callback to use. It will be used by all tasks. - """ - setattr(cls, '_get_stack_pointer_fn', fn) - def get_stack_pointer(self) -> int: """ Get the stack pointer for this task @@ -525,12 +540,8 @@ def get_stack_pointer(self) -> int: :obj:`NotImplementedError`: The architecture hasn't provided a stack pointer callback. """ - try: - fn = getattr(self, '_get_stack_pointer_fn') - except AttributeError: - raise NotImplementedError("Architecture hasn't provided stack pointer callback") from None - - return fn(self.task_struct['thread']) + target = check_target() + return target.get_stack_pointer(self.thread) def _get_rss_field(self) -> int: return int(self.task_struct['mm']['rss'].value()) diff --git a/doc-source/conf.py b/doc-source/conf.py index 0101071bebd..020cec09be5 100644 --- a/doc-source/conf.py +++ b/doc-source/conf.py @@ -60,11 +60,6 @@ def run_apidoc(_): print(line, file=f, end='') f.close() - out_dir = os.path.join(cur_dir, "kdump") - mod_dir = os.path.join(cur_dir, "..", "kdump") - argv = [ '-M', '-e', '-H', 'Kdump Target API Reference', '-f', - '-o', out_dir, mod_dir ] - main(argv) print("*** Generating doc templates") diff --git a/doc-source/development.rst b/doc-source/development.rst index 9d26f65fd6c..06422a42f2c 100644 --- a/doc-source/development.rst +++ b/doc-source/development.rst @@ -6,7 +6,6 @@ Development api_changes testing - kdump/modules crash/modules gdb-internals diff --git a/doc-source/testing.rst b/doc-source/testing.rst index 47de9664c00..792ea650d4c 100644 --- a/doc-source/testing.rst +++ b/doc-source/testing.rst @@ -90,8 +90,7 @@ The ``lint`` target does allow several options: - ``E=1`` -- Only report errors - ``PYLINT_ARGS`` -- Override the default arguments. It will still operate - on the :py:mod:`crash` and :py:mod:`kdump` modules but no other default - arguments will be used. + on the :py:mod:`crash` modules but no other default arguments will be used. Testing with vmcores -------------------- diff --git a/kdump/__init__.py b/kdump/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/kdump/target.py b/kdump/target.py deleted file mode 100644 index 5092b7ed1cf..00000000000 --- a/kdump/target.py +++ /dev/null @@ -1,71 +0,0 @@ -# -*- coding: utf-8 -*- -# vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: - -from typing import Tuple, Callable, Optional - -import sys -import shlex - -from kdumpfile import kdumpfile, KDUMP_KVADDR -from kdumpfile.exceptions import AddressTranslationException, EOFException -from kdumpfile.exceptions import NoDataException -import addrxlat.exceptions - -import gdb - -FetchRegistersCallbackType = Callable[[gdb.InferiorThread, Optional[gdb.RegisterDescriptor]], - gdb.RegisterCollectionType] -StoreRegistersCallbackType = Callable[[gdb.InferiorThread, gdb.RegisterCollectionType], None] - -PTID = Tuple[int, int, int] - -class Target(gdb.LinuxKernelTarget): - - _fetch_registers: FetchRegistersCallbackType - - def __init__(self, debug: bool = False) -> None: - super().__init__() - self.debug = debug - self.shortname = "kdumpfile" - self.longname = "Use a Linux kernel kdump file as a target" - - self.register() - - def open(self, name: str, from_tty: bool) -> None: - print("Opened kdump.Target") - - def close(self) -> None: - try: - self.unregister() - except RuntimeError: - pass - - # pylint: disable=unused-argument - def thread_alive(self, ptid: PTID) -> bool: - return True - - def pid_to_str(self, ptid: PTID) -> str: - return "pid {:d}".format(ptid[1]) - - def set_fetch_registers(self, callback: FetchRegistersCallbackType) -> None: - self._fetch_registers = callback # type: ignore - - def fetch_registers(self, thread: gdb.InferiorThread, - register: Optional[gdb.RegisterDescriptor]) -> gdb.RegisterCollectionType: - try: - return self._fetch_registers(thread, register) # type: ignore - except AttributeError as e: - raise NotImplementedError(f"Target did not define fetch_registers callback: {e}") from e - - def prepare_to_store(self, thread: gdb.InferiorThread) -> None: - pass - - # We don't need to store anything; The regcache is already written. - # pylint: disable=unused-argument - def store_registers(self, thread: gdb.InferiorThread, - register: gdb.RegisterCollectionType) -> None: - pass - - # pylint: disable=unused-argument - def has_execution(self, ptid: PTID) -> bool: - return False diff --git a/tests/gen-import-tests.sh b/tests/gen-import-tests.sh index f5ffe202bcf..d5ff37c076d 100755 --- a/tests/gen-import-tests.sh +++ b/tests/gen-import-tests.sh @@ -13,7 +13,7 @@ import unittest class TestImports(unittest.TestCase): END -for f in $(cd $DIR ; find crash kdump -name '*.py'); do +for f in $(cd $DIR ; find crash -name '*.py'); do path=$(echo $f | sed -e 's#/__init__.py##' -e 's#.py##') name=$(echo $path | tr / .) tname=$(echo $path | tr / _) diff --git a/tests/run-mypy.py b/tests/run-mypy.py index ce164e140c4..ae3765e433e 100644 --- a/tests/run-mypy.py +++ b/tests/run-mypy.py @@ -15,13 +15,11 @@ "--disallow-untyped-globals"] try: - ret = main(None, stdout=sys.stdout, stderr=sys.stderr, args=["-p", "kdump"] + common_args) - ret2 = main(None, stdout=sys.stdout, stderr=sys.stderr, args=["-p", "crash"] + common_args) + ret = main(None, stdout=sys.stdout, stderr=sys.stderr, args=["-p", "crash"] + common_args) except TypeError: - ret = main(None, args=["-p", "kdump"] + common_args) - ret2 = main(None, args=["-p", "crash"] + common_args) + ret = main(None, args=["-p", "crash"] + common_args) -if ret or ret2: +if ret: print("static checking failed.", file=sys.stderr) sys.exit(1) diff --git a/tests/test_target.py b/tests/test_target.py deleted file mode 100644 index dd824992c64..00000000000 --- a/tests/test_target.py +++ /dev/null @@ -1,34 +0,0 @@ -# -*- coding: utf-8 -*- -# vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: - -import unittest -import gdb -import os.path -from kdump.target import Target - -class TestTarget(unittest.TestCase): - def setUp(self): - gdb.execute("file") - self.do_real_tests = os.path.exists("tests/vmcore") - - def tearDown(self): - try: - x = gdb.current_target() - del x - except: - pass - gdb.execute('target exec') - - def test_bad_file(self): - x = Target() - with self.assertRaises(gdb.error): - gdb.execute('target kdumpfile /does/not/exist') - x.unregister() - - def test_real_open_with_no_kernel(self): - if self.do_real_tests: - x = Target() - with self.assertRaises(gdb.error): - gdb.execute('target kdumpfile tests/vmcore') - x.unregister() - From 6d142e6552617c8fdde85d32343b4d10df580774 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 28 Jul 2022 09:40:45 -0400 Subject: [PATCH 19/26] storage: fix lint warnings pylint is complaining about missing names, when the names are autogenerated. Add the names with just a type but not value to silence the checker. Signed-off-by: Jeff Mahoney --- crash/subsystem/storage/__init__.py | 41 ++++++++++++++++++----------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/crash/subsystem/storage/__init__.py b/crash/subsystem/storage/__init__.py index 0651522894d..f0428e3a480 100644 --- a/crash/subsystem/storage/__init__.py +++ b/crash/subsystem/storage/__init__.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: -from typing import Iterable +from typing import Callable, Iterable import gdb from gdb.types import get_basic_type @@ -21,6 +21,13 @@ READ = 0 WRITE = 1 +# Values will be filled in via callback. These are declared here to honor +# imports for lint. +REQ_FUA: int +REQ_PREFLUSH: int +REQ_STARTED: int +REQ_SYNC: int + def dev_to_gendisk(dev: gdb.Value) -> gdb.Value: """ Converts a ``struct device`` that is embedded in a ``struct gendisk`` @@ -295,12 +302,21 @@ def rq_is_sync(request: gdb.Value) -> bool: :obj:`bool`: True for synchronous requests, False otherwise. """ return (request['cmd_flags'] & 1 == 0 or - request['cmd_flags'] & (REQ_SYNC | REQ_FUA | REQ_PREFLUSH) != 0) # type: ignore + request['cmd_flags'] & (REQ_SYNC | REQ_FUA | REQ_PREFLUSH) != 0) + + +_rq_in_flight: Callable[[gdb.Value], bool] + +def _rq_in_flight_rq_state(request: gdb.Value) -> bool: + return (request['rq_state'] != + types.enum_mq_rq_state_type['MQ_RQ_IDLE']) + +def _rq_in_flight_atomic_flags(request: gdb.Value) -> bool: + return (request['atomic_flags'] & + (1 << int(types.enum_rq_atomic_flags_type['REQ_ATOM_STARTED'].enumval)) != 0) -# This is a stub to make static checker happy. It gets overridden once 'struct -# request' is resolved. -def _rq_in_flight(request: gdb.Value) -> bool: - raise RuntimeError("struct request type not resolved yet!") +def _rq_in_flight_cmd_flags(request: gdb.Value) -> bool: + return request['cmd_flags'] & REQ_STARTED != 0 def rq_in_flight(request: gdb.Value) -> bool: """ @@ -359,18 +375,13 @@ def _export_req_flags(req_flag_bits: gdb.Type) -> None: # Check struct request and define functions based on its current form in this # kernel def _check_struct_request(request_s: gdb.Type) -> None: - global _rq_in_flight if struct_has_member(request_s, 'rq_state'): - def _rq_in_flight(request: gdb.Value) -> bool: - return (request['rq_state'] != - types.enum_mq_rq_state_type['MQ_RQ_IDLE']) + impl = _rq_in_flight_rq_state elif struct_has_member(request_s, 'atomic_flags'): - def _rq_in_flight(request: gdb.Value) -> bool: - return (request['atomic_flags'] & - (1 << int(types.enum_rq_atomic_flags_type['REQ_ATOM_STARTED'].enumval)) != 0) + impl = _rq_in_flight_atomic_flags else: - def _rq_in_flight(request: gdb.Value) -> bool: - return request['cmd_flags'] & REQ_STARTED != 0 # type: ignore + impl = _rq_in_flight_cmd_flags + globals()['_rq_in_flight'] = impl symbol_cbs = SymbolCallbacks([('disk_type', _check_types), ('part_type', _check_types)]) From 8460ecab4cf6e71c78ecf688107b5bb6dae0a43d Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 29 Jul 2022 13:57:57 -0400 Subject: [PATCH 20/26] crash.commands.kmem: silence silly pylint warning Signed-off-by: Jeff Mahoney --- crash/commands/kmem.py | 1 + 1 file changed, 1 insertion(+) diff --git a/crash/commands/kmem.py b/crash/commands/kmem.py index b7a1a573d4c..516fef814b2 100644 --- a/crash/commands/kmem.py +++ b/crash/commands/kmem.py @@ -75,6 +75,7 @@ def _find_kmem_cache(self, query: str) -> Optional[KmemCache]: pass return cache + # pylint: disable=too-many-return-statements def execute(self, args: argparse.Namespace) -> None: if args.z: self.print_zones() From 6271365b7044e765b1fc773791d0c495dae7c38a Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 29 Jul 2022 18:42:47 -0400 Subject: [PATCH 21/26] docs: workaround Sphinx bug#10701 There's a bug in Sphinx 5.1.0 (fixed in 5.1.1) that causes make docs to crash with: Handler for event 'autodoc-process-docstring' threw an exception (exception: pop from an empty deque) This commit works around it. Signed-off-by: Jeff Mahoney --- doc-source/conf.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/doc-source/conf.py b/doc-source/conf.py index 020cec09be5..bf518b5602c 100644 --- a/doc-source/conf.py +++ b/doc-source/conf.py @@ -242,3 +242,25 @@ def setup(app): 'Miscellaneous'), ] + + +# Temporary workaround for 5.1.0 bug +import sphinx +if sphinx.__version__ == '5.1.0': + # see https://github.com/sphinx-doc/sphinx/issues/10701 + # hope is it would get fixed for the next release + + # Although crash happens within NumpyDocstring, it is subclass of GoogleDocstring + # so we need to overload method there + from sphinx.ext.napoleon.docstring import GoogleDocstring + from functools import wraps + + @wraps(GoogleDocstring._consume_inline_attribute) + def _consume_inline_attribute_safe(self): + try: + return self._consume_inline_attribute_safe() + except: + return "", [] + + GoogleDocstring._consume_inline_attribute = _consume_inline_attribute_safe + From 320bb2637af0110c3614c213cae9263fea6e4012 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 29 Jul 2022 22:08:07 -0400 Subject: [PATCH 22/26] crash.infra: check mod.__file__ to make mypy happy Signed-off-by: Jeff Mahoney --- crash/infra/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crash/infra/__init__.py b/crash/infra/__init__.py index 3b15529054e..5e8f6119289 100644 --- a/crash/infra/__init__.py +++ b/crash/infra/__init__.py @@ -16,6 +16,8 @@ def autoload_submodules(caller: str, except KeyError: mod = importlib.import_module(caller) mods.append(caller) + if mod.__file__ is None: + return list() path = os.path.dirname(mod.__file__) modules = glob.glob("{}/[A-Za-z0-9_]*.py".format(path)) for modname in modules: From d6684a69d118a2a55d5e75e0fcec8fbc47fcf41d Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 2 Sep 2022 10:53:50 -0400 Subject: [PATCH 23/26] crash.kernel: use a default offset of 0xff000000 for loading modules Kernel modules have a number of sections with an LMA of 0, which ends up making those symbol names appear when NULL is used as a value in the stack trace output. Loading them with a default offset elsewhere just moves those (unused) symbols out of the way. Signed-off-by: Jeff Mahoney --- crash/kernel.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crash/kernel.py b/crash/kernel.py index fd1661e4b0f..d997237c5a4 100644 --- a/crash/kernel.py +++ b/crash/kernel.py @@ -435,6 +435,8 @@ def _try_load_module(self, modname: str, module: gdb.Value, modfile: BinaryIO, if percpu > 0: sections += " -s .data..percpu {:#x}".format(percpu) + sections += " -o 0xff000000" + try: result = gdb.execute("add-symbol-file {} {:#x} {}" .format(modpath, addr, sections), From 801154e6be2dc9ef9ba9c8c1fe63e3cbcefe6927 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 2 Sep 2022 13:38:33 -0400 Subject: [PATCH 24/26] crash.commands.dev: mark -d option as required The -d option is the only one implemented right now and 'pydev' does nothing otherwise. Signed-off-by: Jeff Mahoney --- crash/commands/dev.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crash/commands/dev.py b/crash/commands/dev.py index d388eaf5687..ced373ed364 100644 --- a/crash/commands/dev.py +++ b/crash/commands/dev.py @@ -20,7 +20,8 @@ class DevCommand(Command): def __init__(self, name: str) -> None: parser = ArgumentParser(prog=name) - parser.add_argument('-d', action='store_true', default=False) + parser.add_argument('-d', action='store_true', default=False, + required=True) super().__init__(name, parser) From 5757cc7b49aed649d9611f5cf5509222048a8445 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Fri, 2 Sep 2022 13:49:09 -0400 Subject: [PATCH 25/26] crash.subsystem.storage: update for Linux 5.11 Linux 5.11 merged hd_struct and block_device, changing how to resolve block devices from struct device. Signed-off-by: Jeff Mahoney --- crash/subsystem/storage/__init__.py | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/crash/subsystem/storage/__init__.py b/crash/subsystem/storage/__init__.py index f0428e3a480..2ee6921a2c2 100644 --- a/crash/subsystem/storage/__init__.py +++ b/crash/subsystem/storage/__init__.py @@ -6,14 +6,14 @@ import gdb from gdb.types import get_basic_type -from crash.util import container_of, struct_has_member +from crash.util import container_of, struct_has_member, InvalidComponentError from crash.util.symbols import Types, Symvals, SymbolCallbacks, TypeCallbacks from crash.types.classdev import for_each_class_device from crash.exceptions import DelayedAttributeError, InvalidArgumentError from crash.cache.syscache import kernel, jiffies_to_msec types = Types(['struct gendisk', 'struct hd_struct', 'struct device', - 'struct device_type', 'struct bdev_inode', + 'struct device_type', 'struct bdev_inode', 'struct block_device', 'struct request_queue', 'struct request', 'enum req_flag_bits', 'enum mq_rq_state', 'enum rq_atomic_flags']) symvals = Symvals(['block_class', 'blockdev_superblock', 'disk_type', @@ -28,6 +28,21 @@ REQ_STARTED: int REQ_SYNC: int +def dev_to_bdev(dev: gdb.Value) -> gdb.Value: + """ + Converts a ``struct device'' that is embedded in a ``struct block_device`` + back to the ``struct block_device``. + + Args: + dev: A ``struct device'' contained within a ``struct block_device``. + The vlaue must be of type ``struct device``. + + Returns: + :obj:`gdb.Value`: The converted block device. The value is of type + ``struct block_device``. + """ + return container_of(dev, types.block_device_type, 'bd_device') + def dev_to_gendisk(dev: gdb.Value) -> gdb.Value: """ Converts a ``struct device`` that is embedded in a ``struct gendisk`` @@ -41,7 +56,10 @@ def dev_to_gendisk(dev: gdb.Value) -> gdb.Value: :obj:`gdb.Value`: The converted gendisk. The value is of type ``struct gendisk``. """ - return container_of(dev, types.gendisk_type, 'part0.__dev') + try: + return container_of(dev, types.gendisk_type, 'part0.__dev') + except InvalidComponentError: + return dev_to_bdev(dev)['bd_disk'] def dev_to_part(dev: gdb.Value) -> gdb.Value: """ @@ -73,6 +91,9 @@ def gendisk_to_dev(gendisk: gdb.Value) -> gdb.Value: of type ``struct device``. """ + if struct_has_member(gendisk['part0'], 'bd_device'): + return gendisk['part0']['bd_device'] + return gendisk['part0']['__dev'] def part_to_dev(part: gdb.Value) -> gdb.Value: From 5ce96f70b9c7be9f4abfef365a247a1a7303e06d Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Tue, 28 Feb 2023 11:46:41 -0500 Subject: [PATCH 26/26] run-mypy.py: update main() prototype to drop unused initial argument --- tests/run-mypy.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/run-mypy.py b/tests/run-mypy.py index ae3765e433e..f4df83c6091 100644 --- a/tests/run-mypy.py +++ b/tests/run-mypy.py @@ -15,9 +15,12 @@ "--disallow-untyped-globals"] try: - ret = main(None, stdout=sys.stdout, stderr=sys.stderr, args=["-p", "crash"] + common_args) + ret = main(stdout=sys.stdout, stderr=sys.stderr, args=["-p", "crash"] + common_args) except TypeError: - ret = main(None, args=["-p", "crash"] + common_args) + try: + ret = main(None, stdout=sys.stdout, stderr=sys.stderr, args=["-p", "crash"] + common_args) + except TypeError: + ret = main(None, args=["-p", "crash"] + common_args) if ret: print("static checking failed.", file=sys.stderr)