summaryrefslogtreecommitdiffstats
path: root/chrome_frame/crash_reporting
diff options
context:
space:
mode:
authorsiggi@chromium.org <siggi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-12 00:27:58 +0000
committersiggi@chromium.org <siggi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-12 00:27:58 +0000
commitba31e56dc1ef1f29ddd890df9943dffac7853477 (patch)
treed8cc480a25c56064f54b2a11edb47a91fe01d8eb /chrome_frame/crash_reporting
parent98bea6510a3c12ce40ddc2190dadb32d726414bf (diff)
downloadchromium_src-ba31e56dc1ef1f29ddd890df9943dffac7853477.zip
chromium_src-ba31e56dc1ef1f29ddd890df9943dffac7853477.tar.gz
chromium_src-ba31e56dc1ef1f29ddd890df9943dffac7853477.tar.bz2
Heuristically avoid reporting crashes during DLL loading.
Cleanup white space as requested by Robert in CL 882001. BUG=31980 TEST=Unittests in this change. Review URL: http://codereview.chromium.org/872004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41367 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome_frame/crash_reporting')
-rw-r--r--chrome_frame/crash_reporting/crash_dll.h1
-rw-r--r--chrome_frame/crash_reporting/nt_loader.h8
-rw-r--r--chrome_frame/crash_reporting/nt_loader_unittest.cc41
-rw-r--r--chrome_frame/crash_reporting/vectored_handler-impl.h33
-rw-r--r--chrome_frame/crash_reporting/vectored_handler_unittest.cc52
5 files changed, 117 insertions, 18 deletions
diff --git a/chrome_frame/crash_reporting/crash_dll.h b/chrome_frame/crash_reporting/crash_dll.h
index ee4fafd..ca38e15 100644
--- a/chrome_frame/crash_reporting/crash_dll.h
+++ b/chrome_frame/crash_reporting/crash_dll.h
@@ -6,6 +6,7 @@
// Set either of these environment variables to
// crash at load or unload time, respectively.
+static const wchar_t* kCrashDllName = L"crash_dll.dll";
static const wchar_t* kCrashOnLoadMode = L"CRASH_DLL_CRASH_ON_LOAD";
static const wchar_t* kCrashOnUnloadMode = L"CRASH_DLL_CRASH_ON_UNLOAD";
static const DWORD kCrashAddress = 0x42;
diff --git a/chrome_frame/crash_reporting/nt_loader.h b/chrome_frame/crash_reporting/nt_loader.h
index fc4efb1..c751215 100644
--- a/chrome_frame/crash_reporting/nt_loader.h
+++ b/chrome_frame/crash_reporting/nt_loader.h
@@ -128,13 +128,13 @@ typedef struct _LDR_DATA_TABLE_ENTRY {
union {
LIST_ENTRY HashLinks; // 0x03c
struct {
- void* SectionPointer; // 0x03c
- ULONG CheckSum; // 0x040
+ void* SectionPointer; // 0x03c
+ ULONG CheckSum; // 0x040
};
};
union {
- ULONG TimeDateStamp; // 0x044
- void* LoadedImports; // 0x044
+ ULONG TimeDateStamp; // 0x044
+ void* LoadedImports; // 0x044
};
void *EntryPointActivationContext; // 0x048
void* PatchInformation; // 0x04c
diff --git a/chrome_frame/crash_reporting/nt_loader_unittest.cc b/chrome_frame/crash_reporting/nt_loader_unittest.cc
index 67392bd..45546f6 100644
--- a/chrome_frame/crash_reporting/nt_loader_unittest.cc
+++ b/chrome_frame/crash_reporting/nt_loader_unittest.cc
@@ -7,6 +7,7 @@
#include <winnt.h>
#include <base/at_exit.h>
#include <base/scoped_handle.h>
+#include <base/string_util.h>
#include <base/sys_info.h>
#include <base/thread.h>
#include "chrome_frame/crash_reporting/crash_dll.h"
@@ -116,8 +117,6 @@ TEST(NtLoader, OwnsLoaderLock) {
}
TEST(NtLoader, GetLoaderEntry) {
- ScopedEnterCriticalSection lock(GetLoaderLock());
-
// Get all modules in the current process.
ScopedHandle snap(::CreateToolhelp32Snapshot(TH32CS_SNAPMODULE,
::GetCurrentProcessId()));
@@ -128,14 +127,44 @@ TEST(NtLoader, GetLoaderEntry) {
MODULEENTRY32 module = { sizeof(module) };
ASSERT_TRUE(::Module32First(snap.Get(), &module));
do {
+ ScopedEnterCriticalSection lock(GetLoaderLock());
+
nt_loader::LDR_DATA_TABLE_ENTRY* entry =
nt_loader::GetLoaderEntry(module.hModule);
ASSERT_TRUE(entry != NULL);
- ASSERT_EQ(module.hModule, reinterpret_cast<HMODULE>(entry->DllBase));
- ASSERT_STREQ(module.szModule,
+ EXPECT_EQ(module.hModule, reinterpret_cast<HMODULE>(entry->DllBase));
+ EXPECT_STREQ(module.szModule,
FromUnicodeString(entry->BaseDllName).c_str());
- ASSERT_STREQ(module.szExePath,
+ EXPECT_STREQ(module.szExePath,
FromUnicodeString(entry->FullDllName).c_str());
+
+ ULONG flags = entry->Flags;
+
+ // All entries should have this flag set.
+ EXPECT_TRUE(flags & LDRP_ENTRY_PROCESSED);
+
+ if (0 == (flags & LDRP_IMAGE_DLL)) {
+ // TODO(siggi): write a test to assert this holds true for loading
+ // non-DLL, e.g. exe image files.
+ // Dlls have the LDRP_IMAGE_DLL flag set, any module that doesn't
+ // have that flag has to be the main executable.
+ EXPECT_TRUE(module.hModule == ::GetModuleHandle(NULL));
+ } else {
+ // Since we're not currently loading any modules, all loaded
+ // modules should either have the LDRP_PROCESS_ATTACH_CALLED,
+ // or a NULL entrypoint.
+ if (entry->EntryPoint == NULL) {
+ EXPECT_FALSE(flags & LDRP_PROCESS_ATTACH_CALLED);
+ } else {
+ // Shimeng.dll is an exception to the above, it's loaded
+ // in a special way, see e.g. http://www.alex-ionescu.com/?p=41
+ // for details.
+ bool is_shimeng = LowerCaseEqualsASCII(
+ FromUnicodeString(entry->BaseDllName), "shimeng.dll");
+
+ EXPECT_TRUE(is_shimeng || (flags & LDRP_PROCESS_ATTACH_CALLED));
+ }
+ }
} while(::Module32Next(snap.Get(), &module));
}
@@ -199,8 +228,6 @@ class NtLoaderTest: public testing::Test {
NtLoaderTest* NtLoaderTest::current_ = NULL;
DWORD NtLoaderTest::main_thread_ = ::GetCurrentThreadId();
-const wchar_t kCrashDllName[] = L"crash_dll.dll";
-
} // namespace
static int exceptions_handled = 0;
diff --git a/chrome_frame/crash_reporting/vectored_handler-impl.h b/chrome_frame/crash_reporting/vectored_handler-impl.h
index 9c0e97d..641522f 100644
--- a/chrome_frame/crash_reporting/vectored_handler-impl.h
+++ b/chrome_frame/crash_reporting/vectored_handler-impl.h
@@ -5,6 +5,7 @@
#ifndef CHROME_FRAME_CRASH_REPORTING_VECTORED_HANDLER_IMPL_H_
#define CHROME_FRAME_CRASH_REPORTING_VECTORED_HANDLER_IMPL_H_
#include "chrome_frame/crash_reporting/vectored_handler.h"
+#include "chrome_frame/crash_reporting/nt_loader.h"
#if defined(_M_IX86)
#ifndef EXCEPTION_CHAIN_END
@@ -98,7 +99,7 @@ LONG VectoredHandlerT<E>::Handler(EXCEPTION_POINTERS* exceptionInfo) {
}
// See whether our module is somewhere in the call stack.
- void* back_trace[api_->max_back_trace] = {0};
+ void* back_trace[E::max_back_trace] = {0};
DWORD captured = api_->RtlCaptureStackBackTrace(0, api_->max_back_trace,
&back_trace[0], NULL);
for (DWORD i = 0; i < captured; ++i) {
@@ -139,6 +140,7 @@ namespace {
ret
}
}
+
__declspec(naked)
static char* GetStackTopLimit() {
__asm {
@@ -176,6 +178,35 @@ class Win32VEHTraits {
}
}
+ // We don't want to report exceptions that occur during DLL loading,
+ // as those are captured and ignored by the NT loader. If this thread
+ // is holding the loader's lock, there's a possiblity that the crash
+ // is occurring in a loading DLL, in which case we resolve the
+ // crash address to a module and check on the module's status.
+ const DWORD flags = GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT |
+ GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS;
+ HMODULE crashing_module = NULL;
+ if (nt_loader::OwnsLoaderLock() && ::GetModuleHandleEx(
+ flags, reinterpret_cast<LPCWSTR>(address), &crashing_module)) {
+ nt_loader::LDR_DATA_TABLE_ENTRY* entry =
+ nt_loader::GetLoaderEntry(crashing_module);
+
+ // If:
+ // 1. we found the entry in question, and
+ // 2. the entry is a DLL (has the IMAGE_DLL flag set), and
+ // 3. the DLL has a non-null entrypoint, and
+ // 4. the loader has not tagged it with the process attached called flag
+ // we conclude that the crash is most likely during the loading of the
+ // module and bail on reporting the crash to avoid false positives
+ // during crashes that occur during module loads, such as e.g. when
+ // the hook manager attempts to load buggy window hook DLLs.
+ if (entry &&
+ (entry->Flags & LDRP_IMAGE_DLL) != 0 &&
+ (entry->EntryPoint != NULL) &&
+ (entry->Flags & LDRP_PROCESS_ATTACH_CALLED) == 0)
+ return true;
+ }
+
return false;
}
diff --git a/chrome_frame/crash_reporting/vectored_handler_unittest.cc b/chrome_frame/crash_reporting/vectored_handler_unittest.cc
index d7fc4ac..d17847b 100644
--- a/chrome_frame/crash_reporting/vectored_handler_unittest.cc
+++ b/chrome_frame/crash_reporting/vectored_handler_unittest.cc
@@ -5,6 +5,8 @@
#include <atlbase.h>
#include "base/logging.h"
+#include "chrome_frame/crash_reporting/crash_dll.h"
+#include "chrome_frame/crash_reporting/nt_loader.h"
#include "chrome_frame/crash_reporting/vectored_handler-impl.h"
#include "chrome_frame/crash_reporting/veh_test.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -45,8 +47,12 @@ class MockApi : public Win32VEHTraits, public ModuleOfInterest {
};
}; // namespace
-
typedef VectoredHandlerT<MockApi> VectoredHandlerMock;
+static VectoredHandlerMock* g_mock_veh = NULL;
+static LONG WINAPI VEH(EXCEPTION_POINTERS* exptrs) {
+ return g_mock_veh->Handler(exptrs);
+}
+
TEST(ChromeFrame, ExceptionReport) {
MockApi api;
VectoredHandlerMock veh(&api);
@@ -122,6 +128,45 @@ TEST(ChromeFrame, ExceptionReport) {
EXPECT_EQ(ExceptionContinueSearch,
veh.Handler(&ExceptionInfo(STATUS_ACCESS_VIOLATION, ignore_address)));
testing::Mock::VerifyAndClearExpectations(&api);
+
+ // Exception in a loading module, we are on the stack.
+ // Make sure we don't report it.
+ api.SetSEH(no_seh);
+ api.SetStack(on_stack);
+ EXPECT_CALL(api, WriteDump(_)).Times(0);
+
+ g_mock_veh = &veh;
+ void* id = ::AddVectoredExceptionHandler(FALSE, VEH);
+
+ EXPECT_TRUE(::SetEnvironmentVariable(kCrashOnLoadMode, L"1"));
+ long exceptions_seen = veh.get_exceptions_seen();
+ HMODULE module = ::LoadLibrary(kCrashDllName);
+ EXPECT_EQ(NULL, module);
+
+ testing::Mock::VerifyAndClearExpectations(&api);
+ EXPECT_EQ(exceptions_seen + 1, veh.get_exceptions_seen());
+ EXPECT_TRUE(::SetEnvironmentVariable(kCrashOnLoadMode, NULL));
+
+ // Exception in an unloading module, we are on the stack/
+ // Make sure we report it.
+ EXPECT_TRUE(::SetEnvironmentVariable(kCrashOnUnloadMode, L"1"));
+
+ module = ::LoadLibrary(kCrashDllName);
+
+ api.SetSEH(no_seh);
+ api.SetStack(on_stack);
+ EXPECT_CALL(api, WriteDump(_)).Times(1);
+ EXPECT_TRUE(module != NULL);
+ exceptions_seen = veh.get_exceptions_seen();
+
+ // Crash on unloading.
+ ::FreeLibrary(module);
+
+ EXPECT_EQ(exceptions_seen + 1, veh.get_exceptions_seen());
+ testing::Mock::VerifyAndClearExpectations(&api);
+
+ ::RemoveVectoredExceptionHandler(id);
+ g_mock_veh = NULL;
}
MATCHER_P(ExceptionCodeIs, code, "") {
@@ -150,11 +195,6 @@ DWORD WINAPI CrashingThread(PVOID tmp) {
return 0;
}
-static VectoredHandlerMock* g_mock_veh = NULL;
-static LONG WINAPI VEH(EXCEPTION_POINTERS* exptrs) {
- return g_mock_veh->Handler(exptrs);
-}
-
TEST(ChromeFrame, TrickyStackOverflow) {
MockApi api;
VectoredHandlerMock veh(&api);