Before this commit, the CompactCompatibleArray DLAStep had that caused
the compaction to be sensible to the order of the pairs of edges that
were compacted, and that also caused the compaction operation to overrun
the end of the containing struct in some corner cases.
This commit fixes both bugs.
Now the compaction routine is not ordering-sensitive anymore.
For each pair of edges <A, B> to compact it tries to compact them in 2
possible ways: by aligning A to B shifting A to lower offsets, and by
aligning B to A shifting B to lower offsets.
If both succeed, it picks the best result among the two, which doesn't
depend on the ordering of A and B.
This fixes the order-sensitiveness.
While reworking this logic, the logic was fixed so that if any of the
two compaction attempts causes to overrun the end of the containing
struct, the attempt is not considered successful anymore, and it's
discarded altogether.
One of the two passes to purge unused types from the model had a
ill-defined semantics.
This commit simplifies them in two passes: one that purges all the types
unreachable from outside `Binary::Types` and one that does the same but
also preserves types with a `CustomName` or an `OriginalName`.
We introduce the `InlineDispatcherSwitch` beautify pass. Its goal is to
try and inline the body of some of the `case`s of a exit dispatcher, in
place of the `SetNode` corresponding to that `case`, if this doesn't
introduce duplication in the code (i.e., a single `SetNode` for that
specific case value is present).
Additionally, if the inlining procedure is able to completely remove the
necessity of an exit dispatcher altogether, the pass removes it.
The pass is able to handle chains of weaved dispatcher `switch`es
referring to the same original dispatcher `switch`, by handling the
inline operation and the possible simplification level-wise.
The inlining procedure, cannot take place if a `SetNode` is contained in
the body of the case we are trying to inline, since this can possibly
break the semantics of the state variable of a loop, by placing a
`SetNode` in a more internal loop.
We introduce the PromoteCallNoReturn beautification pass. Its goal is to
restructure sequence of statements, in order to have `call`s to
`noreturn` functions as _inlined_ in the middle of the statement
sequence, and leave _non local control flow statements_ at the end of
that scope. E.g., we prefer:
```
if (cond)
call noreturnfunc();
return;
```
to
```
if (!cond)
return;
call noreturnfunc();
```
In order to do this, contextually, we restructure the routine computing
the `fallthrough` property, in order to be able to differentiate between
the _non local control flow statements_, a call to a `noreturn`
function, or a generic mix of the two (useful when combining results
from the two situations above).
The new analysis is also used in the `promoteNoFallThrough` promotion
pass.
Introduce the `DispatcherKind` attribute as a field in the
`BasicBlockNode` and `ASTNode` classes, in order to be able to
distinguish entry and exit dispatchers, and the related set nodes.
This commits reorders the beautifiers so that do-while loops are matched
before while loops.
This has the effect that loops that can be matched both as while and
do-while loops end up matched as do-while.
The consequence is that we generate C code with a layout that is much
more similar to assembly, and prevents duplicating code to recompute the
condition of the while.
This commit drops `needsTopScopeDeclaration`.
Now all the LocalVariables are declared at the top of the function.
In DecompileFunction.cpp, the emission of LocalVariables if handled at
the scope of GHAST `ASTNode`s instead of being LLVM BasicBlock-based.
This enables in the future to design and implement an analysis that for
each LocalVariable decides the C scope (represented by an `ASTNode`)
where it's declaration should be emitted to always be visible in all its
uses.
The inter-procedural part of the DLA frontend connects actual arguments
of function calls with formal arguments of the callee functions, with
instance links at offset 0.
This represents the information that the type pointed-to by the actual
argument has an instance of the type pointed-to by the formal argument
at offset 0.
Before this commit, this was done even when the actual argument was an
integer constant leading to various problems:
1. Most of the times, small integer constants passed as actual
cannot represent valid pointers, leading to graphs bigger than
necessary.
2. Even when an integer constant might actually represent a pointer into
valid memory, if it does it should fall into some segment.
If it does, by the time DLA runs, those integer constants should have
already converted into calls to the special SegmentRef opcode, which
is already handled properly.
3. Finally, those constants end up being very connected in the DLA
graph, creating connections between other nodes that are otherwise
unrelated or very far from each other. This pollutes the graph and
rapidly degrades the quality of the results.
This commit properly guards the code so that the instance-at-offset-0
link between actual and formal arguments is never injected if the actual
argument is an integer constant.
This commit makes some of the dependencies among types in
HeadersGeneration stricter than they were before.
The patch is a temporary workaround to the problem of pointers to arrays
of struct/union in C.
Basically, in C, `struct X (*ptr_to_array)[2];` declares a variable
`ptr_to_array` that points to an array with two elements of type `struct
X`. The problem is that, because of a quirk of paragraph 6.7.6.2 of the
C11 standard (Array declarators), to declare `ptr_to_array` it is required
to see the copmlete definition of `struct X`.
Even if MSVC seems to compile it just fine, clang and gcc don't.
In principle this could be worked around by doing the following two
things:
1) introducing wrapper structs around arrays of struct/union that are used
as pointees
2) postpone the complete definition of the wrapper to after the element
type of the array is complete.
However for now we just inject a stronger dependency to enforce ordering.
This is actually stricter than necessary and can yield to be unable to
print valid C code for model that was otherwise perfectly valid and could
have been fixed by injecting the wrapper structs properly.
This particular handling of pointers to array is more strict than actually
necessary. It has been implemented as a workaround, instead of handling
the emission of wrapper structs. This latter solution of emitting structs
has already been used in other places but, in all the other places where we
currently do it, it is possible to do it on-the-fly, locally.
On the other hand, for dealing with this case properly we'd have to keep
track of dependencies between the forward declaration of the wrapper, and
the full definition of the element type of the wrapped array.
The emission of the full definition of the wrapper must be postponed until
the element type of the wrapped type is fully defined, otherwise it would
fail compilation. So fow now we've put this forced dependency, that could
be relaxed if we properly handle the array wrappers.
This fixes a crash due to the fact that pointers on the model may have
different size than pointer on LLVM IR, due to revng always using
64-bits DataLayout, even when decompiling 32-bits architecture.
The assertion did not take this into account.
In principle the assertion could be removed, but it might become a
source of broken semantics, so it's better to keep an advanced verions
of the assertion in place, to make sure we only allow situations that we
fully understand, and that we can guarantee don't break semantics.
This Tag is used to tag all the funcitons that we use to decorate
integer literals to decide how to print them.
Using a single Tag shared among all the decorators enables more concise
code to handle it.
A bug in revng allowed to use any TaggedFunctionKinds in place of any
other one. That bug has been fixed and the wrong usages in revng-c have
been updated.
Pietro Fezzardi