This commit introduces the PruneRetSuccessors pass, whose role is to
identify all the indirect jumps whose devirtualized destinations
correspond to return addresses. In fact, they are most likely to be
return instructions and devirtualizing them is always detrimental.
After combining two AS objects, removing an ASOContent in the cleanup
phase may lead to a mismatch in the CSVs promoted to function argument,
with some registers actually being used. In most cases, this results in
promoting only one or few alive registers as function argument.
We used to accidentaly call `resetCacheMustHit` on the just popped
element of the set, resulting in an out-of-bounds access within the
container.
This bug has been spotted thanks to AddressSanitizer.
Add an additional opaque store the the `pc` after each function call in
the enforced functions. In this way, when the `pc` will be promoted to
an alloca, we won't have an inconsistent state of the IR (the callee
cannot propagate the correct value of pc, the safety check introduced by
the function isolation will always be false, and llvm will place a
`llvm.assume` which will in turn make the rest of the code dead and
purged by an additional `-simplifycfg`).
When the `Successor` field is consumed to create the `Sequence` nodes,
we blank the field so that we do not have redundant information in our
graph representation.
Change the AST debug printing from a recursive descent on the AST nodes
in the graph, to an iterative printing of respectively nodes and their
outgoing edges.
Also, we now print the edge going to the `Successor` field, if present.
Reorganize the AST dot printing in order to use LLVM facilities for
files handling. In addition, clean up the code, and add a function
(useful in debug) in order to dump the dot of and AST from GDB (accepts
a `char *` instead of a `std::string`.
Now, when the `debug-log=restructure` flag is active, debug graphs are
organized in a more rational way.
First of all, all the graphs are now put in a single directory, and are
divided first by function name and then by type.
Also, as in the case of the tiling debug graphs, their name is more
consistent with the rest of the codebase.
This commit fixes two interacting problems in the beautification of if
statements.
First, in `simplifyShortCircuit`, there were two assertion on null
pointers that were too strict. These assertions didn't take into
consideration that one among the "then" and the "else" branch of an
`IfNode` could be simplified away _during_ the beautification itself.
Second, a previous commit had moved the execution `flipEmptyThen` after
the two beautification of short-circuit, leading to assertions failed
in `simplifyTrivialShortCircuit`.
This was caused by the fact that `simplifyTrivialShortCircuit` only
never expected `IfNode`s with empty "then".
This was fixed by running `flipEmptyThen` also before
`simplifyTrivialShortCircuit`.
This commit is a hack, that removes an assertion on NDuplicates.
For now we can remove this check, because we declare all the local
variables at the beginning of the function body.
Whenever we start emitting the declarations of local variables
as-late-as-possible, we'll need to take care of this.
Before this commit, we assigned names to struct types without looking at
the function names of which they were return types.
Now we do, so that the name of the generated type for the function `f`
is `f_ret_type`.
This also fixes errors when reparsing the generated C code, where the
names of the struct types did not match.
We now craft a new decompilation pipeline. We remove the exceptions
introduced as fallbacks to preserve the semantics, and we perform passes
of `dce` and `simplifycfg` to remove all the superfluos basic block
remaining after this change. To do this, we need an additional pass that
removes also the calls to the `llvm.assume` intrinsic, and another pass
of `dce` to remove all the dead uses.
In addition, we also remove dead stores to the `cpu_loop_exiting` global
variable in order to improve the decompiled code.
As a byproduct of this, we remove also the dependency from the
`-remove-pc-stores` pass, since its jobs is included by the changes
mentioned above.
This commit fixes an issue that caused to emit duplicated statements
for BasicBlocks that terminated with a conditional branch.
This was caused by a redundant call to `buildStmtsForBasicBlocks()`
inside `createCondExpr`, that has now been removed.
With this commit `ExtractValueInst`s are handled so that they don't have
side effects.
This significantly improves the quality of the decompiled output,
preventing the forced serialization of all the extraction of struct
fields after calls to function that return structs, except for when it's
really needed.
With this commit, the declarations of local variables and function
parameters now use the name of the associated `llvm::Value` if present,
instead of always using the prefix "param_" or "var_".
This commit drops the `--isolate-no-safety-checks` flags since we
adopted a new approach later on in the decompilation pipeline, i.e.,
dropping calls to `raise_exception_helper` and running dead code
elimination.
FunctionIsolation used to base its work on calls to the marker function
`function_call`, as opposed to information provided by the StackAnalysis
(i.e., `revng.member.type` along with `func.call`).
This also affected EnforceABI, which took care of finishing the work
left over by FunctionIsolation. This was hackish and inelegant.
This commit makes FunctionIsolation work exclusively employing
information from StackAnalysis and purges away code that is no longer
necessary from EnforceABI.
We used to detect callee-saved registers only by checking if their final
value was identical to the initial one. However, the need for a more
precise heuristic emerged: we now also check if at least on of the stack
slots contains that same value.
This commit fixes a subtle bug that was preventing us from correctly
considering the effects of function calls during ABI analysis.
Specifically, when merging information from the call site to the caller,
in case the caller did not provide any information about a certain
slots, we used the `DefaultMap::Default` field, which is the correct
thing to do, except for the fact that, in two methods, the `Default`
field was being updated to early.
This basically had the effect of not considering arguments of function
calls as used, e.g., in URAOF.
Alessandro Di Federico