Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tweaks the method resolution to keep super methods #1370

Merged
merged 3 commits into from
Nov 19, 2021

Conversation

ivg
Copy link
Member

@ivg ivg commented Nov 19, 2021

This change affects both dynamic and static interpreters, so read carefully. Before this commit we used basically the same resolution procedure for all Primus Lisp definitions, with the only exception, in the end, we allow more than one method definition for the same name.

The caveat is that when more than one method of the same class is applicable, the most specific methods were chosen and the least specific, i.e., the super methods, were removed from the resolution. On one hand, this enables refinement of a method, on the other we don't have any notation to call the super method, so the refinement is more like a redefinition. This approach is suitable with normal function definitions that are required to be unique (and when we need refinement we can always factor out the common part from the parent definition and reuse it in the more specific one). But methods are used for different purposes - they process signals from the knowledge base or from the Primus Interpreter and when we add a more specific reaction to the signal we still want to keep other reactions (and the knowledge base will actually take care of the refinement by calling Value.merge on the method results).

To highlight the problem here is an example from the real world (that triggers this change). We have two definitions of the bap:pattern-actions method, one that is generic and handles 'funcstart and 'possiblefuncstart actions, and another that is specific to arm and handles arm/thumb interworking via setcontext. Right now they are all defined in the same file, and arm-specific method is triggered even for non-arm targets. Moreover, when arm is not enabled (i.e., not installed or specifically disabled with --no-arm), the method fails on the typechecker (and will fail as well in runtime) because the bap:arm-set-encoding primitive is not available. The immediate solution is to properly constrain the context of applicability of this method definition to (context (target arm)), which leads to disastrous results. Now the resolver thinks that the arm-specific method that handles contexts, is the overload of the more general method (that handles function starts), so it expels the parent method from the list and never calls it. Not what we wanted! In fact, we want all applicable methods to be called no matter their specificity. In other words, when a method is called, we want to call all methods, starting from the parents and ending with the children.

This is a little bit breaking change as if there exists the Primus Lisp code that was relying on the overriding behavior (none that I am aware of) it will no longer work as expected. The solution is to rely on the overloading of normal functions, e.g.,

(defmethod do-stuff (x)
  (declare (context worker))
  parent-code)

(defmethod do-stuff (x)
  (declare (context worker child))
  child-code)

should be rewritten as,

(defun do-stuff (x)
  (declare (context worker))
  parent-code)

(defun do-stuff (x)
  (declare (context worker child))
  child-code)

(defmethod do-stuff (x)
  (do-stuff x))

ivg added 2 commits November 19, 2021 12:15
This change affects both dynamic and static interpreters, so read
carefully. Before this commit we used basically the same resolution procedure
for all Primus Lisp definitions, with the only exception, in the end,
we allow more than one method definition for the same name.

The caveat is that when more than one method of the same class are
applicable, the most specific methods were chose and the least
specific, i.e., the super methods, were removed from the
resolution. On one hand, this enables refinement of a method, on the
other we don't have any notation to call the super method, so the
refinement is more like a defininition. This aproach is suitable with
normal function defintions that are required to be unique (and when we
need refinement we can always factor out the common part from the
parent definition and reuse it in the more specific one). But methods
are used for different purposes - they process signals from the
knowledge base or from the Primus Interpreter and when we add a more
specific reaction to the signal we still want to keep other
reactions (and the knowledge base will actually take care of the
refinement by calling Value.merge on the method results).

To highlight the problem here is example from the real world (that
triggers this change). We have two definitions of the
`bap:pattern-actions` method, one that is generic and handles
`'funcstart` and `'possiblefuncstart` method, and another that is
specific to arm and handles arm/thumb interworking via
setcontext. Right now they all defined in the same file, and
arm-specific method is triggered even for non-arm targets. Moreover,
when arm is not enabled (i.e., not installed or specifically disabled
with --no-arm), the method fails on the typechecker (and will fail as
well in runtime) because the `bap:arm-set-encoding` primitive is not
available. The immediate solution is to properly constrain the context
of applicapibility of this method definition to
`(context (target arm))`, which leads to the disastrous results. Now
the resolver thinks that the arm-specific method that handles
contexts, is the overload of the more general method (that handles
function starts), so it expels the parent method from the list and
never calls it. Not what we wanted! In fact, we want all applicable
methods to be called no matter their specificity. In other words, when
a method is called, we want to call all methods, starting from the
parents and ending with the children.

