From d43c52e7b563117ca7ceef39c8a2db173019b218 Mon Sep 17 00:00:00 2001 From: Serapheim Dimitropoulos Date: Tue, 1 Sep 2020 17:57:51 -0700 Subject: [PATCH 1/3] Fix latest pylint errors Pylint started being more strict about the use of Python's raise-from syntax explained here: https://www.python.org/dev/peps/pep-3134/ The main idea is to provide a proper traceback whenever an exception is caught and rethrown. Specifically in our case though the use of raise-from shouldn't make any difference for us because SDB is a REPL and prints the error messages of our sdb-defined exceptions (which are basically faulty exits of SDB commands). --- sdb/command.py | 14 ++++++++------ sdb/commands/echo.py | 4 ++-- sdb/commands/help.py | 4 ++-- sdb/commands/internal/table.py | 7 ------- sdb/commands/linux/per_cpu.py | 5 +++-- sdb/commands/stacks.py | 8 ++++---- sdb/pipeline.py | 4 ++-- 7 files changed, 21 insertions(+), 25 deletions(-) diff --git a/sdb/command.py b/sdb/command.py index 9aa49726..bc75b046 100644 --- a/sdb/command.py +++ b/sdb/command.py @@ -328,7 +328,8 @@ def call(self, objs: Iterable[drgn.Object]) -> Iterable[drgn.Object]: yield from self.__invalid_memory_objects_check( result, not issubclass(self.__class__, SingleInputCommand)) except drgn.FaultError as err: - raise CommandError(self.name, f"invalid memory access: {str(err)}") + raise CommandError(self.name, + f"invalid memory access: {str(err)}") from err class SingleInputCommand(Command): @@ -409,15 +410,16 @@ def __init__(self, tname = " ".join(self.args.type) try: self.type = target.get_type(tname) - except LookupError: - raise CommandError(self.name, f"could not find type '{tname}'") + except LookupError as err: + raise CommandError(self.name, + f"could not find type '{tname}'") from err def _call(self, objs: Iterable[drgn.Object]) -> Iterable[drgn.Object]: for obj in objs: try: yield drgn.cast(self.type, obj) except TypeError as err: - raise CommandError(self.name, str(err)) + raise CommandError(self.name, str(err)) from err class Dereference(Command): @@ -546,8 +548,8 @@ def _call(self, objs: Iterable[drgn.Object]) -> Iterable[drgn.Object]: for symbol in self.args.symbols: try: yield Address.resolve_for_address(symbol) - except KeyError: - raise SymbolNotFoundError(self.name, symbol) + except KeyError as err: + raise SymbolNotFoundError(self.name, symbol) from err class Walk(Command): diff --git a/sdb/commands/echo.py b/sdb/commands/echo.py index c8a6b7bb..77c04627 100644 --- a/sdb/commands/echo.py +++ b/sdb/commands/echo.py @@ -43,6 +43,6 @@ def _call(self, objs: Iterable[drgn.Object]) -> Iterable[drgn.Object]: for addr in self.args.addrs: try: value_ = int(addr, 0) - except ValueError: - raise sdb.CommandInvalidInputError(self.name, addr) + except ValueError as err: + raise sdb.CommandInvalidInputError(self.name, addr) from err yield sdb.create_object("void *", value_) diff --git a/sdb/commands/help.py b/sdb/commands/help.py index 133910ef..b751c857 100644 --- a/sdb/commands/help.py +++ b/sdb/commands/help.py @@ -39,8 +39,8 @@ def _call(self, objs: Iterable[drgn.Object]) -> None: if self.args.cmd is not None: try: all_cmds[self.args.cmd].help(self.args.cmd) - except KeyError: - raise sdb.error.CommandNotFoundError(self.args.cmd) + except KeyError as err: + raise sdb.error.CommandNotFoundError(self.args.cmd) from err else: cmds: Dict[str, Type[sdb.Command]] = {} for k, v in all_cmds.items(): diff --git a/sdb/commands/internal/table.py b/sdb/commands/internal/table.py index cdddc49c..5b126c91 100644 --- a/sdb/commands/internal/table.py +++ b/sdb/commands/internal/table.py @@ -27,13 +27,6 @@ class Table: __slots__ = "fields", "rjustfields", "formatters", "maxfieldlen", "lines" - # - # The "yapf" tool expects 4 spaces for the continuation lines here, - # where "pylint" expects 8 spaces. To reconcile the difference in - # expectations of both tools, we disable the pylint error, and - # adhere to the yapf format. - # - # pylint: disable=bad-continuation def __init__( self, fields: List[str], diff --git a/sdb/commands/linux/per_cpu.py b/sdb/commands/linux/per_cpu.py index de1f720a..8cbadb79 100644 --- a/sdb/commands/linux/per_cpu.py +++ b/sdb/commands/linux/per_cpu.py @@ -98,6 +98,7 @@ class LxPerCpuCounterSum(sdb.SingleInputCommand): def _call_one(self, obj: drgn.Object) -> Iterable[drgn.Object]: try: sum_ = drgn_percpu.percpu_counter_sum(obj) - except AttributeError: - raise sdb.CommandError(self.name, "input is not a percpu_counter") + except AttributeError as err: + raise sdb.CommandError(self.name, + "input is not a percpu_counter") from err yield drgn.Object(sdb.get_prog(), type="s64", value=sum_) diff --git a/sdb/commands/stacks.py b/sdb/commands/stacks.py index 769040b8..3e06efcf 100644 --- a/sdb/commands/stacks.py +++ b/sdb/commands/stacks.py @@ -295,9 +295,10 @@ def validate_args(self) -> None: # func = sdb.get_object(self.args.function) sym = sdb.get_symbol(func.address_of_()) - except KeyError: + except KeyError as err: raise sdb.CommandError( - self.name, f"symbol '{self.args.function}' does not exist") + self.name, + f"symbol '{self.args.function}' does not exist") from err if func.type_.kind != drgn.TypeKind.FUNCTION: raise sdb.CommandError( self.name, f"'{self.args.function}' is not a function") @@ -356,10 +357,9 @@ def print_header(self) -> None: # task state and program counters. Return a collection sorted by number # of tasks per stack. # - # Note: we disabled pyline C0330 due to https://github.com/PyCQA/pylint/issues/289 @staticmethod def aggregate_stacks( - objs: Iterable[drgn.Object] # pylint: disable=C0330 + objs: Iterable[drgn.Object] ) -> List[Tuple[Tuple[str, Tuple[int, ...]], List[drgn.Object]]]: stack_aggr: Dict[Tuple[str, Tuple[int, ...]], List[drgn.Object]] = defaultdict(list) diff --git a/sdb/pipeline.py b/sdb/pipeline.py index 3a73883f..6ccba8e4 100644 --- a/sdb/pipeline.py +++ b/sdb/pipeline.py @@ -107,7 +107,7 @@ def invoke(myprog: drgn.Program, first_input: Iterable[drgn.Object], raise CommandNotFoundError(name) try: pipeline.append(get_registered_commands()[name](args, name)) - except SystemExit: + except SystemExit as cmd_exit: # # The passed in arguments to each command will be parsed in # the command object's constructor. We use "argparse" to do @@ -116,7 +116,7 @@ def invoke(myprog: drgn.Program, first_input: Iterable[drgn.Object], # SDB session, we only abort this specific pipeline by raising # a CommandArgumentsError. # - raise CommandArgumentsError(name) + raise CommandArgumentsError(name) from cmd_exit else: assert cmd_type == parser.ExpressionType.SHELL_CMD shell_cmd = cmd From 31c4375508d41ced9a7f6840c2837bdbc7349d96 Mon Sep 17 00:00:00 2001 From: Serapheim Dimitropoulos Date: Tue, 1 Sep 2020 18:07:54 -0700 Subject: [PATCH 2/3] Undo commit 9cd5599560e2ffed8b7c5cbc13588caa85c9f56e The latest version of pylint has fixed the false positives that we worked around on that commit. --- tests/integration/test_core_generic.py | 1 - tests/integration/test_linux_generic.py | 1 - tests/integration/test_spl_generic.py | 1 - tests/integration/test_zfs_generic.py | 1 - tests/unit/test_parser.py | 1 - 5 files changed, 5 deletions(-) diff --git a/tests/integration/test_core_generic.py b/tests/integration/test_core_generic.py index ec62a0b2..0a0fea28 100644 --- a/tests/integration/test_core_generic.py +++ b/tests/integration/test_core_generic.py @@ -16,7 +16,6 @@ # pylint: disable=missing-module-docstring # pylint: disable=missing-function-docstring -# pylint: disable=not-callable from typing import Any diff --git a/tests/integration/test_linux_generic.py b/tests/integration/test_linux_generic.py index 5722f9b3..5a1f00ba 100644 --- a/tests/integration/test_linux_generic.py +++ b/tests/integration/test_linux_generic.py @@ -17,7 +17,6 @@ # pylint: disable=missing-module-docstring # pylint: disable=missing-function-docstring # pylint: disable=line-too-long -# pylint: disable=not-callable from typing import Any diff --git a/tests/integration/test_spl_generic.py b/tests/integration/test_spl_generic.py index 2c43ce2e..8a40aba8 100644 --- a/tests/integration/test_spl_generic.py +++ b/tests/integration/test_spl_generic.py @@ -17,7 +17,6 @@ # pylint: disable=missing-module-docstring # pylint: disable=missing-function-docstring # pylint: disable=line-too-long -# pylint: disable=not-callable from typing import Any diff --git a/tests/integration/test_zfs_generic.py b/tests/integration/test_zfs_generic.py index c54c0196..3a295a27 100644 --- a/tests/integration/test_zfs_generic.py +++ b/tests/integration/test_zfs_generic.py @@ -17,7 +17,6 @@ # pylint: disable=missing-module-docstring # pylint: disable=missing-function-docstring # pylint: disable=line-too-long -# pylint: disable=not-callable from typing import Any diff --git a/tests/unit/test_parser.py b/tests/unit/test_parser.py index 9192b236..75626d59 100644 --- a/tests/unit/test_parser.py +++ b/tests/unit/test_parser.py @@ -15,7 +15,6 @@ # # pylint: disable=missing-docstring -# pylint: disable=not-callable from typing import List, Tuple From fb7d435b4a14e595c0a7642b4bfb7a3d8e594c85 Mon Sep 17 00:00:00 2001 From: Matthew Ahrens Date: Tue, 1 Sep 2020 13:14:50 -0700 Subject: [PATCH 3/3] update drgn type instantiation routines drgn changed the way what you instantiate a type object, so we need to update type_canonicalize(). --- sdb/target.py | 5 +++-- tests/unit/__init__.py | 10 +++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/sdb/target.py b/sdb/target.py index 162f6738..b20cc13e 100644 --- a/sdb/target.py +++ b/sdb/target.py @@ -104,12 +104,13 @@ def type_canonicalize(t: drgn.Type) -> drgn.Type: Note: function type's arguments and return types are not canonicalized. """ + global prog if t.kind == drgn.TypeKind.TYPEDEF: return type_canonicalize(t.type) if t.kind == drgn.TypeKind.POINTER: - return drgn.pointer_type(t.size, type_canonicalize(t.type)) + return prog.pointer_type(type_canonicalize(t.type), t.size) if t.kind == drgn.TypeKind.ARRAY: - return drgn.array_type(t.length, type_canonicalize(t.type)) + return prog.array_type(type_canonicalize(t.type), t.length) return t.unqualified() diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py index 6050de16..e0a7d153 100644 --- a/tests/unit/__init__.py +++ b/tests/unit/__init__.py @@ -22,13 +22,13 @@ import sdb -def create_struct_type(name: str, member_names: List[str], +def create_struct_type(prog: drgn.Program, name: str, member_names: List[str], member_types: List[drgn.Type]) -> drgn.Type: """ Creates a structure type given a list of member names and a list of types like this: ``` - create_struct_type(, [, ...], + create_struct_type(, , [, ...], [, , ...]) ``` returns a C structure: @@ -52,7 +52,7 @@ def create_struct_type(name: str, member_names: List[str], else: bit_offset += 8 * type_.size struct_size += type_.size - return drgn.struct_type(name, struct_size, member_list) + return prog.struct_type(name, struct_size, member_list) def setup_basic_mock_program() -> drgn.Program: @@ -97,13 +97,13 @@ def mock_type_find(kind: drgn.TypeKind, name: str, # More complex types are added to the mocked_types table in # an ad-hoc way here. # - struct_type = create_struct_type('test_struct', + struct_type = create_struct_type(prog, 'test_struct', ['ts_int', 'ts_voidp', 'ts_array'], [int_type, voidp_type, int_array_type]) mocked_types['test_struct'] = struct_type structp_type = prog.type('struct test_struct *') complex_struct_type = create_struct_type( - 'complex_struct', ['cs_structp', 'cs_struct', 'cs_structp_null'], + prog, 'complex_struct', ['cs_structp', 'cs_struct', 'cs_structp_null'], [structp_type, struct_type, structp_type]) mocked_types['complex_struct'] = complex_struct_type