[SanitizerCoverage] Add `__sanitizer_cov_bool_flag_init` as the weak interface functions.

[pratyai]

Summary

Following up the discussion on D77638 (and following rGd6cfed6060c283dc4a6bf9ca294dcd732e8b9f72 as example), defining __sanitizer_cov_bool_flag_init as the weak interface functions in various compiler-rt/ files.

[Please note that 2 clang-tidy checks fail. I believe clang-tidy is mistaken in this case.]

Not included here:

• compiler-rt/lib/fuzzer/FuzzerTracePC.cpp [need to implement the handler function]
• compiler-rt/lib/fuzzer/FuzzerTracePC.h
• clang/lib/Driver/SanitizerArgs.cpp [tied to FuzzerTracePC.cpp]
• compiler-rt/lib/fuzzer/CMakeLists.txt
• clang/test/Driver/fuzzer.c
• compiler-rt/lib/sanitizer_common/sanitizer_coverage_win_sections.cpp [this file is about windows specific tricks]

I'll put all the fuzzer related changes together in another diff.

[vitalybuka]

could you try to add libfuzzer test?

[pratyai]

In D77857#1982356, @vitalybuka wrote:

could you try to add libfuzzer test?

Will add a test in compiler-rt/test/fuzzer/.

[pratyai]

Added LinkTest.cpp and link.test, just to verify that the weak interfaces would allow to combine the -fsanitize-coverage=inline-bool-flag with -fsanitize=fuzzer without linker errors.

Verified by dropping the weak interfaces and keeping the tests: the test would complain that __sanitizer_cov_bool_flag_init has undefined reference.

[pratyai]

I'll take an attempt of landing it today if it's alright, since it's accepted. Was waiting in case the newly added test files were problematic. Thank!

[pratyai]

Hmm, it seems I cannot do arc land (permission denied, 403). In that case, would you please land the diff? Thanks!

[there are a bunch of python failure but they seem unrelated and have been failing for other diffs too; the clang-tidy errors still false alarm]

[vitalybuka]

Sure. I wiil.

compiler-rt/test/fuzzer/LinkTest.cpp

4 ↗

(On Diff #259682)

I guess you can remove this file and just compile any existing file

[pratyai]

compiler-rt/test/fuzzer/LinkTest.cpp

4 ↗

(On Diff #259682)

Will do; will update the diff after running the tests.

[vitalybuka]

I removed the test. After some consideration is not a good place, but even in sanitizer_common it's not very useful as it's hard to get new problems like this in future.

I'll put all the fuzzer related changes together in another diff.

What kind of changes?

[pratyai]

In D77857#2003284, @vitalybuka wrote:

I removed the test. After some consideration is not a good place, but even in sanitizer_common it's not very useful as it's hard to get new problems like this in future.

I'll put all the fuzzer related changes together in another diff.

What kind of changes?

Fuzzer seems to automatically include other sanitizer coverage features,:
https://reviews.llvm.org/source/llvm-github/browse/master/clang/lib/Driver/SanitizerArgs.cpp$406
And have callbacks for them:
https://reviews.llvm.org/source/llvm-github/browse/master/compiler-rt/lib/fuzzer/FuzzerTracePC.cpp$38
If the new bool flag feature should also be in there, I can add it. If it's not necessarily the case, I can leave it for future decisions.

Then there is
https://reviews.llvm.org/source/llvm-github/browse/master/compiler-rt/lib/sanitizer_common/sanitizer_coverage_win_sections.cpp$35
which seems to be necessary for the new flag to work on windows systems? I'm trying to figure out a way to test it before sending any diffs. Please let me know if that change is unnecessary too.

I couldn't find anything else that might need change other than what's listed in the description-- at least not from looking at how the inline 8bit counters is used in the codebase.

[vitalybuka]

In D77857#2003413, @pratyai wrote:

In D77857#2003284, @vitalybuka wrote:

I removed the test. After some consideration is not a good place, but even in sanitizer_common it's not very useful as it's hard to get new problems like this in future.

I'll put all the fuzzer related changes together in another diff.

What kind of changes?

Fuzzer seems to automatically include other sanitizer coverage features,:
https://reviews.llvm.org/source/llvm-github/browse/master/clang/lib/Driver/SanitizerArgs.cpp$406
And have callbacks for them:
https://reviews.llvm.org/source/llvm-github/browse/master/compiler-rt/lib/fuzzer/FuzzerTracePC.cpp$38
If the new bool flag feature should also be in there, I can add it. If it's not necessarily the case, I can leave it for future decisions.

Then there is
https://reviews.llvm.org/source/llvm-github/browse/master/compiler-rt/lib/sanitizer_common/sanitizer_coverage_win_sections.cpp$35
which seems to be necessary for the new flag to work on windows systems? I'm trying to figure out a way to test it before sending any diffs. Please let me know if that change is unnecessary too.

I couldn't find anything else that might need change other than what's listed in the description-- at least not from looking at how the inline 8bit counters is used in the codebase.

I think this is unnecessary, even if some other counter are used by libFuzzer. In the current state bool flags does not break anything and we add them into libFuzzer only when needed.
But I don't mind to review your changes if you like to continue.

compiler-rt/lib/sanitizer_common/sanitizer_coverage_interface.inc

 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_trace_div4)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_trace_div8)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_trace_gep)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_trace_pc_guard)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_trace_pc_guard_init)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_trace_pc_indir)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_trace_switch)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_8bit_counters_init)
+INTERFACE_WEAK_FUNCTION(__sanitizer_cov_bool_flag_init)
 INTERFACE_WEAK_FUNCTION(__sanitizer_cov_pcs_init)

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp

 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_const_cmp4, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_const_cmp8, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_switch, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_div4, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_div8, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_gep, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_pc_indir, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_8bit_counters_init, void) {}
+SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_bool_flag_init, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_pcs_init, void) {}
 }  // extern "C"
 // Weak definition for code instrumented with -fsanitize-coverage=stack-depth
 // and later linked with code containing a strong definition.
 // E.g., -fsanitize=fuzzer-no-link
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE uptr __sancov_lowest_stack;
 
 #endif  // !SANITIZER_FUCHSIA

compiler-rt/lib/sanitizer_common/sanitizer_interface_internal.h

extern "C" {
   void __sanitizer_cov_trace_pc_indir();
   SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
   void __sanitizer_cov_trace_pc_guard(__sanitizer::u32*);
   SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
   void __sanitizer_cov_trace_pc_guard_init(__sanitizer::u32*,
                                            __sanitizer::u32*);
   SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
   void __sanitizer_cov_8bit_counters_init();
-  SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
-  void __sanitizer_cov_pcs_init();
+  SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE void
+  __sanitizer_cov_bool_flag_init();
+  SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE void
+  __sanitizer_cov_pcs_init();
 } // extern "C"
 
 #endif  // SANITIZER_INTERFACE_INTERNAL_H