The current implementation DLA cannot reason about function pointers,
because function types have zero size.
Given this shortcoming, llvm::Functions are used (as a hack) to
represent their Functions' return types.
This representation is a key factor of how DLA is able to piece
information together interprocedurally.
This hack has consequences.
Assume we're setting up the DLA graph, and we're looking at a use of an
llvm::Function that is inside the body of an llvm::Function and is not
the callee-operand of a Call.
That use is obviously function-pointer typed.
But if we try to create a LayoutTypeSystemNode for the llvm::Value of
the used llvm::Function, the inner machinery of how the DLA graph is
set up will treat is as the return type of the function, which is
obviously wrong.
So we have to prevent that creation at all costs.
Basically creation of LayoutTypeSystemNodes in the DLA graph associated
with llvm::Functions are valid only for representing return types.
This shortcoming does not affect the power of DLA. In its current form,
it doesn't represent function types at all.
The problem is that until we undo this hack we will not be able to
properly support function types in DLA.
This will have to be fixed in the future.
In the meantime, this commit provides an helper function to centralize
how we detect the offending uses of llvm::Functions. In this way, all
the points of the codebase that are affected by this hack are clearly
marked.
* Make the following private headers public:
* Lift/CPUStateAccessAnalysisPass.h
* Lift/CSVOffsets.h
* Lift/PTCDump.h
* Lift/VariableManager.h
* Move from revngSupport to revngLift:
* IRAnnotators.{h,cpp}
* SelfReferencingDbgAnnotationWriter.{h,cpp}
* Move from revngSupport to revngModel:
* FunctionTags.{h,cpp}
* ProgramCounterHandler.{h,cpp}
* Move from revngSupport to revngRecompile:
* OriginalAssemblyAnnotationWriter.{h,cpp}
This commit adds handling for a corner case of
MergePointeesOfPointerUnion, that wasn't handled before.
Said corner case happens when one of the scalars that gets merged as
part of the DLAStep operations is also itself a pointer to another
scalar that gets merged.
This wasn't accounted for before this commit, but it's actually a valid
scenario that can happen on some inputs.
Before this commit DLA was not properly updating the pointee type for
some pointer types.
This commit reworks the creation of the model types so that there's no
need for in-place fixup of the pointee types: pointers are already
created consistently.
Before this commit, the DLAStep DeduplicateFields was using
NotMergedNode (a reference) in an assertion, in a place where it was
already invalidated by a preceding call to
CollapseSingleChild::collapseSingle that can mutate the graph.
This caused the assertion to trigger erroneously in some situations.
This commit moves the assertion earlier, in a place where the reference
is guaranteed to always be valid.
Before this commit, the DLA frontend wasn't initializing the size of
stack types properly, nor tracking the fact that they are not scalar.
These 2 problems were causing stack types to be merged with other types
in the DLA pipeline, possibly even scalar types and in some situations
changing the size of the stack type.
All these consequences were just wrong and need to be avoided because
they break assumptions of the DLA pipeline, leading to crashes in the
DLA backend, when trying update the model to integrate DLA's results.
This commit prevents all the mentioned wrong manipulation of stack
types, simply by correctly assigning an the size of the stack type from
the beginning, and marking it as non scalar.
The old version was unnecessarily convoluted, and had a bug that caused
invalid iterators to be dereferenced in some corner cases.
This commit reworks the DFS and makes it cleaner and easier to follow,
while fixing the iterator dereference bug.
The DLAStep SimplifyInstanceAtOffset0 now only triggers if the parent
node has the child-at-offset-0 node as its only successor, or when the
child-at-offset-0 doesn't have other predecessors.
Doing this guarantees that it's impossible for another predecessor of
the child-at-offset-0 to start seeing memory accesses that were
initially relative to the parent.
This condition is slightly more restrictive than the previous one, but
it takes into consideration some far reaching consequences.
If SimplifyInstanceAtOffset0 aggressively like we did before, DLA can
end up inferring types in some memory locations, like executable
segments, for which there aren't clues in the binary.
This isn't bad per se, but if DLA does that, the newly recovered type is
identified as non-executable data, causing misdecompilation because
rev.ng doesn't decompile memory regions that it understands as non
executable.
This commit, making SimplifyInstanceAtOffset0 less aggressive, makes it
play better with the rest of the assumptions of the decompilation
pipeline.
It also relaxes a decompilation test that was previously working by
chance and that was effectively beyond the current expressive power of
reasoning for DLA at the moment.
Before this commit the breadth-first traversal was more convoluted and
it used a broken criterion for bailing out, resulting sometimes in
considering a pointer edge as if it was an instance edge.
This in turn caused bugs to the point of the DLA Graph failing
verification.
This commit rewrites and simplifies the main traversal and comparisons
in DeduplicateField to make them robust against these bugs.
Before this commit the code was wrong in 2 ways:
- if failed to iterate on children, because it iterated a range that was
effectively [begin, begin) instead of [begin, end)
- the update of the ARootNext iterator in the loop was broken, but it
was impossible to realize that since the loop was never taken
This commit fixes both these intertwined problems.
Before this commit, the DLAStep MergePointeesOfPointerUnion wasn't
really equipped for dealing with LayoutTypeSystemNodes representing
types imported from Model.
There was some code that tried to deal with them but it was mostly an
afterthought and it wasn't robust.
This commit fixes that, and is able to handle a strict superset of the
scenarios envisioned before, while preserving the property that nodes
representing types imported from Model should be preserved (in
particular their size).
Alessandro Di Federico