Page MenuHomePhabricator

[SanitizerCoverage] Add compiler-rt test for -fsanitize-coverage=inline-bool-flag
ClosedPublic

Authored by pratyai on Apr 7 2020, 4:33 AM.

Diff Detail

Event Timeline

pratyai created this revision.Apr 7 2020, 4:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2020, 4:33 AM
Herald added subscribers: Restricted Project, dberris. · View Herald Transcript
vitalybuka accepted this revision.Apr 8 2020, 6:03 PM
vitalybuka added inline comments.
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
5

can you add comment why it's unsupported here, and XFAIL tsan

This revision is now accepted and ready to land.Apr 8 2020, 6:03 PM
pratyai marked an inline comment as done.Apr 8 2020, 10:18 PM
pratyai added inline comments.
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
5

This is something I just copied from https://reviews.llvm.org/source/llvm-github/browse/master/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp, but cannot find a reference why exactly darwin and tsan are problem.

I can see that trying to instrument both thread and coverage emits a warning about just ignoring the latter (a.cc is just some helloworld code).
/tmp $ clang++ -g a.cc -fsanitize=thread -fsanitize-coverage=trace-pc-guard
clang: warning: argument unused during compilation: '-fsanitize-coverage=trace-pc-guard' [-Wunused-command-line-argument]

The i38c-darwin was there from the version#0 of that^ file. Although originally it had more entries, some of which got removed in https://reviews.llvm.org/rGdd862f9106199ce2dd04ebc93e9bfd203465c1ee.

Not really sure what to comment beside // Kept same as 'sanitizer_coverage_inline8bit_counter.cpp'

vitalybuka retitled this revision from [part 3] sancov/inline-bool-flag compiler-rt test. to [SanitizerCoverage] Add compiler-rt test for -fsanitize-coverage=inline-bool-flag.Apr 8 2020, 10:47 PM
vitalybuka edited the summary of this revision. (Show Details)

I'll submit this a little bit later

pratyai added a comment.EditedApr 9 2020, 2:50 AM

I'll submit this a little bit later

I realized only now that I didn't update a few files in compiler-rt that provides weak, empty implementations:

(the last one was the reason of the fuzzer failure on the previous diff I guess)

Should I add them in this diff (/ another diff / not necessary)?

Yes, please.

I'll submit this a little bit later

I realized only now that I didn't update a few files in compiler-rt that provides weak, empty implementations:

This revision was automatically updated to reflect the committed changes.

vitalybuka: See the "fatal error" on http://reviews.llvm.org/rGd6525eff5 . Is it intentional (or known) that these section names are too long for mach-o?

fhahn added a subscriber: fhahn.Apr 10 2020, 4:42 AM

It looks like this change breaks some macOS builds, e.g. http://green.lab.llvm.org/green/job/clang-stage1-RA/8593/

I’ve disabled it for x86_64-darwin as well (it was already unsupported for i386-darwin) in d6525eff5ebfa0ef1d6cd75cb9b40b1881e7a707

It would be great if you could take a look.

It looks like this change breaks some macOS builds, e.g. http://green.lab.llvm.org/green/job/clang-stage1-RA/8593/

I’ve disabled it for x86_64-darwin as well (it was already unsupported for i386-darwin) in d6525eff5ebfa0ef1d6cd75cb9b40b1881e7a707

It would be great if you could take a look.

Sorry for the breakage; the problematic section was added in D77244 (SanCovBoolFlagSectionName in llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp). I was wondering why the previous section name was dropping the vowels, but didn't realize that there was a character restriction.

If you think just shortening the name to something like sancov_bools would be sufficient, I can send a diff (, how do I test it for mac builds though?).

Thanks!