Enable extra coverage counters on Windows

[MichaelSquires]

Summary

• Enable extra coverage counters on Windows.
• Update extra_counters.test to run on Windows also.
• Update TableLookupTest.cpp to include the required pragma/declspec for the extra coverage counters.

[MichaelSquires]

Formatted with clang-format

[MichaelSquires]

Formatted with clang-format

[rnk]

Seems reasonable to me, I have no familiarity with the extra counters feature, though. You might consider using explicit alignments.

Something else to consider is that the MSVC incremental linker adds padding between variables to allow resizing them between links. I think these days we just don't care about that anymore. In the past, we did work to make sure asan would work under those conditions.

compiler-rt/lib/fuzzer/FuzzerExtraCounters.cpp

46–48

Maybe __libfuzzer_extra_counter_a / __libfuzzer_extra_counter_z would make that clearer. "r, s, t" seems like a surprising part of the alphabet to use when establishing an ordering with lexicographic sorting.

89

Standard optimizations like loop idiom recognition will undo this. Is this enough to prevent ASan instrumentation? I'd suggest making these pointers volatile to be really confident that this won't become instrumented in the future.

[MichaelSquires]

Updates based on code review.

• Added explicit alignment to start and stop sections.
• Updated comments to better explain the alignment/padding requirements of the sections.
• Update the section names to be more intuitive with regards to sorting.
• Updated manual memset to use SecureZeroMemory to ensure it doesn't get optimized out.

[MichaelSquires]

Ping.

I made updates based on recommendations from @rnk.

I'm also realizing I likely don't have commit access and would appreciate if someone could commit on my behalf (assuming review approval).

Thanks!

[MichaelSquires]

Marking previous comments as done. I didn't realize this was something I could do - sorry, first time using this code review system.

[kcc]

One comment that I have is a request to limit the number of #ifdefs in the code to at most one.
We typically achieve this by having platform-specific code in a platform-specific file, guarded with an ifdef.

an extra ifdef in a test is probably unavoidable.

[MichaelSquires]

Moved platform specific code into separate files per request from @kcc.

[morehouse]

LGTM

[kcc]

compiler-rt/test/fuzzer/extra-counters.test

1

does this imply that the test will now work on Mac?

[MichaelSquires]

Mark extra counters test as unsupported on Darwin.

[MichaelSquires]

compiler-rt/test/fuzzer/extra-counters.test

1

No. Good catch, thanks! I updated the test to be unsupported on darwin.

[MichaelSquires]

Can someone commit this for me please? I don't have access.

[mstorsjo]

compiler-rt/lib/fuzzer/FuzzerExtraCountersWindows.cpp

15 ↗

(On Diff #364563)

This broke cross building from case sensitive filesystems with mingw; I pushed a fix for it in ab737d5367cdea30df71ef66f87fe2c6d88e7246.

compiler-rt/lib/fuzzer/FuzzerExtraCounters.cpp

 #include "FuzzerPlatform.h"
 #include <cstdint>
 
+#if LIBFUZZER_WINDOWS
+#include <Windows.h>
+#endif
+
 #if LIBFUZZER_LINUX || LIBFUZZER_NETBSD || LIBFUZZER_FREEBSD ||                \
     LIBFUZZER_FUCHSIA || LIBFUZZER_EMSCRIPTEN
 __attribute__((weak)) extern uint8_t __start___libfuzzer_extra_counters;

 
 }  // namespace fuzzer
 
+#elif LIBFUZZER_WINDOWS
+
+namespace fuzzer {
+
+//
+// The __start___libfuzzer_extra_counters variable is align 16, size 16 to
+// ensure the padding between it and the next variable in this section (either
+// __libfuzzer_extra_counters or __stop___libfuzzer_extra_counters) will be
+// located at (__start___libfuzzer_extra_counters +
+// sizeof(__start___libfuzzer_extra_counters)). Otherwise, the calculation of
+// (stop - (start + sizeof(start))) might be skewed.
+//
+// The section name, __libfuzzer_extra_countaaa ends with "aaa", so it sorts
+// before __libfuzzer_extra_counters alphabetically. We want the start symbol to
+// be placed in the section just before the user supplied counters (if present).
+//
+#pragma section(".data$__libfuzzer_extra_countaaa")
+ATTRIBUTE_ALIGNED(16)
+__declspec(allocate(".data$__libfuzzer_extra_countaaa")) uint8_t
+    __start___libfuzzer_extra_counters[16] = {0};
+
+//
+// Example of what the user-supplied counters should look like. First, the
+// pragma to create the section name. It will fall alphabetically between
+// ".data$__libfuzzer_extra_countaaa" and ".data$__libfuzzer_extra_countzzz".
+// Next, the declspec to allocate the variable inside the specified section.
+// Finally, some array, struct, whatever that is used to track the counter data.
+// The size of this variable is computed at runtime by finding the difference of
+// __stop___libfuzzer_extra_counters and __start___libfuzzer_extra_counters +
+// sizeof(__start___libfuzzer_extra_counters).
+//
+
+//
+//     #pragma section(".data$__libfuzzer_extra_counters")
+//     __declspec(allocate(".data$__libfuzzer_extra_counters"))
+//         uint8_t any_name_variable[64 * 1024];
+//
+
+//
+// Here, the section name, __libfuzzer_extra_countzzz ends with "zzz", so it
+// sorts after __libfuzzer_extra_counters alphabetically. We want the stop
+// symbol to be placed in the section just after the user supplied counters (if
+// present). Align to 1 so there isn't any padding placed between this and the
+// previous variable.
+//
+#pragma section(".data$__libfuzzer_extra_countzzz")
+ATTRIBUTE_ALIGNED(1)
+__declspec(allocate(".data$__libfuzzer_extra_countzzz")) uint8_t
+    __stop___libfuzzer_extra_counters = 0;
+
+uint8_t *ExtraCountersBegin() {
+  return __start___libfuzzer_extra_counters +
+         sizeof(__start___libfuzzer_extra_counters);
+}
+
+uint8_t *ExtraCountersEnd() { return &__stop___libfuzzer_extra_counters; }
+
+ATTRIBUTE_NO_SANITIZE_ALL
+void ClearExtraCounters() {
+  uint8_t *Beg = ExtraCountersBegin();
+  SecureZeroMemory(Beg, ExtraCountersEnd() - Beg);
+}
+
+} // namespace fuzzer
+
 #else
 // TODO: implement for other platforms.
 namespace fuzzer {

compiler-rt/test/fuzzer/TableLookupTest.cpp

 #ifdef __linux__
 __attribute__((section("__libfuzzer_extra_counters")))
 #endif
+#ifdef _WIN32
+#pragma section(".data$__libfuzzer_extra_counters")
+__declspec(allocate(".data$__libfuzzer_extra_counters"))
+#endif
 static uint8_t Counters[N];
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {

compiler-rt/test/fuzzer/extra-counters.test

-REQUIRES: linux
-
 RUN: %cpp_compiler %S/TableLookupTest.cpp -o %t-TableLookupTest
 RUN: not %run %t-TableLookupTest -print_final_stats=1 2>&1 | FileCheck %s
 CHECK: INFO: {{[0-9]+}} Extra Counters