[PATCH] [MC] Disassembled CFG reconstruction

[Quentin Colombet]

  Hi Ahmed,

  Thanks for the last updates.
  We are almost done!

  I am still concerned about the way the MCObjectDisassemble handles the MCModule. Same for the clients of MCObjectDisassemble.
  In my opinion, the current approach is half way "MCObjectDisassemble owns the MCModule" and “MCModule is by its own”. I found that confusing.

  With the current model, I think it makes more sense to make MCObjectDisassemble owns the MCModule.
  Here is my proposal, that adds some laziness in the process.
  * Resurrect the MCModule field in MCObjectDisassemble.
  * Assign NULL to the MCModule field in the constructor.
  * Delete the MCModule field in the destructor.
  * Privatize buildModule.
  * Privatize buildCFG.
  * Resurrect the accessor: getModule.
  * Add a new argument to getModule: getModule(bool WithCFG = true)
  Now, as you can imagine, the getModule method will lazily build the MCModule and its MCFunction.

  Here is the code for that function:
  MCModule *MCObjectDisassemble::getModule(bool WithCFG) {
    if (!Module)
      Module = buildModule(MIA); //// old buildModule method
    if (WithCFG && Module->func_empty()) //// You may need to add func_empty method
      buildCFG();
    return Module;
  }

  Note: you may want to return a const MCModule *. You may also want to make getModule a const method and the MCModule field a mutable field.

  What do you think?

  I have inlined a few comments that are partly irrelevant if you go for the proposal.

  Thanks again for working on this.

  Quentin


================
Comment at: include/llvm/MC/MCModule.h:34
@@ -30,1 +33,3 @@
+/// either instructions or data.
+/// An MCModule is created using MCObjectDisassembler::buildModule.
 class MCModule {
----------------
Typo: An -> A.
But here, putting “nothing" would be better :)

================
Comment at: include/llvm/MC/MCObjectDisassembler.h:51
@@ +50,3 @@
+  /// format-specific features, like mach-o function_start or data_in_code LCs.
+  MCModule *buildModule();
+
----------------
It is still not clear that the client has to free the memory allocated for the module.
In fact, I preferred the old method with the getModule (assuming MCObjectDisassemble owns the MCModule).

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:334
@@ +333,3 @@
+                   utostr(filenum++) + ".dot").str().c_str(),
+                    **FI, IP.get());
+    }
----------------
Where is the delete now?


http://llvm-reviews.chandlerc.com/D811

BRANCH
  mcfunc

ARCANIST PROJECT
  llvm