This bug caused the expression x + (-2) < -3 to be rewritten as
(x > 2) || (x < -1), instead of (x > 2) && (x < -1).
We had tests for a very similar case, but this specific example was
triggered by having the two constant terms in the inequality with a
difference of only one.
This commit rewrites and unifies the logic for handling inequalities,
and adapts the test for checking this specific additional corner case.
Many old unit tests were too rigid, using the model and the LLVM IR.
This commit drops them, and replaces them with decompilation tests based
on revng-qa, relying on `revng model compare` to test model properties,
and on `FileCheck` to test that we emit specific constructs in C.
The new tests cover various features of the decompiler.
* DLA capability to recover complex data structure like linked-lists and
arrays.
* DLA capability to update segment and section types, so that we emit
nice looking accesses to segments in C.
* Capability to emit nice looking integer literals in C
* Capability to emit inline string literals in C and update the model
types of the segments containg such string literals.
This pass is dedicated to applying a bunch of peephole optimizations
that are useful for decompilation and haven't been implemented in LLVM
for whatever reason.
At the moment it only runs an optimization that tries to reduce the uses
of incoming values of PHINodes, under very specific condidtions, which
enables emitting less LocalVariables.
Simplification includes dropping dependencies onto
`FunctionMetadataCachePass` and `LoadModelWrapperPass`, as well
as removing "StructName" logic outright.
This commit drops ValueManipulationAnalysis, which in its original
design based on MinCut and Karger, was never enabled in the
decompilation pipeline.
Until now, VMA was only used in a severely weakened form in
initModelTypes. That for was so weakened that it barely did anything.
We already have a new design for VMA so that it can work before
DataLayoutAnalysis, and on Clift.
At this point, the old VMA is basically useless anyway, and the very few
occasions where it can do something will simply be solved by the
upcoming work on making some of the remaining casts implicit.
At this point it does not make sense to keep VMA alive anymore.
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.
This commit sets a name for types we use in OpaqueExtractValues.
These are StructTypes that can only be returned from isolated functions
with RawFunctionType prototype on the model, or from helpers that do not
represent isolated functions.
The name is required because when we will converto to MLIR LLVM Dialect,
there are checks in place that forbid unnamed types.
The code re-uses code that is used to print C code, in order to make
sure that the struct names we use for this are the same as those we emit
in C, to ease debugging.
This commit also cleans up the unittest to make it more concise, while
still testing the same things.
* Fix non-PrimitiveTypes with low IDs.
* Ensure we use `revng model compare` to test the model.
* Updates the name to reflect the recent changes in naming convention
and in the way we represent `model::Type::ID`s.
Before this commit, the TwosComplementArithmeticNormalizationPass wasn't
taking into consideration properly all the cases where the arithmetic
can wrap around in case of expression such as (x + const1 <=> const2) or
(x - const1 <=> const 2).
This caused 2 different classes of problems that affected semantics.
1) In some cases we were emitting comparisons that were too broad,
meaning that they were true in a larger set of cases than the
original one.
2) In other cases we were emitting tautological comparisons, i.e.
comparisons that were always demonstrably true or false at compile
time.
This commit fixes the problem, treating all the cases in a generalized
unique way that works correctly for every case.
Implements transforms when the results of sub or add with a constant
operand are compared against constant:
1) x + a == b --> x == b - a
2) x + a != b --> x != b - a
3) x + a <= b --> x <= b - a
4) x + a < b --> x < b - a
5) x + a >= b --> x >= b - a
6) x + a > b --> x > b - a
After the transformation, the right hand side is also enqued for
analysis to detect opporunities to use unary minus if a-b is a negative
constant.
This commit changes MarkAssignments (and related tests) so that now
instruction with more than one use are not assigned to a new variable
by default.
In some cases it might still be good to serialize them for readability,
but for those cases we will implementa a standalone pass that only
implements heuristics based on readability.
MarkAssignments now only takes decisions based on semantics of
side-effects.
This commit teaches getExpectedModelType about the fact that various
bitwise operations are only allowed to have integer operands.
It also updates VMA, which uses getExpectedModelType, to take this
into account.
This commit does various things oriented at reducing the number of local
variables emitted in C:
- MarkAssignments now know that @Copy and @Assign involving
@LocalVariable only have side effects that affect the local variable
itself; this enables to reduce the number of times we're forced to
emit a local variable due to interfering side effects
- Drop the @AssignmentMarker FunctionTag; AddAssignmentMarkerPass now
doesn't emit @AssignmentMarker anymore; instead it emits groups of
@LocalVariable, @Copy, and @Assign, which benefit from the previous
point
- Drop 2 MarkAssignments::Reasons: HasManyUses and HasUsesOutsideOfBB;
both these have now been aggregated into the AlwaysAssign reason for
simplicity, representing all reasons non involving side effects
- Update BeautifyGHAST and how it reasons about side effects when
beautifying; before this commit it used @AssignmentMarker, now it
looks at @Assign
- Simplify ExitSSA; before this commit it was trying hard to be smart on
where it emitted the store instructions representing the incoming
values of the PHI that was being destroyed; this seemed smart when we
originally did it but it generated C code that was not really better
to read, so this useless complexity is finally gone
This commit adds a new pass, PrettyIntFormatting, that injects calls to
decorator functions print_hex, print_char, and print_bool around
llvm::ConstantInt in various situations.
It also updates the rest of passes of the decompilation pipelin to
understand these new decorator functions and to properly emit decorated
integer literals in the decompiled C code.
Pietro Fezzardi