This is a little bit breaking change, as if there exists the Primus
Lisp code that was relying on the overriding behavior (none that I am
aware of) it will no longer work as expected. The solution is to rely
on the overloading of normal functions, e.g.,

```
(defmethod do-stuff (x)
  (declare (context worker))
  parent-code)

(defmethod do-stuff (x)
  (declare (context worker child))
  child-code)
```

should be rewritten as,

```
(defun do-stuff (x)
  (declare (context worker))
  parent-code)

(defun do-stuff (x)
  (declare (context worker child))
  child-code)

(defmethod do-stuff (x)
  (do-stuff x))
```
Moves the arm-specific patterns-action that controls arm/thumb
selection to the arm plugin and narrows down its context so that it
will be triggered only when the target is arm.
ivg added a commit to BinaryAnalysisPlatform/bap-testsuite that referenced this pull request Nov 19, 2021
It was never called and was originally wrought for debugging this file
under the normal executor, but the it was really breaking the symbolic
executor, except that before BinaryAnalysisPlatform/bap#1370 it was
never called (yet another reason to merge it, as it looks like from
the way how we use methods we were assuming the behavior that bap#1370
just introduces).
The failure is rather funny, see the linked commit.
@ivg ivg merged commit 63c6950 into BinaryAnalysisPlatform:master Nov 19, 2021
@ivg ivg deleted the tweaks-method-resolution branch December 1, 2021 19:11
ivg added a commit to ivg/opam-repository that referenced this pull request Dec 8, 2021
This release brings This release brings Ghidra as the new disassembler
and lifting backend, significantly improves our Thumb
lifter (especially with respect to interworking), adds
forward-chainging rules and context variables to the knowledge base,
support for LLVM 12, a pass that flattens IR, and a new framework for
pattern matching on bytes that leverages the available patterns and
actions from the Ghidra project.

It also contains many bug fixes and improvements, most notable
performance improvements that make bap from 30 to 50 per cent
faster. See below for the full list of changes.

Package-wise, we split bap into three parts: `bap-core`, `bap`, and
`bap-extra`. The `bap-core` metapackage contains the minimal set of
core packages that is necessary to disassemble the binary, the `bap`
package extends this set with various analysis, finally, `bap-extra`
includes rarely used or hard to install packages, such as the symbolic
executor, which is very heavy on installation, and `bap-ghidra`, which
is right now in a very experimental stage and is only installable on
Ubuntu 18.04, since it requires the libghidra-dev package available
from ppa,

```
sudo add-apt-repository ppa:ivg/ghidra -y
sudo apt-get install libghidra-dev -y
sudo apt-get install libghidra-data -y
```

Changelog
=========

Features
--------

- BinaryAnalysisPlatform/bap#1325 adds armeb abi
- BinaryAnalysisPlatform/bap#1326 adds experimental Ghidra disassembler and lifting backend
- BinaryAnalysisPlatform/bap#1332 adds the flatten pass
- BinaryAnalysisPlatform/bap#1341 adds context variables to the knowledge base
- BinaryAnalysisPlatform/bap#1343 adds register aliases to the Core Theory
- BinaryAnalysisPlatform/bap#1358 adds LLVM 12 support
- BinaryAnalysisPlatform/bap#1360 extends the knowledge monad interface
- BinaryAnalysisPlatform/bap#1363 adds forward-chaining rules and Primus Lisp methods
- BinaryAnalysisPlatform/bap#1364 adds a generic byte pattern matcher based on Ghidra
- BinaryAnalysisPlatform/bap#1365 adds support for the Thumb IT blocks
- BinaryAnalysisPlatform/bap#1369 adds some missing `t2LDR.-i12` instructions to the Thumb lifter

