Page MenuHomePhabricator

[sanitizer] Add a variant of "include" flag that does not abort on missing file and support process name substitution.
ClosedPublic

Authored by eugenis on Jul 20 2015, 5:05 PM.

Details

Reviewers
kcc
samsonov
Summary

include_if_exists=/path/to/asan/options reads flags from the file if it is present.
"%b" in the include file path (for both variants of the flag) is replaced with the basename of the main executable.

Diff Detail

Event Timeline

eugenis updated this revision to Diff 30216.Jul 20 2015, 5:05 PM
eugenis retitled this revision from to [sanitizer] Add a variant of "include" flag that does not abort on missing file..
eugenis updated this object.
eugenis added reviewers: samsonov, kcc.
eugenis set the repository for this revision to rL LLVM.
eugenis added a subscriber: llvm-commits.
samsonov accepted this revision.Jul 21 2015, 12:16 PM
samsonov edited edge metadata.

LGTM

lib/sanitizer_common/sanitizer_flags.cc
89

I would be prefer smth. more verbose like "optional_include", but this is optional.

This revision is now accepted and ready to land.Jul 21 2015, 12:16 PM
eugenis updated this revision to Diff 30281.Jul 21 2015, 1:10 PM
eugenis retitled this revision from [sanitizer] Add a variant of "include" flag that does not abort on missing file. to [sanitizer] Add a variant of "include" flag that does not abort on missing file and support process name substitution..
eugenis updated this object.
eugenis edited edge metadata.
eugenis removed rL LLVM as the repository for this revision.
eugenis updated this revision to Diff 30284.Jul 21 2015, 1:29 PM
samsonov added inline comments.Jul 21 2015, 2:58 PM
lib/sanitizer_common/sanitizer_flags.cc
48

Add a comment that out should be at least kMaxPathLength. Also, you don't seem to actually trim s to this. constant if necessary. Consider using InternalScopedString *out and out->append() instead.

55

if base was trimmed, you would advance pointer t too far, and may make an out-of-bound write on the next iteration.

eugenis updated this revision to Diff 30291.Jul 21 2015, 2:59 PM
eugenis updated this object.

Renamed the option to a more obvious name.
Do you think we should change "%b" to something else? "b" stands for basename (as in man 1 basename). Maybe %n?

I have no strong preference. %b looks okay.

eugenis updated this revision to Diff 30293.Jul 21 2015, 3:15 PM

fixed oob issues

eugenis closed this revision.Jul 21 2015, 4:03 PM

Thanks. Committed as r242853.