From d58230b9dcb3b312a2da8f874daa0cc8dc27da9b Mon Sep 17 00:00:00 2001 From: Timur Iskhodzhanov Date: Tue, 17 Mar 2015 16:50:59 +0000 Subject: [PATCH] [ASan/Win] Fix a CHECK failure when an exception is thrown from a callback passed to BindIoCompletionCallback This also simplifies how we handle QueueUserWorkItem llvm-svn: 232499 --- compiler-rt/lib/asan/asan_win.cc | 70 ++++++++++------------ compiler-rt/lib/interception/interception_win.cc | 13 ++-- .../Windows/bind_io_completion_callback.cc | 70 ++++++++++++++++++++++ .../Windows/queue_user_work_item_report.cc | 29 +++++++++ 4 files changed, 138 insertions(+), 44 deletions(-) create mode 100644 compiler-rt/test/asan/TestCases/Windows/bind_io_completion_callback.cc create mode 100644 compiler-rt/test/asan/TestCases/Windows/queue_user_work_item_report.cc diff --git a/compiler-rt/lib/asan/asan_win.cc b/compiler-rt/lib/asan/asan_win.cc index d80efaa..addb3d4 100644 --- a/compiler-rt/lib/asan/asan_win.cc +++ b/compiler-rt/lib/asan/asan_win.cc @@ -102,61 +102,51 @@ INTERCEPTOR_WINAPI(DWORD, CreateThread, } namespace { -struct UserWorkItemInfo { - DWORD (__stdcall *function)(void *arg); - void *arg; - u32 parent_tid; -}; - BlockingMutex mu_for_thread_tracking(LINKER_INITIALIZED); -// QueueUserWorkItem may silently create a thread we should keep track of. -// We achieve this by wrapping the user-supplied work items with our function. -DWORD __stdcall QueueUserWorkItemWrapper(void *arg) { - UserWorkItemInfo *item = (UserWorkItemInfo *)arg; - - { - // FIXME: GetCurrentThread relies on TSD, which might not play well with - // system thread pools. We might want to use something like reference - // counting to zero out GetCurrentThread() underlying storage when the last - // work item finishes? Or can we disable reclaiming of threads in the pool? - BlockingMutexLock l(&mu_for_thread_tracking); - AsanThread *t = __asan::GetCurrentThread(); - if (!t) { - GET_STACK_TRACE_THREAD; - t = AsanThread::Create(/* start_routine */ nullptr, /* arg */ nullptr, - item->parent_tid, &stack, /* detached */ true); - t->Init(); - asanThreadRegistry().StartThread(t->tid(), 0, 0); - SetCurrentThread(t); - } - } - - DWORD ret = item->function(item->arg); - delete item; - return ret; +void EnsureWorkerThreadRegistered() { + // FIXME: GetCurrentThread relies on TSD, which might not play well with + // system thread pools. We might want to use something like reference + // counting to zero out GetCurrentThread() underlying storage when the last + // work item finishes? Or can we disable reclaiming of threads in the pool? + BlockingMutexLock l(&mu_for_thread_tracking); + if (__asan::GetCurrentThread()) + return; + + AsanThread *t = AsanThread::Create( + /* start_routine */ nullptr, /* arg */ nullptr, + /* parent_tid */ -1, /* stack */ nullptr, /* detached */ true); + t->Init(); + asanThreadRegistry().StartThread(t->tid(), 0, 0); + SetCurrentThread(t); } } // namespace -INTERCEPTOR_WINAPI(BOOL, QueueUserWorkItem, LPTHREAD_START_ROUTINE function, - PVOID arg, ULONG flags) { - UserWorkItemInfo *work_item_info = new UserWorkItemInfo; - work_item_info->function = function; - work_item_info->arg = arg; - work_item_info->parent_tid = GetCurrentTidOrInvalid(); - return REAL(QueueUserWorkItem)(QueueUserWorkItemWrapper, - work_item_info, flags); +INTERCEPTOR_WINAPI(DWORD, NtWaitForWorkViaWorkerFactory, DWORD a, DWORD b) { + // NtWaitForWorkViaWorkerFactory is called from system worker pool threads to + // query work scheduled by BindIoCompletionCallback, QueueUserWorkItem, etc. + // System worker pool threads are created at arbitraty point in time and + // without using CreateThread, so we wrap NtWaitForWorkViaWorkerFactory + // instead and don't register a specific parent_tid/stack. + EnsureWorkerThreadRegistered(); + return REAL(NtWaitForWorkViaWorkerFactory)(a, b); } + // }}} namespace __asan { void InitializePlatformInterceptors() { ASAN_INTERCEPT_FUNC(CreateThread); - ASAN_INTERCEPT_FUNC(QueueUserWorkItem); ASAN_INTERCEPT_FUNC(RaiseException); ASAN_INTERCEPT_FUNC(_except_handler3); ASAN_INTERCEPT_FUNC(_except_handler4); + + // NtWaitForWorkViaWorkerFactory is always linked dynamically. + CHECK(::__interception::OverrideFunction( + "NtWaitForWorkViaWorkerFactory", + (uptr)WRAP(NtWaitForWorkViaWorkerFactory), + (uptr *)&REAL(NtWaitForWorkViaWorkerFactory))); } // ---------------------- TSD ---------------- {{{ diff --git a/compiler-rt/lib/interception/interception_win.cc b/compiler-rt/lib/interception/interception_win.cc index cd241c3..19cf184 100644 --- a/compiler-rt/lib/interception/interception_win.cc +++ b/compiler-rt/lib/interception/interception_win.cc @@ -84,6 +84,7 @@ static size_t RoundUpToInstrBoundary(size_t size, char *code) { cursor += 2; continue; case '\xE9': // E9 XX YY ZZ WW = jmp WWZZYYXX + case '\xB8': // B8 XX YY ZZ WW = mov eax, WWZZYYXX cursor += 5; continue; } @@ -182,10 +183,14 @@ bool OverrideFunction(uptr old_func, uptr new_func, uptr *orig_old_func) { } static const void **InterestingDLLsAvailable() { - const char *InterestingDLLs[] = {"kernel32.dll", - "msvcr110.dll", // VS2012 - "msvcr120.dll", // VS2013 - NULL}; + const char *InterestingDLLs[] = { + "kernel32.dll", + "msvcr110.dll", // VS2012 + "msvcr120.dll", // VS2013 + // NTDLL should go last as it exports some functions that we should override + // in the CRT [presumably only used internally]. + "ntdll.dll", NULL + }; static void *result[ARRAY_SIZE(InterestingDLLs)] = { 0 }; if (!result[0]) { for (size_t i = 0, j = 0; InterestingDLLs[i]; ++i) { diff --git a/compiler-rt/test/asan/TestCases/Windows/bind_io_completion_callback.cc b/compiler-rt/test/asan/TestCases/Windows/bind_io_completion_callback.cc new file mode 100644 index 0000000..c062a79 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Windows/bind_io_completion_callback.cc @@ -0,0 +1,70 @@ +// Make sure we can throw exceptions from work items executed via +// BindIoCompletionCallback. +// +// Clang doesn't support exceptions on Windows yet, so for the time being we +// build this program in two parts: the code with exceptions is built with CL, +// the rest is built with Clang. This represents the typical scenario when we +// build a large project using "clang-cl -fallback -fsanitize=address". +// +// RUN: cl -c %s -Fo%t.obj +// RUN: %clangxx_asan -o %t.exe %s %t.obj +// RUN: %run %t.exe 2>&1 | FileCheck %s + +#include +#include + +void ThrowAndCatch(); + +#if !defined(__clang__) +__declspec(noinline) +void Throw() { + fprintf(stderr, "Throw\n"); +// CHECK: Throw + throw 1; +} + +void ThrowAndCatch() { + int local; + try { + Throw(); + } catch(...) { + fprintf(stderr, "Catch\n"); +// CHECK: Catch + } +} +#else + +char buffer[65536]; +HANDLE done; +OVERLAPPED ov; + +void CALLBACK completion_callback(DWORD error, DWORD bytesRead, + LPOVERLAPPED pov) { + ThrowAndCatch(); + SetEvent(done); +} + +int main(int argc, char **argv) { + done = CreateEvent(0, false, false, "job is done"); + if (!done) + return 1; + HANDLE file = CreateFile( + argv[0], GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, + NULL); + if (!file) + return 2; + if (!BindIoCompletionCallback(file, completion_callback, 0)) + return 3; + + if (!ReadFile(file, buffer, sizeof(buffer), NULL, &ov) && + GetLastError() != ERROR_IO_PENDING) + return 4; + + if (WAIT_OBJECT_0 != WaitForSingleObject(done, INFINITE)) + return 5; + fprintf(stderr, "Done!\n"); +// CHECK: Done! +} +#endif diff --git a/compiler-rt/test/asan/TestCases/Windows/queue_user_work_item_report.cc b/compiler-rt/test/asan/TestCases/Windows/queue_user_work_item_report.cc new file mode 100644 index 0000000..a57e1e7 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Windows/queue_user_work_item_report.cc @@ -0,0 +1,29 @@ +// RUN: %clang_cl_asan -O0 %s -Fe%t +// RUN: not %run %t 2>&1 | FileCheck %s + +#include + +HANDLE done; + +DWORD CALLBACK work_item(LPVOID) { + int subscript = -1; + volatile char stack_buffer[42]; + stack_buffer[subscript] = 42; +// CHECK: AddressSanitizer: stack-buffer-underflow on address [[ADDR:0x[0-9a-f]+]] +// CHECK: WRITE of size 1 at [[ADDR]] thread T1 +// CHECK: {{#0 .* work_item .*queue_user_work_item_report.cc}}:[[@LINE-3]] +// CHECK: Address [[ADDR]] is located in stack of thread T1 at offset {{.*}} in frame +// CHECK: work_item + SetEvent(done); + return 0; +} + +int main(int argc, char **argv) { + done = CreateEvent(0, false, false, "job is done"); + if (!done) + return 1; +// CHECK-NOT: Thread T1 created + QueueUserWorkItem(&work_item, nullptr, 0); + if (WAIT_OBJECT_0 != WaitForSingleObject(done, INFINITE)) + return 2; +} -- 2.7.4