[compiler-rt][interception][asan][win] Improve error reporting

Add a callback from interception to allow asan on Windows to produce
better error messages. If an unrecoverable error occured when
intercepting functions, print a message before terminating.

Additionally, when encountering unknown instructions, a more helpful
message containing the address and the bytes of the unknown instruction
is now printed to help identify the issue and make it easier to propose
a fix.

Depends on D149549

Differential Revision: https://reviews.llvm.org/D149002

compiler-rt/lib/asan/asan_interceptors.cpp

@@ -656,6 +656,7 @@ void InitializeAsanInterceptors() {
   static bool was_called_once;
   CHECK(!was_called_once);
   was_called_once = true;
+  InitializePlatformInterceptors();
   InitializeCommonInterceptors();
   InitializeSignalInterceptors();
 
@@ -741,8 +742,6 @@ void InitializeAsanInterceptors() {
   ASAN_INTERCEPT_FUNC(vfork);
 #endif
 
-  InitializePlatformInterceptors();
-
   VReport(1, "AddressSanitizer: libc interceptors initialized\n");
 }
 

compiler-rt/lib/asan/asan_win.cpp

@@ -159,6 +159,8 @@ INTERCEPTOR_WINAPI(HANDLE, CreateThread, LPSECURITY_ATTRIBUTES security,
 namespace __asan {
 
 void InitializePlatformInterceptors() {
+  __interception::SetErrorReportCallback(Report);
+
   // The interceptors were not designed to be removable, so we have to keep this
   // module alive for the life of the process.
   HMODULE pinned;

compiler-rt/lib/interception/interception_win.cpp

@@ -141,8 +141,27 @@ static const int kBranchLength =
     FIRST_32_SECOND_64(kJumpInstructionLength, kIndirectJumpInstructionLength);
 static const int kDirectBranchLength = kBranchLength + kAddressLength;
 
+#  if defined(_MSC_VER)
+#    define INTERCEPTION_FORMAT(f, a)
+#  else
+#    define INTERCEPTION_FORMAT(f, a) __attribute__((format(printf, f, a)))
+#  endif
+
+static void (*ErrorReportCallback)(const char *format, ...)
+    INTERCEPTION_FORMAT(1, 2);
+
+void SetErrorReportCallback(void (*callback)(const char *format, ...)) {
+  ErrorReportCallback = callback;
+}
+
+#  define ReportError(...)                \
+    do {                                  \
+      if (ErrorReportCallback)            \
+        ErrorReportCallback(__VA_ARGS__); \
+    } while (0)
+
 static void InterceptionFailed() {
-  // Do we have a good way to abort with an error message here?
+  ReportError("interception_win: failed due to an unrecoverable error.\n");
   // This acts like an abort when no debugger is attached. According to an old
   // comment, calling abort() leads to an infinite recursion in CheckFailed.
   __debugbreak();
@@ -657,10 +676,18 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
   }
 #endif
 
-  // Unknown instruction!
-  // FIXME: Unknown instruction failures might happen when we add a new
-  // interceptor or a new compiler version. In either case, they should result
-  // in visible and readable error messages.
+  // Unknown instruction! This might happen when we add a new interceptor, use
+  // a new compiler version, or if Windows changed how some functions are
+  // compiled. In either case, we print the address and 8 bytes of instructions
+  // to notify the user about the error and to help identify the unknown
+  // instruction. Don't treat this as a fatal error, though we can break the
+  // debugger if one has been attached.
+  u8 *bytes = (u8 *)address;
+  ReportError(
+      "interception_win: unhandled instruction at %p: %02x %02x %02x %02x %02x "
+      "%02x %02x %02x\n",
+      (void *)address, bytes[0], bytes[1], bytes[2], bytes[3], bytes[4],
+      bytes[5], bytes[6], bytes[7]);
   if (::IsDebuggerPresent())
     __debugbreak();
   return 0;

compiler-rt/lib/interception/interception_win.h

@@ -41,6 +41,11 @@ bool OverrideImportedFunction(const char *module_to_patch,
                               const char *function_name, uptr new_function,
                               uptr *orig_old_func);
 
+// Sets a callback to be used for reporting errors by interception_win. The
+// callback will be called with printf-like arguments. Intended to be used with
+// __sanitizer::Report. Pass nullptr to disable error reporting (default).
+void SetErrorReportCallback(void (*callback)(const char *format, ...));
+
 #if !SANITIZER_WINDOWS64
 // Exposed for unittests
 bool OverrideFunctionWithDetour(

compiler-rt/lib/interception/tests/interception_win_test.cpp

@@ -18,6 +18,8 @@
 #if !SANITIZER_DEBUG
 #if SANITIZER_WINDOWS
 
+#include <stdarg.h>
+
 #define WIN32_LEAN_AND_MEAN
 #include <windows.h>
 
@@ -728,7 +730,37 @@ TEST(Interception, UnsupportedInstructionWithTrampoline) {
   TestOverrideFunction override = OverrideFunctionWithTrampoline;
   FunctionPrefixKind prefix = FunctionPrefixPadding;
 
+  static bool reportCalled;
+  reportCalled = false;
+
+  struct Local {
+    static void Report(const char *format, ...) {
+      if (reportCalled)
+        FAIL() << "Report called more times than expected";
+      reportCalled = true;
+      ASSERT_STREQ(
+          "interception_win: unhandled instruction at %p: %02x %02x %02x %02x "
+          "%02x %02x %02x %02x\n",
+          format);
+      va_list args;
+      va_start(args, format);
+      u8 *ptr = va_arg(args, u8 *);
+      for (int i = 0; i < 8; i++) EXPECT_EQ(kUnsupportedCode1[i], ptr[i]);
+      int bytes[8];
+      for (int i = 0; i < 8; i++) {
+        bytes[i] = va_arg(args, int);
+        EXPECT_EQ(kUnsupportedCode1[i], bytes[i]);
+      }
+      va_end(args);
+    }
+  };
+
+  SetErrorReportCallback(Local::Report);
   EXPECT_FALSE(TestFunctionPatching(kUnsupportedCode1, override, prefix));
+  SetErrorReportCallback(nullptr);
+
+  if (!reportCalled)
+    ADD_FAILURE() << "Report not called";
 }
 
 TEST(Interception, PatchableFunctionPadding) {