Page MenuHomePhabricator

[libFuzzer] Portably disassemble and find calls to "sanitizer_cov_trace_pc_guard".
ClosedPublic

Authored by mpividori on Jan 12 2017, 7:33 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

mpividori updated this revision to Diff 84212.Jan 12 2017, 7:33 PM
mpividori retitled this revision from to [libFuzzer] Portably disassemble and find calls to "sanitizer_cov_trace_pc_guard"..
mpividori updated this object.
mpividori added reviewers: kcc, zturner.
mpividori set the repository for this revision to rL LLVM.
mpividori added a subscriber: llvm-commits.
zturner added inline comments.Jan 13 2017, 10:15 AM
lib/Fuzzer/FuzzerTracePC.cpp
162 ↗(On Diff #84212)

Can you put the continue on a new line?

lib/Fuzzer/FuzzerUtilWindows.cpp
181 ↗(On Diff #84212)

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?

mpividori added inline comments.Jan 13 2017, 1:53 PM
lib/Fuzzer/FuzzerTracePC.cpp
162 ↗(On Diff #84212)

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?

lib/Fuzzer/FuzzerUtilWindows.cpp
181 ↗(On Diff #84212)

@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?

mpividori updated this revision to Diff 85192.Jan 20 2017, 2:10 PM

@zturner , I update diff to include suggestions.

zturner accepted this revision.Jan 20 2017, 2:13 PM
This revision is now accepted and ready to land.Jan 20 2017, 2:13 PM
This revision was automatically updated to reflect the committed changes.