New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature AsmCFG #309
Feature AsmCFG #309
Conversation
e553f35
to
cd73364
Compare
@@ -142,15 +142,13 @@ | |||
|
|||
|
|||
# Generate dotty graph | |||
all_blocs = [] | |||
all_blocs = BasicBlocks() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is intended, but I noticed that you switch to "blocks", in general. In some parts of this PR you leave it as "bloc". Especially in the code above and below, down to asmbloc.py.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, "bloc" is the French word for "block". This is an error which is repeated all along Miasm, even in main structure (asm_bloc
) or module name (miasm2.core.asmbloc
).
Instead of breaking a lot of APIs, I prefer correct them inside function when I modify code around it. But we may correct it once and for all, a day.
So, following this reasoning, I will fix this one 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a quick review, there are too many lines to change regarding this correction, lines which are not correlated with the logic of the concerned commit.
Then, I prefer to make a separate commit for all of them, and, a separate PR for the same reasons.
Thanks for your review!
cd73364
to
d6222c4
Compare
Introduce
AsmCFG
, a class standing for an ASM Control Flow Graph (CFG). This class aims to replace the old representation of ASM blocks as a list ofasm_bloc
instance.Indeed, the representation as a
DiGraph
seems to be closer to what one can expect as an output of a disassembler. In addition, algorithm applied on blocks list, such as block splitting, bad destination retrieving, block graphing and sanity checking for assembler are working on a graph (but were usingblock.bto
s to build a kind of intermediate graph construction, ending in more complex code snippets). As these algorithms were built to work on a disassembler outputs and are strongly connected to it, they have been rewritten asAsmCFG
method.In order to be much easier to use, edges are binded on blocks constraints, ie. adding or deleting an edge will impact implicated
.bto
s. The reverse operation (re-synchronization between.bto
s) and edges can be done thanks to arebuild_edges
helper.A regression test for
miasm2.core.asmbloc
has also been added (it should be enough commented to illustrates some uses and associated APIs).As an example, with the new API:
To prevent hours of debugging, a few
DeprecationWarning
with hints have been added on old APIs.As they are related, this PR also introduces
DiGraphSimplifier
, which aims to be the counterpart ofExpressionSimplifier
forDiGraph
.One can register graph simplification, which will be applied on a graph (actually, on a copy of this graph) until a fixed point has been reached.
The exposed API is close to the one of
ExpressionSimplfier
, ie usingenable_passes(list of simplifications)
to enable simplification passes, then applying the simplifier thanks tosimp.apply(graph)
orsimp(graph)
to get back a new, simplified, graph.Its use is illustrated in
miasm2.core.asmbloc: bbl_simplifier
which ship a block merging pass (formerbloc_merge
). This implementation gets rid of some piece of code (see 3882129 for more details), and seems, in my opinion, easier to understand.