Page MenuHomePhabricator

[profile] In Android, do not mkdir() dirs in GCOV_PREFIX
ClosedPublic

Authored by pirama on Jul 24 2019, 3:12 PM.

Details

Summary

In Android, attempting to mkdir() or even stat() top-level directories
like /data causes noisy selinux denials. During whole-system coverage
instrumentation, this causes a deluge of noisy messages that drown out
legitimate selinux denials, that should be audited and fixed.

To avoid this, skip creating any directory in GCOV_PREFIX (thereby
assuming that it exists).

  • Android platform ensures that the GCOV_PREFIX used in Android is

created and read/writable by all processes.

  • This only affects the Android platform (by checking against

ANDROID_API_FUTURE) and for apps built with Clang coverage, the
runtime will still create any non-existent parent directories for the
coverage files.

Diff Detail

Repository
rL LLVM

Event Timeline

pirama created this revision.Jul 24 2019, 3:12 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 24 2019, 3:12 PM
Herald added subscribers: Restricted Project, krytarowski. · View Herald Transcript
davidxl added inline comments.Jul 24 2019, 8:43 PM
compiler-rt/lib/profile/InstrProfilingUtil.c
53 ↗(On Diff #211608)

getenv is repeatedly called in the loop which is not desired. Can you add a helper function and call it outside the loop to 'strip' the prefix from path?

pirama updated this revision to Diff 211818.Jul 25 2019, 2:13 PM

Refactored the loop to call getenv just once.

pirama updated this revision to Diff 211819.Jul 25 2019, 2:14 PM

Remove stray new line.

pirama marked an inline comment as done.Jul 25 2019, 2:16 PM
pirama added inline comments.
compiler-rt/lib/profile/InstrProfilingUtil.c
53 ↗(On Diff #211608)

I refactored to call getenv() just once. I kept the special case within this function to avoiding adding #ifdef ANDROID at two different places, but happy to add if you think that'd make this function cleaner.

davidxl added inline comments.Jul 25 2019, 2:20 PM
compiler-rt/lib/profile/InstrProfilingUtil.c
52 ↗(On Diff #211819)

needs null check of gcov_prefix.

pirama updated this revision to Diff 211821.Jul 25 2019, 2:26 PM

Call strlen(gcov_prefix) only if its not null.

pirama marked an inline comment as done.Jul 25 2019, 2:27 PM
pirama added inline comments.
compiler-rt/lib/profile/InstrProfilingUtil.c
52 ↗(On Diff #211819)

Aah, good catch. Sorry I missed this.

davidxl accepted this revision.Jul 25 2019, 2:48 PM

lgtm

This revision is now accepted and ready to land.Jul 25 2019, 2:48 PM
pirama updated this revision to Diff 211833.Jul 25 2019, 3:08 PM

Apply clang-format

This revision was automatically updated to reflect the committed changes.