[sanitizer][asan][win] Only unmap unneeded shadow memory on x86_64

[alvinhochun]

Summary

D21942 / 1128db8fe1c13800ebc77206efc50d0a219b8750 added support for
committing shadow memory on demand on Win 64-bit. The reason it is not
enabled on 32-bit wasn't clear but the page table overhead on Windows 7
may be a contributing factor.

In AsanMapUnmapCallback::OnUnmap, FlushUnneededASanShadowMemory is
called to release shadow memory. It calls ReleaseMemoryPagesToOS,
which had been a no-op on Windows, until D95892 /
81b1d3da094c54ffd75e05c8d4683792edf17f4c in which it was changed to
unmap full pages that the memory region covers. This was done on both
32-bit and 64-bit.

AddressSanitizerInterface.GetHeapSizeTest appears to fail on i686
targets as a side effect of this. This test allocates and frees a huge
chunk of memory which causes shadow memory to be unmapped immediately.
When the test allocates the chunk of memory a second time, asan tries to
reuse the same shadow memory region, but because the shadow memory has
now been unmapped, it causes an access violation and crashes the test.

x86_64 is not affected, because the code that handles commiting shadow
memory on demand also handles this situation, allowing the test to work
without crashing.

Therefore, this patch changes FlushUnneededASanShadowMemory on Windows
to only release/unmap the shadow memory on x86_64 to stop this from
happening on i686.

[alvinhochun]

I also made a patch D149020 to do the opposite, i.e. enable commit on demand on i686, but it seems to be the worse idea and more risky.

[mstorsjo]

I think this is reasonable. I didn't follow the explanation in the description in detail, but I think it sounds reasonable. This does fix running check-asan-dynamic on both MSVC and MinGW in i386 mode (for MinGW, on top of the other patches that are currently in review). However for MSVC, the GetHeapSizeTest test still fails in check-asan:

Failed Tests (2):
  AddressSanitizer-Unit :: ./Asan-i386-calls-Test.exe/AddressSanitizerInterface/GetHeapSizeTest
  AddressSanitizer-Unit :: ./Asan-i386-inline-Test.exe/AddressSanitizerInterface/GetHeapSizeTest

[alvinhochun]

In D149025#4290881, @mstorsjo wrote:

However for MSVC, the GetHeapSizeTest test still fails in check-asan

I think I know what's going on here -- if you compiled the tests before applying the patch, then applied the patch and reran ninja check-asan, the tests don't actually get relinked.The static asan libs are linked in by -fsanitize=address, so CMake does not have any idea about the dependency on the static asan libs. You have to manually trigger a change, like touch one of the test source, to get the tests to be relinked using the rebuilt static asan.

On my MSVC test setup these tests do pass with this change. (Admittedly I also was confused when reverting the change didn't cause the tests to fail at first.)

[mstorsjo]

In D149025#4291611, @alvinhochun wrote:

In D149025#4290881, @mstorsjo wrote:

However for MSVC, the GetHeapSizeTest test still fails in check-asan

I think I know what's going on here -- if you compiled the tests before applying the patch, then applied the patch and reran ninja check-asan, the tests don't actually get relinked.The static asan libs are linked in by -fsanitize=address, so CMake does not have any idea about the dependency on the static asan libs. You have to manually trigger a change, like touch one of the test source, to get the tests to be relinked using the rebuilt static asan.

On my MSVC test setup these tests do pass with this change. (Admittedly I also was confused when reverting the change didn't cause the tests to fail at first.)

Oh, thanks, you're right! By forcing it to relink those executables, the tests do pass now. Thanks!

[alvinhochun]

Ping, can someone accept this revision?

compiler-rt/lib/asan/asan_win.cpp

 
 void InstallAtExitCheckLeaks() {}
 
 void AsanApplyToGlobals(globals_op_fptr op, const void *needle) {
   UNIMPLEMENTED();
 }
 
 void FlushUnneededASanShadowMemory(uptr p, uptr size) {
+  // Only asan on 64-bit Windows supports committing shadow memory on demand.
+#if SANITIZER_WINDOWS64
   // Since asan's mapping is compacting, the shadow chunk may be
   // not page-aligned, so we only flush the page-aligned portion.
   ReleaseMemoryPagesToOS(MemToShadow(p), MemToShadow(p + size));
+#endif
 }
 
 // ---------------------- TSD ---------------- {{{
 static bool tsd_key_inited = false;
 
 static __declspec(thread) void *fake_tsd = 0;
 
 // https://docs.microsoft.com/en-us/windows/desktop/api/winternl/ns-winternl-_teb