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.
- change the font of the emitted graph to `Courier`, which is the only
monospace font graphviz guarantees to support
- tweak color of the union definitions to be slightly lighter (so that
it doesn't clash as much with the other colors used)
- change the color of all the "other" nodes (like typedef) from white
to grey, so that they are visible on light backgrounds.
- leave a todo about `buildFieldName` being obsolete.
For now, the only serialization trait we were verifying
a polymorphic TTG type to have was the wrong (the one
that only printed base class's fields).
This commit explicitly disables said serializer and
ensures it's never used.
Here's an illustration of the impact of the changes:
```
model::UpcastableType MyType = getTypeFromSomewhere();
model::Type &View = *MyType;
model::PointerType &Pointer = MyType->asPointer();
serialize(MyType); // Good
serialize(Pointer); // Good
serialize(View); // new: explicit error
// old: only print base type's fields
```
Alessandro Di Federico