Skip to content
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

Adds optional '-logprefix' command line option to drcov #3510

Merged
merged 3 commits into from
Apr 6, 2019
Merged

Adds optional '-logprefix' command line option to drcov #3510

merged 3 commits into from
Apr 6, 2019

Conversation

gaasedelen
Copy link
Contributor

@gaasedelen gaasedelen commented Apr 4, 2019

TL;DR

This PR adds an optional -logprefix command line toggle to the drcov tool.

By default, drcov will prefix log files with 'drcov', eg drcov.ls.57735.0000.proc.log. The addition of this command line option allows users to change the prefix to a string of their choosing. This can be used to build a clear relationship between which invocation produced which log file.

Problem Usecase

A growing number of DynamoRIO users have been integrating drcov into their fuzzing and instrumentation workflows. A common nuisance among these users is that there isn't a great way for a fuzz harness (or anything) that wraps drcov to specify an output filename for the coverage log.

As a result, it can be difficult to correlate which fuzzed input testcase generated which log after the fact with any certainty.

doom@upwn64:~/projects/dynamorio/testing$ ../build/bin64/drrun -t drcov -- /bin/ls /what/the/fuzz
/bin/ls: cannot access '/what/the/fuzz': No such file or directory

doom@upwn64:~/projects/dynamorio/testing$ ../build/bin64/drrun -t drcov -- /bin/ls .
drcov.ls.57735.0000.proc.log  drcov.ls.57736.0000.proc.log

doom@upwn64:~/projects/dynamorio/testing$ ls -al
total 80
drwxrwxr-x  2 doom doom  4096 Apr  4 14:21 .
drwxrwxr-x 14 doom doom  4096 Apr  4 13:57 ..
-rw-rw----  1 doom doom 37465 Apr  4 14:21 drcov.ls.57735.0000.proc.log
-rw-rw----  1 doom doom 32505 Apr  4 14:21 drcov.ls.57736.0000.proc.log

Which log is which? Obviously, this becomes increasingly tedious when there are tens of thousands of executions / logs being generated simultaneously.

Example Usage

The example below demonstrates the usage of the new -logprefix option:

doom@upwn64:~/projects/dynamorio/testing$ ../build/bin64/drrun -t drcov -- /bin/ls
1.txt  2.txt  drcov.ls.57606.0000.proc.log

doom@upwn64:~/projects/dynamorio/testing$ ../build/bin64/drrun -t drcov -logprefix "testcase_123" -- /bin/ls
1.txt  2.txt  drcov.ls.57606.0000.proc.log  testcase_123.ls.57609.0000.proc.log

doom@upwn64:~/projects/dynamorio/testing$ ls -al
total 76
drwxrwxr-x  2 doom doom  4096 Apr  4 13:58 .
drwxrwxr-x 14 doom doom  4096 Apr  4 13:57 ..
-rw-rw-r--  1 doom doom     0 Apr  4 13:57 1.txt
-rw-rw-r--  1 doom doom     7 Apr  4 13:57 2.txt
-rw-rw----  1 doom doom 32041 Apr  4 13:58 drcov.ls.57606.0000.proc.log
-rw-rw----  1 doom doom 32201 Apr  4 13:58 testcase_123.ls.57609.0000.proc.log

Additional Comments

Most users would prefer the ability to specify a static / known log filename. But being able to change the log file prefix seemed like a fair compromise and the path of least resistance for the existing codebase.

I am open to alternative solutions, but would like to see a change like this integrated in one form or another.

@Carrotman42
Copy link
Contributor

Derek volunteered to take a look.

@derekbruening
Copy link
Contributor

Thank you for the detailed PR description!

There is the -logdir option already but I could see not wanting every individual run to go into its own single-file directory. Will take a look at the patch next.

The Appveyor failure is the flaky type_is_instr assert #3320, grrr.

@derekbruening derekbruening merged commit 9d6a10e into DynamoRIO:master Apr 6, 2019
hgreving2304 pushed a commit that referenced this pull request Apr 22, 2019
Adds a new command line option '-logprefix' to drcov for better control over the output file.
Updates the changelog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants