Refactor PyVEX to pass lint (#294)

* Refactor PyVEX to pass lint

* Re-add irop_enums_to_ints to pyvex namespace

* Replace log.trace with log.debug

* Register lifters in one place instead of relying on lifter import order

* Fix pylint errors

* Rename lift.py to lift_function.py

Author: twizmwazin

.pre-commit-config.yaml

@@ -78,7 +78,7 @@ repos:
     rev: v1.10.0
     hooks:
     # Python
-    # -   id: python-use-type-annotations
+    -   id: python-use-type-annotations
     -   id: python-no-log-warn
     # Documentation
     -   id: rst-backticks
@@ -92,7 +92,7 @@ repos:
     -   id: check-builtin-literals
     -   id: check-docstring-first
 
-# -   repo: https://github.com/charliermarsh/ruff-pre-commit
-#     rev: v0.0.230
-#     hooks:
-#     - id: ruff
+-   repo: https://github.com/charliermarsh/ruff-pre-commit
+    rev: v0.0.231
+    hooks:
+    - id: ruff

make_ffi.py

@@ -1,14 +1,14 @@
-import re
+import logging
 import os
-import sys
-import cffi
-import subprocess
 import platform
+import re
+import subprocess
+import sys
 
-import logging
+import cffi
 
-l = logging.getLogger("cffier")
-l.setLevel(logging.DEBUG)
+log = logging.getLogger("cffier")
+log.setLevel(logging.DEBUG)
 
 
 def find_good_scan(questionable):
@@ -18,7 +18,7 @@ def find_good_scan(questionable):
 
     while len(questionable):
         ffi = cffi.FFI()
-        l.debug("scan - trying %d good and %d questionable", len(known_good), len(questionable))
+        log.debug("scan - trying %d good and %d questionable", len(known_good), len(questionable))
 
         candidate = known_good + questionable[:end_line]
         failed_line = -1
@@ -98,7 +98,7 @@ def doit(vex_path):
             errs.append((" ".join(cmd), -1, "does not exist"))
             continue
     else:
-        l.warning("failed commands:\n" + "\n".join("{} ({}) -- {}".format(*e) for e in errs))
+        log.warning("failed commands:\n" + "\n".join("{} ({}) -- {}".format(*e) for e in errs))
         raise Exception(
             "Couldn't process pyvex headers."
             + 'Please set CPP environmental variable to local path of "cpp".'
@@ -145,7 +145,5 @@ def get_guest_offsets(vex_path):
 
 
 if __name__ == "__main__":
-    import sys
-
     logging.basicConfig(level=logging.DEBUG)
     doit(sys.argv[1])

pyproject.toml

@@ -10,3 +10,12 @@ force-exclude = '''
   vex
 )/
 '''
+
+[tool.ruff]
+line-length = 120
+select = [
+  "E",
+  "F",
+  "I",
+  "TID",
+]

pyvex/__init__.py

@@ -4,96 +4,54 @@
 """
 __version__ = "9.2.37.dev0"
 
-if bytes is str:
-    raise Exception("This module is designed for python 3 only. Please install an older version to use python 2.")
-
-import os
-import sys
-import hashlib
-import pickle
-import tempfile
-from typing import Any
-import cffi
-from .vex_ffi import ffi_str as _ffi_str
-
-ffi = cffi.FFI()
-
-import logging
-
-logging.getLogger("pyvex").addHandler(logging.NullHandler())
-
-
-def _locate_lib(module: str, library: str) -> str:
-    """
-    Attempt to find a native library without using pkg_resources, and only fall back to pkg_resources upon failures.
-    This is because "import pkg_resources" is slow.
-
-    :return:    The full path of the native library.
-    """
-    base_dir = os.path.dirname(__file__)
-    attempt = os.path.join(base_dir, library)
-    if os.path.isfile(attempt):
-        return attempt
-
-    import pkg_resources  # pylint:disable=import-outside-toplevel
-
-    return pkg_resources.resource_filename(module, os.path.join("lib", library))
-
-
-def _parse_ffi_str():
-    hash = hashlib.md5(_ffi_str.encode("utf-8")).hexdigest()
-    cache_location = os.path.join(tempfile.gettempdir(), f"pyvex_ffi_parser_cache.{hash}")
-
-    if os.path.isfile(cache_location):
-        # load the cache
-        with open(cache_location, "rb") as f:
-            cache = pickle.loads(f.read())
-        ffi._parser._declarations = cache["_declarations"]
-        ffi._parser._int_constants = cache["_int_constants"]
-    else:
-        ffi.cdef(_ffi_str)
-        # cache the result
-        cache = {
-            "_declarations": ffi._parser._declarations,
-            "_int_constants": ffi._parser._int_constants,
-        }
-        with open(cache_location, "wb") as f:
-            f.write(pickle.dumps(cache))
-
-
-def _find_c_lib():
-    # Load the c library for calling into VEX
-    if sys.platform in ("win32", "cygwin"):
-        library_file = "pyvex.dll"
-    elif sys.platform == "darwin":
-        library_file = "libpyvex.dylib"
-    else:
-        library_file = "libpyvex.so"
-
-    pyvex_path = _locate_lib(__name__, os.path.join("lib", library_file))
-    # parse _ffi_str and use cache if possible
-    _parse_ffi_str()
-    # RTLD_GLOBAL used for sim_unicorn.so
-    lib = ffi.dlopen(pyvex_path)
-    if not lib.vex_init():
-        raise ImportError("libvex failed to initialize")
-    # this looks up all the definitions (wtf)
-    dir(lib)
-    return lib
-
-
-pvc = _find_c_lib()  # type: Any # This should be properly typed, but this seems non trivial
-
-# pylint: disable=wildcard-import
-from .enums import *
-from . import stmt, expr, const
+from . import const, expr, stmt
 from .block import IRSB, IRTypeEnv
+from .const import get_type_size, get_type_spec_size, tag_to_const_class
+from .enums import (
+    IRCallee,
+    IRRegArray,
+    VEXObject,
+    default_vex_archinfo,
+    get_enum_from_int,
+    get_int_from_enum,
+    irop_enums_to_ints,
+    vex_endness_from_string,
+)
+from .errors import PyVEXError
 from .expr import get_op_retty
-from .const import tag_to_const_class, get_type_size, get_type_spec_size
 from .lifting import lift, lifters
-from .errors import PyVEXError
+from .native import ffi, pvc
 
 # aliases....
 IRStmt = stmt
 IRExpr = expr
 IRConst = const
+
+
+__all__ = [
+    "const",
+    "expr",
+    "stmt",
+    "IRSB",
+    "IRTypeEnv",
+    "get_type_size",
+    "get_type_spec_size",
+    "irop_enums_to_ints",
+    "tag_to_const_class",
+    "IRCallee",
+    "IRRegArray",
+    "VEXObject",
+    "default_vex_archinfo",
+    "get_enum_from_int",
+    "get_int_from_enum",
+    "vex_endness_from_string",
+    "PyVEXError",
+    "get_op_retty",
+    "lift",
+    "lifters",
+    "ffi",
+    "pvc",
+    "IRStmt",
+    "IRExpr",
+    "IRConst",
+]

pyvex/block.py

@@ -1,23 +1,18 @@
 import copy
-import sys
 import itertools
+import logging
 from typing import List, Optional
 
-from pyvex.expr import IRExpr
-from pyvex.stmt import IRStmt
-
-from . import VEXObject
 from . import expr, stmt
 from .const import get_type_size
-from .stmt import WrTmp, LoadG, LLSC, Dirty, CAS, get_enum_from_int, get_int_from_enum, Exit, IMark
-from .expr import RdTmp
 from .data_ref import DataRef
+from .enums import VEXObject
 from .errors import SkipStatementsError
+from .expr import RdTmp
+from .native import pvc
+from .stmt import CAS, LLSC, Dirty, Exit, IMark, IRExpr, IRStmt, LoadG, WrTmp, get_enum_from_int, get_int_from_enum
 
-
-import logging
-
-l = logging.getLogger("pyvex.block")
+log = logging.getLogger("pyvex.block")
 
 
 class IRSB(VEXObject):
@@ -88,8 +83,10 @@ def __init__(
         :type arch:                 :class:`archinfo.Arch`
         :param max_inst:            The maximum number of instructions to lift. (See note below)
         :param max_bytes:           The maximum number of bytes to use.
-        :param num_inst:            Replaces max_inst if max_inst is None. If set to None as well, no instruction limit is used.
-        :param num_bytes:           Replaces max_bytes if max_bytes is None. If set to None as well, no  byte limit is used.
+        :param num_inst:            Replaces max_inst if max_inst is None. If set to None as well, no instruction limit
+                                    is used.
+        :param num_bytes:           Replaces max_bytes if max_bytes is None. If set to None as well, no  byte limit is
+                                    used.
         :param bytes_offset:        The offset into `data` to start lifting at. Note that for ARM THUMB mode, both
                                     `mem_addr` and `bytes_offset` must be odd (typically `bytes_offset` is set to 1).
         :param traceflags:          The libVEX traceflags, controlling VEX debug prints.
@@ -117,10 +114,10 @@ def __init__(
         self.addr = mem_addr
         self.arch = arch
 
-        self.statements = []  # type: List[IRStmt]
-        self.next = None  # type: Optional[IRExpr]
+        self.statements: List[IRStmt] = []
+        self.next: Optional[IRExpr] = None
         self._tyenv = None
-        self.jumpkind = None  # type: Optional[str]
+        self.jumpkind: Optional[str] = None
         self._direct_next = None
         self._size = None
         self._instructions = None
@@ -133,6 +130,8 @@ def __init__(
             # This is the slower path (because we need to call _from_py() to copy the content in the returned IRSB to
             # the current IRSB instance. You should always call `lift()` directly. This method is kept for compatibility
             # concerns.
+            from pyvex.lifting import lift
+
             irsb = lift(
                 data,
                 mem_addr,
@@ -225,8 +224,8 @@ def convert_tmp(tmp):
 
         def convert_expr(expr_):
             """
-            Converts a VEX expression to use tmps in the appended-block instead of the appended-to-block. Used to prevent
-            collisions in tmp numbers between the two blocks.
+            Converts a VEX expression to use tmps in the appended-block instead of the appended-to-block. Used to
+            prevent collisions in tmp numbers between the two blocks.
             :param tmp:       The VEX expression to convert
             :vartype expr:    :class:`IRExpr`
             """
@@ -304,7 +303,7 @@ def typecheck(self):
             assert self.next is not None, "Missing next expression"
             assert self.jumpkind is not None, "Missing jumpkind"
 
-            # type assertions
+            # Type assertions
             assert isinstance(self.next, expr.IRExpr), "Next expression is not an expression"
             assert type(self.jumpkind is str), "Jumpkind is not a string"
             assert self.jumpkind.startswith("Ijk_"), "Jumpkind is not a jumpkind enum"
@@ -316,7 +315,7 @@ def typecheck(self):
                 assert isinstance(st, stmt.IRStmt), "Statement %d is not an IRStmt" % i
                 try:
                     assert st.typecheck(self.tyenv), "Statement %d failed to typecheck" % i
-                except:  # pylint: disable=bare-except
+                except Exception:  # pylint: disable=bare-except
                     assert False, "Statement %d errored in typechecking" % i
 
                 if type(st) is stmt.NoOp:
@@ -331,7 +330,7 @@ def typecheck(self):
 
             assert last_imark is not None, "No IMarks present in block"
         except AssertionError as e:
-            l.debug(e.args[0])
+            log.debug(e.args[0])
             return False
         return True
 
@@ -433,7 +432,8 @@ def operations(self):
     @property
     def all_constants(self):
         """
-        Returns all constants in the block (including incrementing of the program counter) as :class:`pyvex.const.IRConst`.
+        Returns all constants in the block (including incrementing of the program counter) as
+        :class:`pyvex.const.IRConst`.
         """
         return sum((e.constants for e in self.expressions), [])
 
@@ -634,7 +634,7 @@ def lookup(self, tmp):
         Return the type of temporary variable `tmp` as an enum string
         """
         if tmp < 0 or tmp > self.types_used:
-            l.debug("Invalid temporary number %d", tmp)
+            log.debug("Invalid temporary number %d", tmp)
             raise IndexError(tmp)
         return self.types[tmp]
 
@@ -670,7 +670,3 @@ def typecheck(self):
             except ValueError:
                 return False
         return True
-
-
-from . import pvc
-from .lifting import lift