DW_AT_abstract_origin chains are walked recursively to find a DIE's
name, with no cycle check. Some binaries (e.g. debian-bookworm-mipsel
/usr/sbin/pwck) ship DWARF with a cycle in that chain, so getName ended
up in infinite recursion.
Make the walk iterative.
Make cannotBePointer recurse through an Add when both of its
operands cannot be pointers. This lets canDisambiguatePointerOperand
identify the only viable base pointer of an outer Add even when the
offset side is itself an Add (or a deeper tree of Adds) whose leaves
are all clearly-non-pointer values, e.g.
%offset = add i64 (%a*%b), (%c*%d)
%result = add i64 %arg, %offset
Two unit tests cover the simple two-mul case and a deeper
four-mul binary-tree variant.
The ModuleSlotTracker is owned by a new function-level
ModuleSlotTrackerAnalysis whose Result holds the MST through a
unique_ptr (MST is neither copyable nor movable, but the FAM
cache requires a movable Result). Its run method calls
incorporateFunction on the function.
AvailableExpressionsAnalysis and InstructionToSerializePicker
fetch the MST via FAM.getResult<ModuleSlotTrackerAnalysis>(F) and
forward it down to AvailableExpressionsResult and the monotone
framework, so all dumpToString calls on the logging path share
the same slot-numbering cache.
Drop the workaround that skipped recursion into a Use whose User
was a CallInst with that Use as an arg operand. The original
motivation no longer holds with the rest of the recent changes,
and the workaround was suppressing valid serialisation
opportunities at call sites.
Remove the workaround that suppressed the redundant
self-assignment that the pass now occasionally introduces in
non-legacy mode.
The remaining self-assignments are not bugs but sub-optimal C
generation, so we just disable the -Wself-assign warning in the
tests.
The sub-optimal code generation comes from passes that emit local
variables (allocas) before conversion to Clift, e.g. STS and
ExitSSA (with more planned). Each of these passes generates local
variables independently and they don't know about each other; the
order between them might also be swapped in the future. The only
clean fix is to unify them into a single transformation pass that
takes the results of multiple analyses (in the LLVM sense) and
emits local variables once. We have plans to do that, and when
it's done the warning can be re-enabled.
Hoist the "use already picked in a previous iteration" early-out
to the very start of the per-use loop in
pickInstructionForMemoryRead, so that the rest of the body never
runs for uses that are going to be replaced anyway. The check
itself is unchanged.
Introduce a new canonicalisation pass that forces a small set of
exponential-dataflow-prone instructions (SelectInst plus the FShl
and FShr intrinsic patterns) to be stored into a dedicated local
variable. Without this, expanding their value into C expressions
during decompilation can blow up into exponential path enumeration
on real binaries.
This is a temporary workaround. A more principled split of
pathological dataflows is planned and will replace this pass.
If an instruction picked for serialisation has zero uses, skip
emitting an alloca/copy pair for it. The Clifter recognises a
zero-use instruction as a statement and turns it into an
ExpressionStatement in place, preserving the ordering as if the
local variable had been emitted, but without the cost of a real
local variable. The legacy mode keeps the previous behaviour.
When a use of `I` is going to be replaced by a copy from
SelectedAssign and `User` does not write memory, there is no need
to recur on uses of `User`: those uses will be reading from
SelectedAssign's location, so the original memory read no longer
flows through them.
This avoids a class of redundant explorations and the spurious
serialisations they used to cause.
The "skip over the Assign operand" workaround is only needed in
legacy mode, where assignments are calls to opaque functions that
the rest of the pass cannot reason about. Gate it on `IsLegacy`
so that the non-legacy code path treats the pointer operand of a
StoreInst as a regular use, which is semantic-preserving and
unblocks downstream simplifications.
The macro is no longer necessary: the code now ensures we never
need to assign anything to an array, which would be ill-advised
nevertheless given that array-typed rvalues in C immediately
decay to pointers.
The macro is no longer necessary: the code now ensures we never
need to bitcast anything to an array, which would be ill-advised
nevertheless given that array-typed rvalues in C immediately
decay to pointers.
When importing an alloca whose array size is not a constant 1,
import the alloca's element type as an opaque-array struct
instead of a real C array, matching the treatment that other
array-typed values already get. This keeps the rest of the
emitted code free of array rvalues that would otherwise decay to
pointers.
Drop the previous workaround that always reported a struct byte
size of 0 (with a TODO referencing the missing DataLayout) and use
DataLayout::getStructLayout to get the real struct size. The
DataLayout is now available via the field added in a previous
commit.
Use model-sized pointers on function and call boundaries. These
are necessary for type checking, because llvm::DataLayout at the
moment is not guaranteed to have the same pointer size as the
model, due to quirks of how the DataLayout is initialised. The
function/call boundary is exactly where the model pointer size
becomes observable, so that's where the explicit cast is emitted.
Now that we need the llvm::DataLayout for e.g. computing pointer
sizes, caching pointer types becomes risky: different DataLayouts
coming from different LLVM Modules could disagree on the pointer
size, and securing against this would require clearing the caches
at every call to import.
Given that the caches in practice were not caching operations that
are supposed to be slow, just drop all the type caches to simplify
the codebase.
Replace the standalone PointerSize field with a pointer to the
function's llvm::DataLayout, and read the pointer size off the
DataLayout when needed. This keeps the importer aligned with what
LLVM thinks the pointer size is, instead of relying on an external
source of truth that may go stale.
Use llvm::DataLayout to compute helper struct layouts in the
Clifter, mirroring what HelpersToHeader does. The layout query
replaces a couple of hand-rolled offset/size computations and
keeps the importer in sync with what LLVM and the helpers header
believe about each helper struct.
When emitting the header for QEMU/revng helper struct types, use
the LLVM DataLayout to walk the StructType, so that field offsets
and the overall struct size match what LLVM actually produces. The
old code printed fields in declaration order without consulting
the layout, which broke whenever the struct included padding.
Teach CliftToCEmitter how to emit assignments and bitcasts whose
target type is an array, by emitting calls to the new
assign_array and bit_cast_to_array macros. The shape of the call
encodes the element type and the number of elements, which is what
the macros need to expand correctly.
assign_array is necessary as a fallback for when the decompiled C
code may need assignments among array types. bit_cast_to_array is
necessary because when the target type of a bitcast is an array
the regular bit_cast macro is not enough.
Rework primitive-types.h so that the various bit_cast macros share
a single implementation based on `typeof`/`__typeof__`. As a
consequence both keywords are added to the model's reserved-name
set so that user identifiers cannot collide with them.
When importing an llvm::ArrayType, emit a Clift StructType with the
OpaqueType rank in its handle and a single i8 array field of the
right byte size, instead of a real array type. This avoids
exposing C array types as rvalues in the generated code, which is
a foot-gun because array rvalues immediately decay to pointers,
and provides a single name (via the existing OpaqueType infra) for
arrays of equal byte size.
Rework the GEP-importing logic to handle the i8-typed,
byte-strided GEPs produced by ArithmeticToGEP (and natively used
elsewhere) in addition to the structured ones it already supported.
Indices are now interpreted as byte offsets and the importer walks
the model type tree at byte granularity to materialise the right
sequence of field/element accesses.
Drop the !revng.variable_type metadata and the matching helpers,
along with the only place in the Clifter that still consumed them.
After the previous LVB cleanup nothing emits this metadata, and the
model variable type can be reconstructed from the alloca's array
type alone.
Emit forward declarations and definitions of the opaque-array
artificial structs from the C model header (and from the helpers
header). Definitions are gated on a new DefineOpaqueTypes flag
threaded through PTMLHeaderBuilder::printModelHeader so that the
helpers header keeps emitting only the declarations.
Teach Clift's verify-against-model and ImportDescriptiveInfo
visitors about types whose handle has the OpaqueType rank: the
verifier rejects non-struct types with such handles, and the name
importer fills in the type's mutable name with
NameBuilder::opaqueTypeName based on the wrapped byte size.
Teach NameBuilder and ModelCBuilder how to produce names and
references for opaque-array types: NameBuilder gains
opaqueTypeName(ByteSize), ModelCBuilder gains the matching
reference and definition tags. Both rely on the new
Configuration::OpaqueTypePrefix.
Add a canonicalisation pass that rewrites extractvalue from a
pointer-returning struct as an i8 GEP plus a load. Together with
ArithmeticToGEP this lets later passes consume the result through
the same GEP-based interface they use for ordinary memory accesses.
Introduce the ArithmeticToGEP canonicalisation pass. It detects
pointers (LLVM-typed pointer values, model pointers flagged by the
"revng.pointers" metadata, and pointer-tagged extract values) and
rewrites integer arithmetic computed on top of them as i8 GEPs,
followed by a ptrtoint when the original use needs an integer.
For each obvious pointer the pass also walks backwards through Add
chains and disambiguates the pointer operand of each Add when the
other operand cannot be a pointer, so the rewrite extends to integer
arithmetic that only later flows into a pointer use.
Drop the special case in serializeToLocalVariable that, for
struct-typed values, allocated `[N x i8]` of the struct's data-layout
size instead of an alloca of the struct type itself. The Clifter now
handles arbitrary alloca element types directly, so we can just use
the value's LLVM type.
In non-legacy mode, replace the hand-written aliasing checks in
AEMFP::mayClobber with the answer of LLVM's AAManager, queried
through an AliasAnalysis * threaded into the framework. Legacy mode
keeps its previous logic, since it operates on calls to opaque
copy/assign functions that LLVM AA cannot reason about.
The pass now registers and consumes AAManager via the
FunctionAnalysisManager, with the usual GlobalsAA / module-proxy
cross-registration so that local AA can fall back to GlobalsAA
without crashing.
Make AEMFP::mayClobber and AEMFP::applyTransferFunctionImpl proper
members instead of free functions taking the framework state as a
parameter. This removes a fair amount of boilerplate around passing
the AliasAnalysis pointer along, and keeps all the logic that
operates on the lattice in one place.
No behavioural change.
Convert SwitchToStatements to a new-PassManager FunctionPass and
introduce two function-level analyses,
AvailableExpressionsAnalysis and InstructionToSerializePicker,
backed by AnalysisInfoMixin. The non-legacy production entry point
now sets up an FAM, registers GlobalsAA proxies and runs the pass
through a FunctionPassManager; the legacy entry point keeps using
the legacy FunctionPass wrapper.
The pass also exposes a registerCallbacks hook so the upcoming unit
tests can load it from `revng opt -passes=switch-to-statements-test`
without dragging the model along.
Replace the loose ResultMap typedef with the
MFP::MFIResultMap<AEMFP<IsLegacy>> alias and keep the maximal fixed
point result inside AvailableExpressionsResult, so callers of
AEResult can fetch availability-at-a-point without reaching into a
separate map.
This is a structural step toward turning available-expressions
discovery into a real new-PassManager analysis.
Alessandro Di Federico