[sanitizers][Windows] Better debug output and continuable option for interception failures on Windows

[mcgov]

Summary

Currently interception failures just debugbreak, this is confusing to users since the failure can manifest in unexpected ways when a debugger is not attached. When a debugger is attached, there is no good error message explaining what has gone wrong.

This patch fixes this issue and outputs an clear error to the debugger, as well as provides an option for continuing past the interception failure if the user doesn't need the function that has failed to intercept.

[mcgov]

fixed, break should occur if the env variable is not set OR if there was an error finding the variable, so anytime the return code is 0.

[mcgov]

removed whitespace diff

[vitalybuka]

b\compiler-rt\lib\interception\interception_win.cpp

149

can we use sanitizer_common flags?

[mcgov]

b\compiler-rt\lib\interception\interception_win.cpp

149

This is a good point, I was trying to only use things I was sure was initialized in the winapi but using an asan_option is cleaner.

One hiccup, adding this header to interception_win... There's something additional needed for the common_flag symbol to not be undefined in asan_dll_thunk. Do I need to add something to resolve this at runtime in that thunk library? Any tips here are appreciated

[vitalybuka]

b\compiler-rt\lib\interception\interception_win.cpp

149

Not sure how this works on windows, but in general we init interceptors after common_flags()

[vitalybuka]

b\compiler-rt\lib\interception\interception_win.cpp

149

This stuff is common for all sanitizers so ASAN_ is not good prefix.

149

I think you can add

static bool break_on_interception_failure;
void __interception::SetDebugBreakOnIntercepionFailure(bool v) {
   break_on_interception_failure = v;
}

And then in asan,msan... get it from flags() and forward it here

164

On linux we print and move forward. Usually failed interceptor is no a problem as program may not even call it.
Why it needs to be DebugBreake in a first place?

[mcgov]

b\compiler-rt\lib\interception\interception_win.cpp

149

I think the issue isn't the init order, just adding the common_flags symbol to a library that previously didn't use it.

149

This is great, thank you! I'll give it a shot.

164

It doesn't really need to be one and I like the idea of just printing something and moving forward. The current failure mode is to crash, which prompts the user to investigate in a debugger to get this output spew. Just printing out a warning would be better, I'll look into it before testing that other change above.

[rnk]

I'm in favor of the direction of the change, too, fwiw.

b\compiler-rt\lib\interception\interception_win.cpp

 
 #if SANITIZER_WINDOWS
 #include "sanitizer_common/sanitizer_platform.h"
 #define WIN32_LEAN_AND_MEAN
 #include <windows.h>
 
 namespace __interception {
 static const int kAddressLength = FIRST_32_SECOND_64(4, 8);
 static const int kJumpInstructionLength = 5;
 static const int kShortJumpInstructionLength = 2;
 static const int kIndirectJumpInstructionLength = 6;
 static const int kBranchLength =
     FIRST_32_SECOND_64(kJumpInstructionLength, kIndirectJumpInstructionLength);
 static const int kDirectBranchLength = kBranchLength + kAddressLength;
 
 static void InterceptionFailed() {
-  // Do we have a good way to abort with an error message here?
-  __debugbreak();
+  /* If the user wants to continue past an interception failure, they can set
+   * this variable. If not, these failures should be fatal, prompting them to
+   * open a debugger. */
+  char dummy_buffer[1] = {0};
+  bool break_on_interception_failure =
+      GetEnvironmentVariableA("ASAN_WIN_CONTINUE_ON_INTERCEPTION_FAILURE",
+                              dummy_buffer, 0) == 0;
+  bool debugger_is_present = IsDebuggerPresent();
+  if (debugger_is_present) {
+    OutputDebugStringA(
+        "====================================================================\n"
+        "Warning!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"
+        "AddressSanitizer (ASan) interception failed for a targeted function.\n"
+        "Please file a bug report which includes your Windows build number.\n"
+        "This is listed as 'OS Version' in the command output from \n"
+        "systeminfo.\n"
+        "====================================================================\n"
+        );
+  }
+  if (break_on_interception_failure || debugger_is_present) {
+    DebugBreak();
+  }
 }
 
 static bool DistanceIsWithin2Gig(uptr from, uptr target) {
 #if SANITIZER_WINDOWS64
   if (from < target)
     return target - from <= (uptr)0x7FFFFFFFU;
   else
     return from - target <= (uptr)0x80000000U;
 #else
   // In a 32-bit address space, the address calculation will wrap, so this check
   // is unnecessary.
   return true;
 #endif
 }
 
 static uptr GetMmapGranularity() {