Improvements
------------

- BinaryAnalysisPlatform/bap#1336 improves the `main` function discovery heuristics
- BinaryAnalysisPlatform/bap#1337 adds more Primus Lisp stubs and fixes some existing
- BinaryAnalysisPlatform/bap#1342 uses context variables to store the current theory
- BinaryAnalysisPlatform/bap#1344 uses the context variables to store the Primus Lisp state
- BinaryAnalysisPlatform/bap#1355 tweaks symbolization and function start identification facilities
- BinaryAnalysisPlatform/bap#1353 improves arm-family support
- BinaryAnalysisPlatform/bap#1356 stops proposing aliases as potential subroutine names
- BinaryAnalysisPlatform/bap#1361 rewrites knowledge and primus monads
- BinaryAnalysisPlatform/bap#1370 tweaks Primus Lisp' method resolution to keep super methods
- BinaryAnalysisPlatform/bap#1375 error handling and performance tweaks
- BinaryAnalysisPlatform/bap#1378 improves reification of calls in the IR theory (part I)
- BinaryAnalysisPlatform/bap#1379 improves semantics of some ITT instructions
- BinaryAnalysisPlatform/bap#1380 Fixes handling of fallthroughs in IR theory

Bug Fixes
---------

- BinaryAnalysisPlatform/bap#1328 fixes C.ABI.Args `popn` and `align_even` operators
- BinaryAnalysisPlatform/bap#1329 fixes frame layout calculation in the Primus loader
- BinaryAnalysisPlatform/bap#1330 fixes the address size computation in the llvm backend
- BinaryAnalysisPlatform/bap#1333 fixes and improves label handling in the IR theor
- BinaryAnalysisPlatform/bap#1338 fixes core:eff theory
- BinaryAnalysisPlatform/bap#1340 fixes the Node.update for graphs with unlabeled nodes
- BinaryAnalysisPlatform/bap#1347 fixes a knowledge base race condition in the run plugin
- BinaryAnalysisPlatform/bap#1348 fixes endianness in the raw loader
- BinaryAnalysisPlatform/bap#1349 short-circuits evaluation of terms in Bap_main.init
- BinaryAnalysisPlatform/bap#1350 fixes variable rewriter and some Primus Lisp symbolic functions
- BinaryAnalysisPlatform/bap#1351 fixes and improves aarch64 lifter
- BinaryAnalysisPlatform/bap#1352 fixes several Primus Lisp stubs
- BinaryAnalysisPlatform/bap#1357 fixes some T32 instructions that are accessing to PC
- BinaryAnalysisPlatform/bap#1359 fixes handling of let-bound variables in flatten pass
- BinaryAnalysisPlatform/bap#1366 fixes a bug in the `cmp` semantics
- BinaryAnalysisPlatform/bap#1374 fixes handling modified immediate constants in ARM T32 encoding
- BinaryAnalysisPlatform/bap#1376 fixes fresh variable generation
- BinaryAnalysisPlatform/bap#1377 fixes the IR theory implementation

Tooling
-------

- BinaryAnalysisPlatform/bap#1319 fixes the shared folder in deb packages
- BinaryAnalysisPlatform/bap#1320 removes sudo from postinst and postrm actions in the deb packages
- BinaryAnalysisPlatform/bap#1321 enables push flag in the publish-docker-image action
- BinaryAnalysisPlatform/bap#1323 fixes the ppx_bap version in the dev-repo opam file
- BinaryAnalysisPlatform/bap#1331 fixes the docker publisher, also enables manual triggering
- BinaryAnalysisPlatform/bap#1327 fixes a typo in the ubuntu dockerfiles
- BinaryAnalysisPlatform/bap#1345 fixes bapdoc
- BinaryAnalysisPlatform/bap#1346 nightly tests are failing due to a bug upstream
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant