[PATCH] D28635: [libFuzzer] Portably disassemble and find calls to "sanitizer_cov_trace_pc_guard".

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added inline comments.


================
Comment at: lib/Fuzzer/FuzzerTracePC.cpp:162
+      size_t PcOffsetEnd = S.find(':');
+      if (PcOffsetEnd == std::string::npos) continue;
+      S.resize(PcOffsetEnd);
----------------
zturner wrote:
> Can you put the `continue` on a new line?
Ok, I will do that. But, in line 169, the continue is in the same line too. Do you want me to update that too?


================
Comment at: lib/Fuzzer/FuzzerUtilWindows.cpp:181
+std::string DisassembleCmd(const std::string &FileName) {
+  return "dumpbin /disasm " + FileName;
+}
----------------
zturner wrote:
> `dumpbin` is not guaranteed to be in the user's path.  Is this going to cause a problem?  Also I believe `dumpbin` will use Intel syntax for the disassembly.  Does objdump use AT&T syntax by default?  If so, will this cause a problem?
@zturner you are right. I didn't realize of that since I was using a terminal with the VS environment.
We can use :  `link /dump /disasm` , which is included in PATH, but I couldn't find information if the `/dump` option is a new option or it is also included in old versions.
Also, I think we should consider the case when users only using llvm tools, so in that case we could look for: `llvm-objdump`

So, I propose to do this:
First: See if `dumpbin` is present in PATH,
Else: See if `link`  is present in PATH,
Else: See if `llvm-objdump` is present in PATH,
Else: Fail

Would you agree?


Repository:
  rL LLVM

https://reviews.llvm.org/D28635