Rework syscall invocation for proper behavior under typeguard (#1672)

* Rework syscall invocation for proper behavior under typeguard

Previously, using Typeguard with Manticore would break several emulated
syscalls.  With this commit, it does not.

Some background information:

When a syscall is made from an emulated binary, Manticore uses the
syscall number to look up the appropriate Python method that models that
syscall, and then uses Python introspection to massage arguments to that
syscall model function as deemed appropriate.

Previously, this mechanism used the deprecated `inspect.getfullargspec`
to determine the number of arguments to the model function, and whether
or not it takes varargs.

However, `inspect.getfullargspec` doesn't look through wrapper
functions; it looks only at exactly the function object it is given.
This is an issue, however, when trying to inspect _decorated_ functions.
(The use of a decorator in Python introduces a _new_ function object
that wraps the decorated item.)

How did that break Manticore when using Typeguard?  It turns out that
When using Typeguard via the `--typeguard-packages=manticore` option to
`pytest`, the Typeguard plugin implicitly adds a `@typeguard.check_types`
decorator on _every_ function & method in the `manticore` package.

In this way, each syscall implementation function in Manticore ends up
with a wrapper around it, and the syscall invocation mechanism based on
`inspect.getfullargspec` would somewhat quietly cause syscalls to break.

Now, instead of using `inspect.getfullargspec`, Manticore's syscall
invocation mechansim uses the non-deprecated `inspect.signature` API to
get the needed information.  This API _does_ look through wrapper
functions. Additionally, it allows us to get rid of some conditional
logic, about whether `self` appears in a function's parameter list or
not.

* Update manticore/native/cpu/abstractcpu.py

* Get rid of the `wrapt` library

We only were using `wrapt` in a single place -- to implement the
`unimplemented` syscall decorator.  That dependency was added in #1384,
so that the old syscall mechanism could work with the decorator.

Now, with the rework of Manticore's syscall mechanism, this is no longer
necessary, and a "regular" Python decorator implemented using
`functools.wraps` should work just fine.

* Fix rewrite of `unimplemented`

* Rework platform `unimplemented` decorator, take 3

Additionally, add some extra tests for the decorator, to get better
coverage from where it is used.

* Clean up `test_sycalls.py` imports

* Fix an unbound variable error

(found with `mypy --check-untyped-defs` and some grep)

* Get rid of some unused imports

manticore/native/cpu/abstractcpu.py

@@ -23,11 +23,17 @@
 from capstone.x86 import X86_REG_ENDING
 from capstone.arm import ARM_REG_ENDING
 
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Callable, Dict, Optional, Tuple
 
 logger = logging.getLogger(__name__)
 register_logger = logging.getLogger(f"{__name__}.registers")
 
+
+def _sig_is_varargs(sig: inspect.Signature) -> bool:
+    VAR_POSITIONAL = inspect.Parameter.VAR_POSITIONAL
+    return any(p.kind == VAR_POSITIONAL for p in sig.parameters.values())
+
+
 ###################################################################################
 # Exceptions
 
@@ -309,26 +315,21 @@ def values_from(self, base):
             yield base
             base += word_bytes
 
-    def get_argument_values(self, model, prefix_args):
+    def get_argument_values(self, model: Callable, prefix_args: Tuple) -> Tuple:
         """
         Extract arguments for model from the environment and return as a tuple that
         is ready to be passed to the model.
 
-        :param callable model: Python model of the function
-        :param tuple prefix_args: Parameters to pass to model before actual ones
+        :param model: Python model of the function
+        :param prefix_args: Parameters to pass to model before actual ones
         :return: Arguments to be passed to the model
-        :rtype: tuple
         """
-        spec = inspect.getfullargspec(model)
-
-        if spec.varargs:
-            logger.warning("ABI: A vararg model must be a unary function.")
+        sig = inspect.signature(model)
+        if _sig_is_varargs(sig):
+            model_name = getattr(model, "__qualname__", "<no name>")
+            logger.warning("ABI: %s: a vararg model must be a unary function.", model_name)
 
-        nargs = len(spec.args) - len(prefix_args)
-
-        # If the model is a method, we need to account for `self`
-        if inspect.ismethod(model):
-            nargs -= 1
+        nargs = len(sig.parameters) - len(prefix_args)
 
         def resolve_argument(arg):
             if isinstance(arg, str):
@@ -343,11 +344,9 @@ def resolve_argument(arg):
         from ..models import isvariadic  # prevent circular imports
 
         if isvariadic(model):
-            arguments = prefix_args + (argument_iter,)
+            return prefix_args + (argument_iter,)
         else:
-            arguments = prefix_args + tuple(islice(argument_iter, nargs))
-
-        return arguments
+            return prefix_args + tuple(islice(argument_iter, nargs))
 
     def invoke(self, model, prefix_args=None):
         """

manticore/platforms/linux.py

@@ -9,7 +9,6 @@
 import time
 import resource
 import tempfile
-from typing import Deque, Union, List, TypeVar, cast, Optional
 
 import io
 import os
@@ -33,10 +32,12 @@
 from ..native.state import State
 from ..platforms.platform import Platform, SyscallNotImplemented, unimplemented
 
-from typing import Any, Dict, IO, List, Optional, Set, Tuple, Union
+from typing import Any, cast, Deque, Dict, IO, List, Optional, Set, Tuple, Union
+
 
 logger = logging.getLogger(__name__)
 
+
 MixedSymbolicBuffer = Union[List[Union[bytes, Expression]], bytes]
 
 

manticore/platforms/linux_syscall_stubs.py

@@ -1,6 +1,10 @@
 from ..platforms.platform import SyscallNotImplemented, unimplemented
 from .linux_syscalls import amd64
 
+import logging
+
+logger = logging.getLogger(__name__)
+
 
 class SyscallStubs:
     """
@@ -20,9 +24,11 @@ def __init__(self, *, default_to_fail=False, parent=None):
         self.parent = parent
 
     def __getattr__(self, item):
-        print(
-            "System calls should be copied and pasted into linux.py, not implemented within the stub file.",
-            "If you're seeing this message, you may have forgotten to do that.",
+        logger.warning(
+            f"Getting {item!r} attribute from {self.__class__}: "
+            "System calls should be copied and pasted into linux.py, "
+            "not implemented within the stub file. "
+            "If you're seeing this message, you may have forgotten to do that."
         )
         return getattr(self.parent, item)
 

manticore/platforms/platform.py

@@ -1,8 +1,10 @@
-import wrapt
 import logging
+
+from functools import wraps
+from typing import Any, Callable, TypeVar
+
 from ..utils.event import Eventful
 
-from typing import Callable, Dict, Tuple
 
 logger = logging.getLogger(__name__)
 
@@ -11,23 +13,29 @@ class OSException(Exception):
     pass
 
 
-@wrapt.decorator
-def unimplemented(wrapped: Callable, _instance, args: Tuple, kwargs: Dict):
-    cpu = getattr(getattr(_instance, "parent", None), "current", None)
-    addr_str = "" if cpu is None else f" at {hex(cpu.read_register('PC'))}"
-    logger.warning(
-        f"Unimplemented system call: %s: %s(%s)",
-        addr_str,
-        wrapped.__name__,
-        ", ".join(hex(a) if isinstance(a, int) else str(a) for a in args),
-    )
-    return wrapped(*args, **kwargs)
+T = TypeVar("T")
+
+
+def unimplemented(wrapped: Callable[..., T]) -> Callable[..., T]:
+    @wraps(wrapped)
+    def new_wrapped(self: Any, *args, **kwargs) -> T:
+        cpu = getattr(getattr(self, "parent", None), "current", None)
+        pc_str = "<unknown PC>" if cpu is None else hex(cpu.read_register("PC"))
+        logger.warning(
+            f"Unimplemented system call: %s: %s(%s)",
+            pc_str,
+            wrapped.__name__,
+            ", ".join(hex(a) if isinstance(a, int) else str(a) for a in args),
+        )
+        return wrapped(self, *args, **kwargs)
+
+    return new_wrapped
 
 
 class SyscallNotImplemented(OSException):
     """
     Exception raised when you try to call an unimplemented system call.
-    Go to linux.py and add it!
+    Go to linux.py and add an implementation!
     """
 
     def __init__(self, idx, name):

mypy.ini

@@ -37,9 +37,6 @@ ignore_missing_imports = True
 [mypy-prettytable.*]
 ignore_missing_imports = True
 
-[mypy-wrapt.*]
-ignore_missing_imports = True
-
 [mypy-wasm.*]
 ignore_missing_imports = True
 

setup.py

@@ -50,7 +50,6 @@ def rtd_dependent_deps():
     python_requires=">=3.6",
     install_requires=[
         "pyyaml",
-        "wrapt",
         # evm dependencies
         "pysha3",
         "prettytable",

tests/native/test_syscalls.py

@@ -1,16 +1,18 @@
+import logging
 import random
 import struct
 import socket
 import tempfile
+import time
 import unittest
 
 import os
 import errno
+import re
 
-from manticore.core.smtlib import *
 from manticore.platforms import linux, linux_syscall_stubs
 from manticore.platforms.linux import SymbolicSocket
-from manticore.platforms.platform import SyscallNotImplemented
+from manticore.platforms.platform import SyscallNotImplemented, logger as platform_logger
 
 
 class LinuxTest(unittest.TestCase):
@@ -374,27 +376,31 @@ def test_llseek_end_broken(self):
         self.linux.sys_write(fd, 0x1200, len(buf))
 
         # FIXME: currently broken -- raises a Python OSError invalid argument exception!
+        resultp = 0x1900
         res = self.linux.sys_llseek(fd, 0, -2 * len(buf), resultp, os.SEEK_END)
         self.assertTrue(res < 0)
 
-    def test_unimplemented(self):
+    def test_unimplemented_stubs(self) -> None:
         stubs = linux_syscall_stubs.SyscallStubs(default_to_fail=False)
 
-        if hasattr(stubs, "sys_bpf"):
+        with self.assertLogs(platform_logger, logging.WARNING) as cm:
             self.assertRaises(SyscallNotImplemented, stubs.sys_bpf, 0, 0, 0)
-
-            self.linux.stubs.default_to_fail = False
-            self.linux.current.RAX = 321  # SYS_BPF
-            self.assertRaises(SyscallNotImplemented, self.linux.syscall)
-
-            self.linux.stubs.default_to_fail = True
-            self.linux.current.RAX = 321
-            self.linux.syscall()
-            self.assertEqual(0xFFFFFFFFFFFFFFFF, self.linux.current.RAX)
-        else:
-            import warnings
-
-            warnings.warn(
-                "Couldn't find sys_bpf in the stubs file. "
-                + "If you've implemented it, you need to fix test_syscalls:LinuxTest.test_unimplemented"
-            )
+        # make sure that log message contains expected info
+        pat = re.compile(r"Unimplemented system call: .+: .+\(.+\)", re.MULTILINE)
+        self.assertRegex("\n".join(cm.output), pat)
+
+        self.linux.stubs.default_to_fail = False
+        self.linux.current.RAX = 321  # SYS_BPF
+        self.assertRaises(SyscallNotImplemented, self.linux.syscall)
+
+        self.linux.stubs.default_to_fail = True
+        self.linux.current.RAX = 321
+        self.linux.syscall()
+        self.assertEqual(0xFFFFFFFFFFFFFFFF, self.linux.current.RAX)
+
+    def test_unimplemented_linux(self) -> None:
+        with self.assertLogs(platform_logger, logging.WARNING) as cm:
+            self.linux.sys_futex(0, 0, 0, 0, 0, 0)
+        # make sure that log message contains expected info
+        pat = re.compile(r"Unimplemented system call: .+: .+\(.+\)", re.MULTILINE)
+        self.assertRegex("\n".join(cm.output), pat)