This commit takes out of EnforceABI the part taking care of creating
wrappers for calls to helpers and promoting CSV to local variables.
This decoupling, enables to run -promote-csvs multiple times, for
instance after inlining.
FunctionTags goal is to solve the long-standing problem of identifying
what type of function are we dealing with. Is it a lifted function? An
helper?
Now we have a sane way to determine this using Metadata and a proper
API.
This commit further reduces the time spent in AVI by considering all the
function calls as indirect. In fact, we currently don't handle indirect
jumps whose target is affected by computation happening before a
function call.
This commit greatly improves the performance by ensuring that, when
computing the set of nodes we want to consider for AVI, we do not
traverse the dispatcher.
Doing so, means including *a lot* of irrelevant nodes and wasting a lot
of computation, since the CFG usually is not influenced by stuff
happening before an indirect jump.
In at least a situation the speedup is in the order of 20x, however this
depends on the size of the binary, since traversing the dispatcher means
including all the binary in the computations (as opposed to just the set
of blocks involved in the dataflow to compute a certain expression).
QEMU helpers are compiled with -O0. This led the functions in the QEMU
helper module to have the `optnone` attribute, preventing any
optimization.
This has now been fixed on the QEMU side with the following flags:
clang -Xclang -disable-O0-optnone
Isolated functions return structs. These structs used to be constructed
using `IRBuilder::CreateAggregateRet`, which is implemented using
`insertvalue` instructions. However, this approach led to ugly
decompiled code.
This commit introduces "constructor" functions that can be easily
pattern matched down the pipeline.
This commit ensures that FunctionIsolation and EnforceABI do only
thing. This means that they no longer modify `root`.
Instead, we have a new pass, `invoke-isolated-functions` that needs to
be run after them and replaces the entry point of the functions with
invokes to the isolated functions, possibly with the appropriate
arguments.
The `revng` script looks in several paths for analysis
libraries. However, before this commit, in case multiple versions of the
same library were available, you'd get unpredictable results.
This commit ensures that each library is considered at most once, in the
right order.
When we compare StackAnalysis test results, the order in which things
appear in the JSON is relevant. However, the output was
non-deterministic due to a `std::map` using a pointer as key.
This commit improves the situation by sorting the elements by name
before dumping them in JSON.
With this quarantine, when nodes are removed from RegionCFG, they are
not really freed, but they are held here until the RegionCFG itself goes
out of scope.
This is unfortunately necessary now, since the CFG restructuring
algorithm uses maps and sets (e.g. Backedges.) that are indexed using
a BasicBlockNodeT *.
If we don't hold the removed nodes in quarantine, the system allocator
can reuse the blocks, allocating new nodes at the same address, and
causing false-positive hits in some of the mentioned maps. This was the
most straightforward solution for now.
Other solutions we have considered:
- use a special monotonic allocator for BasicBlockNodes
- this should work, but in principle it gives the same results as the
current solution, with more boilerplate. Also, at the moment
std::unique_ptr is not allocator aware, so we would need to change
BlockNodes to not use them, and this would require even more
boilerplate.
- change the API for RegionCFG::removeNode, to take as arguments the
reference to the data structure and maps that must be updated, so that
when we remove the node from RegionCFG we also clear it from the maps.
However, this is very invasive, it requires changing the public facing
API, it requirese coupling the RegionCFG API with internal details,
and in the future it would need to be updated for every new map that
must be updated on removal of a node.
This commit fixes a bug causing a failing assertion on Backedges that
jump from an inner MetaRegion to an outer MetaRegion after region
collapsing.
Constructor declarations for `BasicBlockNode` were not consistent.
We had default-constructor, and move constructor explicitly deleted, but
the copy constructor was not explicitly deleted, even though it was
implicitly deleted. Make deletion explicit, in accordance to the fact
that all other special member functions for construction and assignment
are deleted.
Implement a beautify phase which does the following:
- Compute, for every scope in the AST, if that scope is `fallthrough`
or `nofallthrough` scope. Basically, the `nofallthrough` scopes are
scope which ends with a `return`, `continue', or `break`.
- Using the information computed before, we can promote the scope of an
`IfNode` using the following criterion: if one of the two branches of
the `IfNode` is a `nofallthrough` scope, we are sure that the other
branch is not reachable from the former one. We can therefore, promote
the latter as `fallthrough` block of the `IfNode` (of course taking care
of inverting the condition statement if we are promoting to
`fallthrough` the `then` branch.
If both the `then` and the `else` branches can be promoted as
`nofallthrough`, we have a function that evaluates the weight of the two
branches, and promotes the heavier one. This helps reducing the
Cognitive Complexity of the generated code
When printing do-while loops in C, we wrongly emitted redundant
statements before the `do`, representing computation necessary for
evaluating the exit condition from the loop.
These statements were duplicated at end of the loop body, and were
entirely redundant before the `do`.
This commit removes them.
The PromoteStackPointerPass now trivially handles running on modules
where the global variable for the stack pointer is missing.
When the global is not found, the pass just does nothing and returns
false.
This is necessary to handle very small functions, typically from tests.
Before this commit, whenever an Instruction needed to be serialized, it
forced to serialize all the pending Instructions.
Now this happens only for Instructions with side effect, that may
interfere with the pending Instructions if they are not serialized as
well.
In all the other cases, serializing an Instruction does not force
serialization of other Instructions.