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
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:
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
test_x.py and the testcase will run in our internal CI at every push, ensuring that we do not break this feature again.
These are some guidelines so that we can keep the codebase in good shape!
We try to get as close as the PEP8 code convention as is reasonable without being dumb. If you use Vim, the python-mode plugin does all you need. You can also manually configure 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
@propertydecorator) 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.
.pylintrcfrom the angr-dev repo. It's fairly permissive, but our CI server will fail your builds if pylint complains under those settings.
DO NOT, under ANY circumstances,
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.,
SimErrorin SimuVEX, etc) and raise that. We catch, and properly handle, the right types of errors in the right places, but
Exceptionare 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.
__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.
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.
Here is an example of function documentation. Ideally the parameter descriptions should be aligned vertically to make the docstrings as readable as possible.
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.
def read_bytes(self, addr, n): """ Read `n` bytes at address `addr` in memory and return an array of bytes. """
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 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_*():. Each of them will be run as a test, and if they raise any exceptions or assertions, the test fails. Use the
nose.tools.assert_* functions for better error messages.
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.