From baff938211b0127f6254c3179eb9a58af49ce8d0 Mon Sep 17 00:00:00 2001 From: mmentovai Date: Wed, 7 Feb 2007 20:20:10 +0000 Subject: Airbag windows client didn't trap VC8 parameter validation errors. Now it does. (#120) r=bryner. http://groups.google.com/group/airbag-dev/browse_thread/thread/3f21d0e379e32771 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@120 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/windows/handler/exception_handler.cc | 213 ++++++++++++++++++------ src/client/windows/handler/exception_handler.h | 49 +++++- src/google_airbag/common/minidump_format.h | 24 ++- src/processor/testdata/test_app.cc | 1 + 4 files changed, 233 insertions(+), 54 deletions(-) (limited to 'src') diff --git a/src/client/windows/handler/exception_handler.cc b/src/client/windows/handler/exception_handler.cc index 31127dda..d2220eb6 100644 --- a/src/client/windows/handler/exception_handler.cc +++ b/src/client/windows/handler/exception_handler.cc @@ -36,7 +36,6 @@ #include "client/windows/handler/exception_handler.h" #include "common/windows/guid_string.h" -#include "google_airbag/common/minidump_format.h" namespace google_airbag { @@ -65,12 +64,14 @@ ExceptionHandler::ExceptionHandler(const wstring &dump_path, minidump_write_dump_(NULL), installed_handler_(install_handler), previous_filter_(NULL), + previous_iph_(NULL), handler_thread_(0), handler_critical_section_(), handler_start_semaphore_(NULL), handler_finish_semaphore_(NULL), requesting_thread_id_(0), exception_info_(NULL), + assertion_(NULL), handler_return_value_(false) { // set_dump_path calls UpdateNextID. This sets up all of the path and id // strings, and their equivalent c_str pointers. @@ -116,6 +117,10 @@ ExceptionHandler::ExceptionHandler(const wstring &dump_path, handler_stack_->push_back(this); previous_filter_ = SetUnhandledExceptionFilter(HandleException); +#if _MSC_VER >= 1400 // MSVC 2005/8 + previous_iph_ = _set_invalid_parameter_handler(HandleInvalidParameter); +#endif // _MSC_VER >= 1400 + LeaveCriticalSection(&handler_stack_critical_section_); } } @@ -129,6 +134,11 @@ ExceptionHandler::~ExceptionHandler() { EnterCriticalSection(&handler_stack_critical_section_); SetUnhandledExceptionFilter(previous_filter_); + +#if _MSC_VER >= 1400 // MSVC 2005/8 + _set_invalid_parameter_handler(previous_iph_); +#endif // _MSC_VER >= 1400 + if (handler_stack_->back() == this) { handler_stack_->pop_back(); } else { @@ -172,7 +182,7 @@ DWORD ExceptionHandler::ExceptionHandlerThreadMain(void *lpParameter) { WAIT_OBJECT_0) { // Perform the requested action. self->handler_return_value_ = self->WriteMinidumpWithException( - self->requesting_thread_id_, self->exception_info_); + self->requesting_thread_id_, self->exception_info_, self->assertion_); // Allow the requesting thread to proceed. ReleaseSemaphore(self->handler_finish_semaphore_, 1, NULL); @@ -184,36 +194,75 @@ DWORD ExceptionHandler::ExceptionHandlerThreadMain(void *lpParameter) { return 0; } +// HandleException and HandleInvalidParameter must create an +// AutoExceptionHandler object to maintain static state and to determine which +// ExceptionHandler instance to use. The constructor locates the correct +// instance, and makes it available through get_handler(). The destructor +// restores the state in effect prior to allocating the AutoExceptionHandler. +class AutoExceptionHandler { + public: + AutoExceptionHandler() { + // Increment handler_stack_index_ so that if another Airbag handler is + // registered using this same HandleException function, and it needs to be + // called while this handler is running (either becaause this handler + // declines to handle the exception, or an exception occurs during + // handling), HandleException will find the appropriate ExceptionHandler + // object in handler_stack_ to deliver the exception to. + // + // Because handler_stack_ is addressed in reverse (as |size - index|), + // preincrementing handler_stack_index_ avoids needing to subtract 1 from + // the argument to |at|. + // + // The index is maintained instead of popping elements off of the handler + // stack and pushing them at the end of this method. This avoids ruining + // the order of elements in the stack in the event that some other thread + // decides to manipulate the handler stack (such as creating a new + // ExceptionHandler object) while an exception is being handled. + EnterCriticalSection(&ExceptionHandler::handler_stack_critical_section_); + handler_ = ExceptionHandler::handler_stack_->at( + ExceptionHandler::handler_stack_->size() - + ++ExceptionHandler::handler_stack_index_); + LeaveCriticalSection(&ExceptionHandler::handler_stack_critical_section_); + + // In case another exception occurs while this handler is doing its thing, + // it should be delivered to the previous filter. + SetUnhandledExceptionFilter(handler_->previous_filter_); +#if _MSC_VER >= 1400 // MSVC 2005/8 + _set_invalid_parameter_handler(handler_->previous_iph_); +#endif // _MSC_VER >= 1400 + } + + ~AutoExceptionHandler() { + // Put things back the way they were before entering this handler. + SetUnhandledExceptionFilter(ExceptionHandler::HandleException); +#if _MSC_VER >= 1400 // MSVC 2005/8 + _set_invalid_parameter_handler(ExceptionHandler::HandleInvalidParameter); +#endif // _MSC_VER >= 1400 + + EnterCriticalSection(&ExceptionHandler::handler_stack_critical_section_); + --ExceptionHandler::handler_stack_index_; + LeaveCriticalSection(&ExceptionHandler::handler_stack_critical_section_); + } + + ExceptionHandler *get_handler() const { return handler_; } + + private: + ExceptionHandler *handler_; +}; + // static LONG ExceptionHandler::HandleException(EXCEPTION_POINTERS *exinfo) { - // Increment handler_stack_index_ so that if another Airbag handler is - // registered using this same HandleException function, and it needs to be - // called while this handler is running (either becaause this handler - // declines to handle the exception, or an exception occurs during - // handling), HandleException will find the appropriate ExceptionHandler - // object in handler_stack_ to deliver the exception to. - // - // Because handler_stack_ is addressed in reverse (as |size - index|), - // preincrementing handler_stack_index_ avoids needing to subtract 1 from - // the argument to |at|. - // - // The index is maintained instead of popping elements off of the handler - // stack and pushing them at the end of this method. This avoids ruining - // the order of elements in the stack in the event that some other thread - // decides to manipulate the handler stack (such as creating a new - // ExceptionHandler object) while an exception is being handled. - EnterCriticalSection(&handler_stack_critical_section_); - ExceptionHandler *current_handler = - handler_stack_->at(handler_stack_->size() - ++handler_stack_index_); - LeaveCriticalSection(&handler_stack_critical_section_); - - // In case another exception occurs while this handler is doing its thing, - // it should be delivered to the previous filter. - LPTOP_LEVEL_EXCEPTION_FILTER previous = current_handler->previous_filter_; - LPTOP_LEVEL_EXCEPTION_FILTER restore = SetUnhandledExceptionFilter(previous); - + AutoExceptionHandler auto_exception_handler; + ExceptionHandler *current_handler = auto_exception_handler.get_handler(); + + // Ignore EXCEPTION_BREAKPOINT and EXCEPTION_SINGLE_STEP exceptions. This + // logic will short-circuit before calling WriteMinidumpOnHandlerThread, + // allowing something else to handle the breakpoint without incurring the + // overhead transitioning to and from the handler thread. + DWORD code = exinfo->ExceptionRecord->ExceptionCode; LONG action; - if (current_handler->WriteMinidumpOnHandlerThread(exinfo)) { + if (code != EXCEPTION_BREAKPOINT && code != EXCEPTION_SINGLE_STEP && + current_handler->WriteMinidumpOnHandlerThread(exinfo, NULL)) { // The handler fully handled the exception. Returning // EXCEPTION_EXECUTE_HANDLER indicates this to the system, and usually // results in the applicaiton being terminated. @@ -223,31 +272,88 @@ LONG ExceptionHandler::HandleException(EXCEPTION_POINTERS *exinfo) { // application to be restarted. action = EXCEPTION_EXECUTE_HANDLER; } else { - // There was an exception, but the handler decided not to handle it. + // There was an exception, it was a breakpoint or something else ignored + // above, or it was passed to the handler, which decided not to handle it. // This could be because the filter callback didn't want it, because // minidump writing failed for some reason, or because the post-minidump // callback function indicated failure. Give the previous handler a // chance to do something with the exception. If there is no previous // handler, return EXCEPTION_CONTINUE_SEARCH, which will allow a debugger // or native "crashed" dialog to handle the exception. - action = previous ? previous(exinfo) : EXCEPTION_CONTINUE_SEARCH; + if (current_handler->previous_filter_) { + action = current_handler->previous_filter_(exinfo); + } else { + action = EXCEPTION_CONTINUE_SEARCH; + } } - // Put things back the way they were before entering this handler. - SetUnhandledExceptionFilter(restore); - EnterCriticalSection(&handler_stack_critical_section_); - --handler_stack_index_; - LeaveCriticalSection(&handler_stack_critical_section_); - return action; } -bool ExceptionHandler::WriteMinidumpOnHandlerThread(EXCEPTION_POINTERS *exinfo) { +#if _MSC_VER >= 1400 // MSVC 2005/8 +// static +void ExceptionHandler::HandleInvalidParameter(const wchar_t *expression, + const wchar_t *function, + const wchar_t *file, + unsigned int line, + uintptr_t reserved) { + // This is an invalid parameter, not an exception. It's safe to play with + // sprintf here. + AutoExceptionHandler auto_exception_handler; + ExceptionHandler *current_handler = auto_exception_handler.get_handler(); + + MDRawAssertionInfo assertion; + memset(&assertion, 0, sizeof(assertion)); + _snwprintf_s(reinterpret_cast(assertion.expression), + sizeof(assertion.expression) / sizeof(assertion.expression[0]), + _TRUNCATE, L"%s", expression); + _snwprintf_s(reinterpret_cast(assertion.function), + sizeof(assertion.function) / sizeof(assertion.function[0]), + _TRUNCATE, L"%s", function); + _snwprintf_s(reinterpret_cast(assertion.file), + sizeof(assertion.file) / sizeof(assertion.file[0]), + _TRUNCATE, L"%s", file); + assertion.line = line; + assertion.type = MD_ASSERTION_INFO_TYPE_INVALID_PARAMETER; + + if (!current_handler->WriteMinidumpOnHandlerThread(NULL, &assertion)) { + if (current_handler->previous_iph_) { + // The handler didn't fully handle the exception. Give it to the + // previous invalid parameter handler. + current_handler->previous_iph_(expression, function, file, line, reserved); + } else { + // If there's no previous handler, pass the exception back in to the + // invalid parameter handler's core. That's the routine that called this + // function, but now, since this function is no longer registered (and in + // fact, no function at all is registered), this will result in the + // default code path being taken: _CRT_DEBUGGER_HOOK and _invoke_watson. + // Use _invalid_parameter where it exists (in _DEBUG builds) as it passes + // more information through. In non-debug builds, it is not available, + // so fall back to using _invalid_parameter_noinfo. See invarg.c in the + // CRT source. +#ifdef _DEBUG + _invalid_parameter(expression, function, file, line, reserved); +#else // _DEBUG + _invalid_parameter_noinfo(); +#endif // _DEBUG + } + } + + // The handler either took care of the invalid parameter problem itself, + // or passed it on to another handler. "Swallow" it by exiting, paralleling + // the behavior of "swallowing" exceptions. + exit(0); +} +#endif // _MSC_VER >= 1400 + +bool ExceptionHandler::WriteMinidumpOnHandlerThread( + EXCEPTION_POINTERS *exinfo, MDRawAssertionInfo *assertion) { EnterCriticalSection(&handler_critical_section_); // Set up data to be passed in to the handler thread. requesting_thread_id_ = GetCurrentThreadId(); exception_info_ = exinfo; + assertion_ = assertion; // This causes the handler thread to call WriteMinidumpWithException. ReleaseSemaphore(handler_start_semaphore_, 1, NULL); @@ -259,6 +365,7 @@ bool ExceptionHandler::WriteMinidumpOnHandlerThread(EXCEPTION_POINTERS *exinfo) // Clean up. requesting_thread_id_ = 0; exception_info_ = NULL; + assertion_ = NULL; LeaveCriticalSection(&handler_critical_section_); @@ -266,7 +373,7 @@ bool ExceptionHandler::WriteMinidumpOnHandlerThread(EXCEPTION_POINTERS *exinfo) } bool ExceptionHandler::WriteMinidump() { - bool success = WriteMinidumpOnHandlerThread(NULL); + bool success = WriteMinidumpOnHandlerThread(NULL, NULL); UpdateNextID(); return success; } @@ -279,15 +386,17 @@ bool ExceptionHandler::WriteMinidump(const wstring &dump_path, return handler.WriteMinidump(); } -bool ExceptionHandler::WriteMinidumpWithException(DWORD requesting_thread_id, - EXCEPTION_POINTERS *exinfo) { +bool ExceptionHandler::WriteMinidumpWithException( + DWORD requesting_thread_id, + EXCEPTION_POINTERS *exinfo, + MDRawAssertionInfo *assertion) { // Give user code a chance to approve or prevent writing a minidump. If the // filter returns false, don't handle the exception at all. If this method // was called as a result of an exception, returning false will cause // HandleException to call any previous handler or return // EXCEPTION_CONTINUE_SEARCH on the exception thread, allowing it to appear // as though this handler were not present at all. - if (filter_&& !filter_(callback_context_, exinfo)) { + if (filter_ && !filter_(callback_context_, exinfo, assertion)) { return false; } @@ -318,14 +427,22 @@ bool ExceptionHandler::WriteMinidumpWithException(DWORD requesting_thread_id, airbag_info.dump_thread_id = GetCurrentThreadId(); airbag_info.requesting_thread_id = requesting_thread_id; - MINIDUMP_USER_STREAM airbag_info_stream; - airbag_info_stream.Type = MD_AIRBAG_INFO_STREAM; - airbag_info_stream.BufferSize = sizeof(airbag_info); - airbag_info_stream.Buffer = &airbag_info; + // Leave room in user_stream_array for a possible assertion info stream. + MINIDUMP_USER_STREAM user_stream_array[2]; + user_stream_array[0].Type = MD_AIRBAG_INFO_STREAM; + user_stream_array[0].BufferSize = sizeof(airbag_info); + user_stream_array[0].Buffer = &airbag_info; MINIDUMP_USER_STREAM_INFORMATION user_streams; user_streams.UserStreamCount = 1; - user_streams.UserStreamArray = &airbag_info_stream; + user_streams.UserStreamArray = user_stream_array; + + if (assertion) { + user_stream_array[1].Type = MD_ASSERTION_INFO_STREAM; + user_stream_array[1].BufferSize = sizeof(MDRawAssertionInfo); + user_stream_array[1].Buffer = assertion; + ++user_streams.UserStreamCount; + } // The explicit comparison to TRUE avoids a warning (C4800). success = (minidump_write_dump_(GetCurrentProcess(), @@ -342,7 +459,7 @@ bool ExceptionHandler::WriteMinidumpWithException(DWORD requesting_thread_id, if (callback_) { success = callback_(dump_path_c_, next_minidump_id_c_, callback_context_, - exinfo, success); + exinfo, assertion, success); } return success; diff --git a/src/client/windows/handler/exception_handler.h b/src/client/windows/handler/exception_handler.h index 33091189..fb5ddf50 100644 --- a/src/client/windows/handler/exception_handler.h +++ b/src/client/windows/handler/exception_handler.h @@ -59,6 +59,7 @@ #ifndef CLIENT_WINDOWS_HANDLER_EXCEPTION_HANDLER_H__ #define CLIENT_WINDOWS_HANDLER_EXCEPTION_HANDLER_H__ +#include #include #include @@ -69,6 +70,8 @@ #include #include +#include "google_airbag/common/minidump_format.h" + namespace google_airbag { using std::vector; @@ -80,13 +83,15 @@ class ExceptionHandler { // processing of an exception. A FilterCallback is called before writing // a minidump. context is the parameter supplied by the user as // callback_context when the handler was created. exinfo points to the - // exception record. + // exception record, if any; assertion points to assertion information, + // if any. // // If a FilterCallback returns true, Airbag will continue processing, // attempting to write a minidump. If a FilterCallback returns false, Airbag // will immediately report the exception as unhandled without writing a // minidump, allowing another handler the opportunity to handle it. - typedef bool (*FilterCallback)(void *context, EXCEPTION_POINTERS *exinfo); + typedef bool (*FilterCallback)(void *context, EXCEPTION_POINTERS *exinfo, + MDRawAssertionInfo *assertion); // A callback function to run after the minidump has been written. // minidump_id is a unique id for the dump, so the minidump @@ -94,6 +99,8 @@ class ExceptionHandler { // by the user as callback_context when the handler was created. exinfo // points to the exception record, or NULL if no exception occurred. // succeeded indicates whether a minidump file was successfully written. + // assertion points to information about an assertion if the handler was + // invoked by an assertion. // // If an exception occurred and the callback returns true, Airbag will treat // the exception as fully-handled, suppressing any other handlers from being @@ -109,6 +116,7 @@ class ExceptionHandler { const wchar_t *minidump_id, void *context, EXCEPTION_POINTERS *exinfo, + MDRawAssertionInfo *assertion, bool succeeded); // Creates a new ExceptionHandler instance to handle writing minidumps. @@ -142,6 +150,8 @@ class ExceptionHandler { MinidumpCallback callback, void *callback_context); private: + friend class AutoExceptionHandler; + // Function pointer type for MiniDumpWriteDump, which is looked up // dynamically. typedef BOOL (WINAPI *MiniDumpWriteDump_type)( @@ -160,14 +170,30 @@ class ExceptionHandler { // Signals the exception handler thread to handle the exception. static LONG WINAPI HandleException(EXCEPTION_POINTERS *exinfo); +#if _MSC_VER >= 1400 // MSVC 2005/8 + // This function will be called by some CRT functions when they detect + // that they were passed an invalid parameter. Note that in _DEBUG builds, + // the CRT may display an assertion dialog before calling this function, + // and the function will not be called unless the assertion dialog is + // dismissed by clicking "Ignore." + static void HandleInvalidParameter(const wchar_t *expression, + const wchar_t *function, + const wchar_t *file, + unsigned int line, + uintptr_t reserved); +#endif // _MSC_VER >= 1400 + // This is called on the exception thread or on another thread that // the user wishes to produce a dump from. It calls // WriteMinidumpWithException on the handler thread, avoiding stack // overflows and inconsistent dumps due to writing the dump from // the exception thread. If the dump is requested as a result of an // exception, exinfo contains exception information, otherwise, it - // is NULL. - bool WriteMinidumpOnHandlerThread(EXCEPTION_POINTERS *exinfo); + // is NULL. If the dump is requested as a result of an assertion + // (such as an invalid parameter being passed to a CRT function), + // assertion contains data about the assertion, otherwise, it is NULL. + bool WriteMinidumpOnHandlerThread(EXCEPTION_POINTERS *exinfo, + MDRawAssertionInfo *assertion); // This function does the actual writing of a minidump. It is called // on the handler thread. requesting_thread_id is the ID of the thread @@ -175,7 +201,8 @@ class ExceptionHandler { // an exception, exinfo contains exception information, otherwise, // it is NULL. bool WriteMinidumpWithException(DWORD requesting_thread_id, - EXCEPTION_POINTERS *exinfo); + EXCEPTION_POINTERS *exinfo, + MDRawAssertionInfo *assertion); // Generates a new ID and stores it in next_minidump_id_, and stores the // path of the next minidump to be written in next_minidump_path_. @@ -219,6 +246,14 @@ class ExceptionHandler { // that there is no previous unhandled exception filter. LPTOP_LEVEL_EXCEPTION_FILTER previous_filter_; +#if _MSC_VER >= 1400 // MSVC 2005/8 + // Beginning in VC 8, the CRT provides an invalid parameter handler that will + // be called when some CRT functions are passed invalid parameters. In + // earlier CRTs, the same conditions would cause unexpected behavior or + // crashes. + _invalid_parameter_handler previous_iph_; +#endif // _MSC_VER >= 1400 + // The exception handler thread. HANDLE handler_thread_; @@ -245,6 +280,10 @@ class ExceptionHandler { // thread, if an exception occurred. NULL for user-requested dumps. EXCEPTION_POINTERS *exception_info_; + // If the handler is invoked due to an assertion, this will contain a + // pointer to the assertion information. It is NULL at other times. + MDRawAssertionInfo *assertion_; + // The return value of the handler, passed from the handler thread back to // the requesting thread. bool handler_return_value_; diff --git a/src/google_airbag/common/minidump_format.h b/src/google_airbag/common/minidump_format.h index 535a6626..c211619f 100644 --- a/src/google_airbag/common/minidump_format.h +++ b/src/google_airbag/common/minidump_format.h @@ -494,7 +494,8 @@ typedef enum { MD_LAST_RESERVED_STREAM = 0x0000ffff, /* Airbag extension types. 0x4767 = "Gg" */ - MD_AIRBAG_INFO_STREAM = 0x47670001 /* MDRawAirbagInfo */ + MD_AIRBAG_INFO_STREAM = 0x47670001, /* MDRawAirbagInfo */ + MD_ASSERTION_INFO_STREAM = 0x47670002 /* MDRawAssertionInfo */ } MDStreamType; /* MINIDUMP_STREAM_TYPE */ @@ -1032,6 +1033,27 @@ typedef enum { MD_AIRBAG_INFO_VALID_REQUESTING_THREAD_ID = 1 << 1 } MDAirbagInfoValidity; +typedef struct { + /* expression, function, and file are 0-terminated UTF-16 strings. They + * may be truncated if necessary, but should always be 0-terminated when + * written to a file. + * Fixed-length strings are used because MiniDumpWriteDump doesn't offer + * a way for user streams to point to arbitrary RVAs for strings. */ + u_int16_t expression[128]; /* Assertion that failed... */ + u_int16_t function[128]; /* ...within this function... */ + u_int16_t file[128]; /* ...in this file... */ + u_int32_t line; /* ...at this line. */ + u_int32_t type; +} MDRawAssertionInfo; + +/* For (MDRawAssertionInfo).info: */ +typedef enum { + MD_ASSERTION_INFO_TYPE_UNKNOWN = 0, + + /* Used for assertions that would be raised by the MSVC CRT but are + * directed to an invalid parameter handler instead. */ + MD_ASSERTION_INFO_TYPE_INVALID_PARAMETER +} MDAssertionInfoData; #if defined(_MSC_VER) #pragma warning(pop) diff --git a/src/processor/testdata/test_app.cc b/src/processor/testdata/test_app.cc index 439c2191..d1dbbcce 100644 --- a/src/processor/testdata/test_app.cc +++ b/src/processor/testdata/test_app.cc @@ -41,6 +41,7 @@ namespace { static bool callback(const wchar_t *dump_path, const wchar_t *id, void *context, EXCEPTION_POINTERS *exinfo, + MDRawAssertionInfo *assertion, bool succeeded) { if (succeeded) { printf("dump guid is %ws\n", id); -- cgit v1.2.1