[libFuzzer][MSVC] Make Sanitizer Coverage MSVC-compatible

[metzman]

Summary

Make Sanitizer Coverage work when compiled work when compiler-rt
is compiled with MSVC.

The previous solution did not work for MSVC because MSVC tried to
align the .SCOV$CZ section even though we used
declspec(align(1)) on its only symbol:
stop___sancov_cntrs.
Because the counter array is composed
of 1 byte elements, it does not always end on an 8 or 4 byte
boundary. This means that padding was sometimes added to
added to align the next section, .SCOV$CZ.
Use a different strategy now: instead of only instructing
the compiler not to align the symbol, make the section
one byte long by making its only symbol a uint8_t, so that
the linker won't try to align it.

[metzman]

• add __declspec(align(1) back to be safe

[metzman]

• formatting

[metzman]

• improve comment

[metzman]

Matt and Reid could you please take a look?

This is the last major change needed to support MSVC in libFuzzer.

I figured out that I needed to make this change by using dumpbin on sanitizer libraries.
Before this change, when compiled with MSVC I saw this:

.SCOV$CZ name
C0100040 flags
         8 byte align

even though I saw the section was 1 byte aligned when compiled with clang.
Now that I've made the change the section is 1 byte aligned.

I don't think I need the __declspec(align(1)) any more but I've kept it to be safe.
Also, I think only __stop__sancov_counters needs to be a uint8_t,
__stop__sancov_pcs can probably be anything because the pc table always ends on an address that
is a multiple of word size so no padding is ever added. However I've also made it a uint8_t for consistency.

[metzman]

• undo change made for testing

[morehouse]

Code looks fine to me, but I don't have enough Windows knowledge to to evaluate the approach. I'll let @rnk comment on that.

[rnk]

lgtm

compiler-rt/lib/sanitizer_common/sanitizer_coverage_win_sections.cc

 // variable is actually the address of data that comes before our middle
 // section. We also need to prevent the linker from adding any padding. Each
 // technique we use for this is explained in the comments below.
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_platform.h"
 #if SANITIZER_WINDOWS
 #include <stdint.h>
-extern "C" {
-// The Guard array and counter array should both be merged into the .data
-// section to reduce the number of PE sections However, because PCTable is
-// constant it should be merged with the .rdata section.
-#pragma section(".SCOV$GA", read, write)  // NOLINT
-// Use align(1) to avoid adding any padding that will mess up clients trying to
-// determine the start and end of the array.
-__declspec(allocate(".SCOV$GA")) __declspec(align(1)) uint64_t
-    __start___sancov_guards = 0;
-#pragma section(".SCOV$GZ", read, write)  // NOLINT
-__declspec(allocate(".SCOV$GZ")) __declspec(align(1)) uint64_t
-    __stop___sancov_guards = 0;
 
+extern "C" {
+// Use uint64_t so the linker won't need to add any padding if it tries to word
+// align the start of the 8-bit counters array. The array will always start 8
+// bytes after __start_sancov_cntrs.
 #pragma section(".SCOV$CA", read, write)  // NOLINT
-__declspec(allocate(".SCOV$CA")) __declspec(align(1)) uint64_t
-    __start___sancov_cntrs = 0;
+__declspec(allocate(".SCOV$CA")) uint64_t __start___sancov_cntrs = 0;
+
+// Even though we said not to align __stop__sancov_cntrs (using the "align"
+// declspec), MSVC's linker may try to align the section, .SCOV$CZ, containing
+// it. This can cause a mismatch between the number of PCs and counters since
+// each PCTable element is 8 bytes (unlike counters which are 1 byte) so no
+// padding would be added to align .SCOVP$Z, However, if .SCOV$CZ section is 1
+// byte, the linker won't try to align it on an 8-byte boundary, so use a
+// uint8_t for __stop_sancov_cntrs.
 #pragma section(".SCOV$CZ", read, write)  // NOLINT
 __declspec(allocate(".SCOV$CZ")) __declspec(align(1)) uint64_t
     __stop___sancov_cntrs = 0;
 
+#pragma section(".SCOV$GA", read, write)  // NOLINT
+__declspec(allocate(".SCOV$GA")) uint64_t __start___sancov_guards = 0;
+#pragma section(".SCOV$GZ", read, write)  // NOLINT
+__declspec(allocate(".SCOV$GZ")) __declspec(align(1)) uint8_t
+    __stop___sancov_guards = 0;
+
+// The guard array and counter array should both be merged into the .data
+// section to reduce the number of PE sections. However, because PCTable is
+// constant it should be merged with the .rdata section.
 #pragma comment(linker, "/MERGE:.SCOV=.data")
 
-// Use uint64_t so there won't be any issues if the linker tries to word align
-// the pc array.
 #pragma section(".SCOVP$A", read)  // NOLINT
-__declspec(allocate(".SCOVP$A")) __declspec(align(1)) uint64_t
-    __start___sancov_pcs = 0;
+__declspec(allocate(".SCOVP$A")) uint64_t __start___sancov_pcs = 0;
 #pragma section(".SCOVP$Z", read)  // NOLINT
-__declspec(allocate(".SCOVP$Z")) __declspec(align(1)) uint64_t
+__declspec(allocate(".SCOVP$Z")) __declspec(align(1)) uint8_t
     __stop___sancov_pcs = 0;
 
 #pragma comment(linker, "/MERGE:.SCOVP=.rdata")
 }
 #endif  // SANITIZER_WINDOWS