[compiler-rt] 0716888 - [compiler-rt][interception][asan][win] Improve error reporting
Alvin Wong via llvm-commits
llvm-commits at lists.llvm.org
Thu May 4 07:41:45 PDT 2023
Author: Alvin Wong
Date: 2023-05-04T22:41:26+08:00
New Revision: 0716888d828f2e3cca3ac511d05d47c2ca698319
URL: https://github.com/llvm/llvm-project/commit/0716888d828f2e3cca3ac511d05d47c2ca698319
DIFF: https://github.com/llvm/llvm-project/commit/0716888d828f2e3cca3ac511d05d47c2ca698319.diff
LOG: [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
Added:
Modified:
compiler-rt/lib/asan/asan_interceptors.cpp
compiler-rt/lib/asan/asan_win.cpp
compiler-rt/lib/interception/interception_win.cpp
compiler-rt/lib/interception/interception_win.h
compiler-rt/lib/interception/tests/interception_win_test.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp
index 0f1600552481b..9c5a7e4c32884 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/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");
}
diff --git a/compiler-rt/lib/asan/asan_win.cpp b/compiler-rt/lib/asan/asan_win.cpp
index 3c795eb8f713d..25f2e6cd551fe 100644
--- a/compiler-rt/lib/asan/asan_win.cpp
+++ b/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;
diff --git a/compiler-rt/lib/interception/interception_win.cpp b/compiler-rt/lib/interception/interception_win.cpp
index aa413ee3fcb60..7485cd463564b 100644
--- a/compiler-rt/lib/interception/interception_win.cpp
+++ b/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;
diff --git a/compiler-rt/lib/interception/interception_win.h b/compiler-rt/lib/interception/interception_win.h
index 4590013019e37..f6eca82191cba 100644
--- a/compiler-rt/lib/interception/interception_win.h
+++ b/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(
diff --git a/compiler-rt/lib/interception/tests/interception_win_test.cpp b/compiler-rt/lib/interception/tests/interception_win_test.cpp
index 34283dd0dc362..9159ce405f2dc 100644
--- a/compiler-rt/lib/interception/tests/interception_win_test.cpp
+++ b/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) {
More information about the llvm-commits
mailing list