mirror of
https://github.com/angr/angr
synced 2026-06-08 13:09:39 +00:00
e427875cbd
* fix typos This mostly fixes comments, but in a handful of places fixes bugs due to typos. One example: ``` - insn_op_idx=None if cmsg.operand_idx == -1 else cmsg.opearnd_idx, + insn_op_idx=None if cmsg.operand_idx == -1 else cmsg.operand_idx, ``` * more typo fixes * address lint issues * more import fixes? * address feedback from PR --------- Co-authored-by: Brian Caswell <bcaswell@microsoft.com>
180 lines
7.6 KiB
ReStructuredText
180 lines
7.6 KiB
ReStructuredText
Reporting Bugs
|
|
==============
|
|
|
|
If you've found something that angr isn't able to solve and appears to be a bug,
|
|
please let us know!
|
|
|
|
|
|
#. Create a fork off of angr/binaries and angr/angr
|
|
#. Give us a pull request with angr/binaries, with the binaries in question
|
|
#. Give us a pull request for angr/angr, with testcases that trigger the
|
|
binaries in ``angr/tests/broken_x.py``, ``angr/tests/broken_y.py``, etc
|
|
|
|
Please try to follow the testcase format that we have (so the code is in a
|
|
test_blah function), that way we can very easily merge that and make the scripts
|
|
run.
|
|
|
|
An example is:
|
|
|
|
.. code-block:: python
|
|
|
|
def test_some_broken_feature():
|
|
p = angr.Project("some_binary")
|
|
result = p.analyses.SomethingThatDoesNotWork()
|
|
assert result == "what it should *actually* be if it worked"
|
|
|
|
if __name__ == '__main__':
|
|
test_some_broken_feature()
|
|
|
|
This will *greatly* help us recreate your bug and fix it faster.
|
|
|
|
The ideal situation is that, when the bug is fixed, your testcases passes (i.e.,
|
|
the assert at the end does not raise an AssertionError).
|
|
|
|
Then, we can just fix the bug and rename ``broken_x.py`` to ``test_x.py`` and
|
|
the testcase will run in our internal CI at every push, ensuring that we do not
|
|
break this feature again.
|
|
|
|
Developing angr
|
|
===============
|
|
|
|
These are some guidelines so that we can keep the codebase in good shape!
|
|
|
|
pre-commit
|
|
----------
|
|
|
|
Many angr repos contain pre-commit hooks provided by `pre-commit
|
|
<https://pre-commit.com/>`_. Installing this is as easy as ``pip install
|
|
pre-commit``. After ``git`` cloning an angr repository, if the repo contains a
|
|
``.pre-commit-config.yaml``, run ``pre-commit install``. Future ``git``
|
|
commits will now invoke these hooks automatically.
|
|
|
|
Coding style
|
|
------------
|
|
|
|
We format our code with `black <https://github.com/psf/black>`_ and otherwise
|
|
try to get as close as the `PEP8 code convention
|
|
<http://legacy.python.org/dev/peps/pep-0008/>`_ as is reasonable without being
|
|
dumb. If you use Vim, the `python-mode <https://github.com/klen/python-mode>`_
|
|
plugin does all you need. You can also `manually configure
|
|
<https://wiki.python.org/moin/Vim>`_ vim to adopt this behavior.
|
|
|
|
Most importantly, please consider the following when writing code as part of angr:
|
|
|
|
|
|
* Try to use attribute access (see the ``@property`` decorator) instead of
|
|
getters and setters wherever you can. This isn't Java, and attributes enable
|
|
tab completion in iPython. That being said, be reasonable: attributes should
|
|
be fast. A rule of thumb is that if something could require a constraint
|
|
solve, it should not be an attribute.
|
|
|
|
* Use `our pylintrc from the angr-dev repo
|
|
<https://github.com/angr/angr-dev/blob/master/pylintrc>`_. It's fairly
|
|
permissive, but our CI server will fail your builds if pylint complains under
|
|
those settings.
|
|
|
|
* DO NOT, under ANY circumstances, ``raise Exception`` or ``assert False``.
|
|
**Use the right exception type**. If there isn't a correct exception type,
|
|
subclass the core exception of the module that you're working in (i.e.,
|
|
``AngrError`` in angr, ``SimError`` in SimuVEX, etc) and raise that. We catch,
|
|
and properly handle, the right types of errors in the right places, but
|
|
``AssertionError`` and ``Exception`` are not handled anywhere and
|
|
force-terminate analyses.
|
|
|
|
* Avoid tabs; use space indentation instead. Even though it's wrong, the de
|
|
facto standard is 4 spaces. It is a good idea to adopt this from the
|
|
beginning, as merging code that mixes both tab and space indentation is awful.
|
|
|
|
* Avoid super long lines. It's okay to have longer lines, but keep in mind that
|
|
long lines are harder to read and should be avoided. Let's try to stick to
|
|
**120 characters**.
|
|
|
|
* Avoid extremely long functions, it is often better to break them up into
|
|
smaller functions.
|
|
|
|
* Always use ``_`` instead of ``__`` for private members (so that we can access
|
|
them when debugging). *You* might not think that anyone has a need to call a
|
|
given function, but trust us, you're wrong.
|
|
|
|
* Format your code with ``black``; config is already defined within
|
|
``pyproject.toml``.
|
|
|
|
Documentation
|
|
-------------
|
|
|
|
Document your code. Every *class definition* and *public function definition*
|
|
should have some description of:
|
|
|
|
* What it does.
|
|
* What are the type and the meaning of the parameters.
|
|
* What it returns.
|
|
|
|
Class docstrings will be enforced by our linter. Do *not* under any
|
|
circumstances write a docstring which doesn't provide more information than the
|
|
name of the class. What you should try to write is a description of the
|
|
environment that the class should be used in. If the class should not be
|
|
instantiated by end-users, write a description of where it will be generated and
|
|
how instances can be acquired. If the class should be instantiated by end-users,
|
|
explain what kind of object it represents at its core, what behavior is expected
|
|
of its parameters, and how to safely manage objects of its type.
|
|
|
|
We use `Sphinx <http://www.sphinx-doc.org/en/stable/>`_ to generate the API
|
|
documentation. Sphinx supports docstrings written in `ReStructured Text
|
|
<http://openalea.gforge.inria.fr/doc/openalea/doc/_build/html/source/sphinx/rest_syntax.html#auto-document-your-python-code>`_
|
|
with special `keywords
|
|
<http://www.sphinx-doc.org/en/stable/domains.html#info-field-lists>`_ to
|
|
document function and class parameters, return values, return types, members,
|
|
etc.
|
|
|
|
Here is an example of function documentation. Ideally the parameter descriptions
|
|
should be aligned vertically to make the docstrings as readable as possible.
|
|
|
|
.. code-block:: python
|
|
|
|
def prune(self, filter_func=None, from_stash=None, to_stash=None):
|
|
"""
|
|
Prune unsatisfiable paths from a stash.
|
|
|
|
:param filter_func: Only prune paths that match this filter.
|
|
:param from_stash: Prune paths from this stash. (default: 'active')
|
|
:param to_stash: Put pruned paths in this stash. (default: 'pruned')
|
|
:returns: The resulting PathGroup.
|
|
:rtype: PathGroup
|
|
"""
|
|
|
|
This format has the advantage that the function parameters are clearly
|
|
identified in the generated documentation. However, it can make the
|
|
documentation repetitive, in some cases a textual description can be more
|
|
readable. Pick the format you feel is more appropriate for the functions or
|
|
classes you are documenting.
|
|
|
|
.. code-block:: python
|
|
|
|
def read_bytes(self, addr, n):
|
|
"""
|
|
Read `n` bytes at address `addr` in memory and return an array of bytes.
|
|
"""
|
|
|
|
Unit tests
|
|
----------
|
|
|
|
If you're pushing a new feature and it is not accompanied by a test case it
|
|
**will be broken** in very short order. Please write test cases for your stuff.
|
|
|
|
We have an internal CI server to run tests to check functionality and regression
|
|
on each commit. In order to have our server run your tests, write your tests in
|
|
a format acceptable to `nosetests <https://nose.readthedocs.org/en/latest/>`_ in
|
|
a file matching ``test_*.py`` in the ``tests`` folder of the appropriate
|
|
repository. A test file can contain any number of functions of the form ``def
|
|
test_*():`` or classes of the form ``class Test*(unittest.TestCase):``. Each of
|
|
them will be run as a test, and if they raise any exceptions or assertions, the
|
|
test fails. Do not use the ``nose.tools.assert_*`` functions, as we are
|
|
presently trying to migrate to ``nose2``. Use ``assert`` statements with
|
|
descriptive messages or the ``unittest.TestCase`` assert methods.
|
|
|
|
Look at the existing tests for examples. Many of them use an alternate format
|
|
where the ``test_*`` function is actually a generator that yields tuples of
|
|
functions to call and their arguments, for easy parametrization of tests.
|
|
|
|
Finally, do not add docstrings to your test functions.
|