<div dir="ltr">On Fri, Jun 14, 2013 at 6:35 AM, Bloom, Louis S. <<a href="mailto:lbloom@draper.com">lbloom@draper.com</a>> wrote:<br>> Hi Ahmed,<br>><br>> I've been playing with this CFG reconstruction patch and discovered issues with branch evaluation when the operands are encoded as MCExpr types instead of immediates. This is a problem when a disassembler has been set up for symbolic disassembly as many operands are encoded as MCExpr types. See attached patches, I modified the evaluateBranch function inside of MCInstrAnalysis.cpp in order to handle branch evaluation of MCExpr types. I have also modified the tryAddingSymbolicOperand function inside of MCExternalSymbolizer.cpp to encode the immediate values necessary within MCExpr types as they correspond to the immediate operands which the symbols they replace. Ultimately, this allows for CFG reconstruction to occur as expected even during symbolic disassembly.<br>

><br>> The patches do the following:<br>><br>> * ext-sym-patch1.txt - Adds the original immediate value to symbolic operands.<br>> * instr-anal-patch2.txt - Using ext-sym-patch1.txt, adds support for branch evaluation by evaluating MCExprs as immediates.<br>

<br>Hi Louis,<br><br>There are definitely lots of improvements to be made in the area, thanks for<br>having a look!  I am currently working on stuff that will eventually need this,<br>but even though you're on the right track, the patch needs a few changes.<br>

<br>- First, the value set to an MCSymbol should be the symbol's absolute value.<br>  In this case, the patches make two changes that cancel each other out:<br>  + in evaluateBranch, we only need to add Addr+Size to immediate PC-relative<br>

    operands, and not to the result of EvaluateAsAbsolute (which, as the name<br>    indicates, returns an absolute value, as opposed to a PC-relative one).<br>  + in MCExternalSymbolizer, the values assigned to the MCSymbols should be<br>

    absolute ("Value"), not PC-relative-ish ("- Address - Offset - InstSize").<br><br>- For both SymbolicOp.AddSymbol and .SubSymbol, the patch sets the same ImmVal,<br>  which is the one that's already in SymbolicOp.Value. The correct values can't<br>

  be determined in MCExternalSymbolizer, since the C API states that, in the<br>  LLVMOpInfoSymbol1 struct, you can't have both .Name and .Value, which is the<br>  correct value in this case.<br><br>- This brings us to another point: we've shown that this can't easily work for<br>

  MCExternalSymbolizer, but this isn't actually a problem, since if you can<br>  create an MC CFG, you need an ObjectFile, which is enough to run the<br>  MCObjectSymbolizer. Basically, MCExternalSymbolizer shouldn't change, as it's<br>

  used in places (otool, lldb) where you "can't” create an MC CFG anyway.<br>  MCObjectSymbolizer should, as they both interact when you run<br>  'llvm-objdump -cfg -symbolize’. If you somehow setup your own symbolic<div>

  callbacks on an MCDisassembler you give to MCObjectDisassembler,</div><div>  you should switch to MCObjectSymbolizer!</div><div><div><br>- If the generic evaluateBranch should handle MCExprs, then so should the<br>  target-specific ones (look at *MCTargetDesc.cpp for both AArch64 and ARM).<br>

<br><br>All in all, there are two possibilities here:</div><div>- either working around the problem: in llvm-objdump, moving the "if (CFG)”</div><div>  block to before the "if (Symbolize)” one, and ensuring (documentation +</div>

<div>  assert) that the disassembler given to an MCObjectDisassembler doesn’t</div><div>  have symbolization enabled (this is redundant/useless for the current</div><div>  MCObjectDisassembler anyway).</div><div>- properly handling MCExprs in MCInstrAnalysis, and setting the right</div>

<div>  value whenever we create an MCSymbol, given the comments above.</div><div><br>Thanks!<br><br>-- Ahmed Bougacha<br><br>> Very Respectfully,<br>> LOUIS S. BLOOM, 2d Lt, USAF<br>> AFIT Student - Northeastern University<br>

