This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer coverage] coverage pc buffer
ClosedPublic

Authored by aizatsky on Jan 4 2016, 2:07 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

aizatsky updated this revision to Diff 43922.Jan 4 2016, 2:07 PM
aizatsky retitled this revision from to [sanitizer coverage] coverage pc buffer.
aizatsky updated this object.
aizatsky updated this revision to Diff 43934.Jan 4 2016, 3:31 PM

test & nits

aizatsky added a reviewer: kcc.Jan 4 2016, 3:33 PM
aizatsky added a project: Restricted Project.
aizatsky added a subscriber: llvm-commits.
kcc added inline comments.Jan 4 2016, 3:53 PM
include/sanitizer/coverage_interface.h
45 ↗(On Diff #43934)

.. of the ...
Isn't the return value the same as __sanitizer_get_total_unique_coverage() ?
If so, it should be reflected in the comment or in the interface

lib/sanitizer_common/sanitizer_coverage_libcdep.cc
215 ↗(On Diff #43934)

just in case, hide it under a flag (coverage_pc_buffer?).
We can make the flag on by default.

test/asan/TestCases/coverage-pcbuffer.cc
4 ↗(On Diff #43934)

do you need coverage=1 here?

aizatsky updated this revision to Diff 43944.Jan 4 2016, 4:40 PM
aizatsky marked 3 inline comments as done.

review

All done. PTAL.

kcc accepted this revision.Jan 4 2016, 4:48 PM
kcc edited edge metadata.

LGTM

lib/sanitizer_common/sanitizer_coverage_libcdep.cc
218 ↗(On Diff #43945)

Style: I would prefer
X = nullptr;
if (cond())

X = ...
This revision is now accepted and ready to land.Jan 4 2016, 4:48 PM
This revision was automatically updated to reflect the committed changes.
aizatsky marked an inline comment as done.