> Draper Laboratory Fellow<br>><br>> DISCLAIMER: The views expressed are those of the author and do not reflect the official policy or position of the Department of Defense or the U.S. Government.<br>><br>> -----Original Message-----<br>

> From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Ahmed Bougacha<br>

> Sent: Thursday, May 23, 2013 9:07 PM<br>> To: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>> Subject: [llvm] r182628 - MC: Disassembled CFG reconstruction.<br>><br>> Author: ab<br>

> Date: Thu May 23 20:07:04 2013<br>> New Revision: 182628<br>><br>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=182628&view=rev">http://llvm.org/viewvc/llvm-project?rev=182628&view=rev</a><br>

> Log:<br>> MC: Disassembled CFG reconstruction.<br>><br>> This patch builds on some existing code to do CFG reconstruction from<br>> a disassembled binary:<br>> - MCModule represents the binary, and has a list of MCAtoms.<br>

> - MCAtom represents either disassembled instructions (MCTextAtom), or<br>>   contiguous data (MCDataAtom), and covers a specific range of addresses.<br>> - MCBasicBlock and MCFunction form the reconstructed CFG. An MCBB is<br>

>   backed by an MCTextAtom, and has the usual successors/predecessors.<br>> - MCObjectDisassembler creates a module from an ObjectFile using a<br>>   disassembler. It first builds an atom for each section. It can also<br>

>   construct the CFG, and this splits the text atoms into basic blocks.<br>><br>> MCModule and MCAtom were only sketched out; MCFunction and MCBB were<br>> implemented under the experimental "-cfg" llvm-objdump -macho option.<br>

> This cleans them up for further use; llvm-objdump -d -cfg now generates<br>> graphviz files for each function found in the binary.<br>><br>> In the future, MCObjectDisassembler may be the right place to do<br>

> "intelligent" disassembly: for example, handling constant islands is just<br>> a matter of splitting the atom, using information that may be available<br>> in the ObjectFile. Also, better initial atom formation than just using<br>

> sections is possible using symbols (and things like Mach-O's<br>> function_starts load command).<br>><br>> This brings two minor regressions in llvm-objdump -macho -cfg:<br>> - The printing of a relocation's referenced symbol.<br>

> - An annotation on loop BBs, i.e., which are their own successor.<br>><br>> Relocation printing is replaced by the MCSymbolizer; the basic CFG<br>> annotation will be superseded by more related functionality.<br>

><br>><br>> Added:<br>>     llvm/trunk/include/llvm/MC/MCFunction.h<br>>     llvm/trunk/include/llvm/MC/MCObjectDisassembler.h<br>>     llvm/trunk/include/llvm/Support/StringRefMemoryObject.h<br>>     llvm/trunk/lib/MC/MCFunction.cpp<br>

>     llvm/trunk/lib/MC/MCObjectDisassembler.cpp<br>>     llvm/trunk/lib/Support/StringRefMemoryObject.cpp<br>> Removed:<br>>     llvm/trunk/tools/llvm-objdump/MCFunction.cpp<br>>     llvm/trunk/tools/llvm-objdump/MCFunction.h<br>

> Modified:<br>>     llvm/trunk/include/llvm/MC/MCAtom.h<br>>     llvm/trunk/include/llvm/MC/MCInstrAnalysis.h<br>>     llvm/trunk/include/llvm/MC/MCModule.h<br>>     llvm/trunk/lib/MC/CMakeLists.txt<br>>     llvm/trunk/lib/MC/MCAtom.cpp<br>

>     llvm/trunk/lib/MC/MCInstrAnalysis.cpp<br>>     llvm/trunk/lib/MC/MCModule.cpp<br>>     llvm/trunk/lib/Support/CMakeLists.txt<br>>     llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp<br>

>     llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp<br>>     llvm/trunk/tools/llvm-objdump/CMakeLists.txt<br>>     llvm/trunk/tools/llvm-objdump/MachODump.cpp<br>>     llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp<br>

>     llvm/trunk/tools/llvm-objdump/llvm-objdump.h<br>><br>> _______________________________________________<br>> llvm-commits mailing list<br>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>

> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>><br>> _______________________________________________<br>> llvm-commits mailing list<br>

> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>></div>

</div></div>