Skip to content

Commit

Permalink
Avoid reporting crashes for exceptions that hit our SEH from calls to…
Browse files Browse the repository at this point in the history 
… the original implementation of BindToStorage() when we do not wrap the bind status callback.

BUG=42660
TEST=Induce exception in code called under original IMoniker::BindToStorage implementation when we don't wrap the callback and notice that no crash is reported.


Review URL: http://codereview.chromium.org/1748016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46176 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
robertshield@chromium.org committed May 1, 2010
1 parent c96c3c9 commit 7235431
Show file tree
Hide file tree
Showing 8 changed files with 301 additions and 142 deletions.
12 changes: 4 additions & 8 deletions chrome_frame/chrome_frame_reporting.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,6 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const wchar_t* dll_path) {
return &custom_info;
}


void CALLBACK BreakpadHandler(EXCEPTION_POINTERS *ptrs) {
WriteMinidumpForException(ptrs);
}

extern "C" IMAGE_DOS_HEADER __ImageBase;

bool InitializeCrashReporting() {
Expand All @@ -61,9 +56,9 @@ bool InitializeCrashReporting() {
if (!always_take_dump && !GoogleUpdateSettings::GetCollectStatsConsent())
return true;

// Set the handler for ExceptionBarrier for this module:
DCHECK(ExceptionBarrier::handler() == NULL);
ExceptionBarrier::set_handler(BreakpadHandler);
// If we got here, we want to report crashes, so make sure all
// ExceptionBarrierBase instances do so.
ExceptionBarrierConfig::set_enabled(true);

// Get the alternate dump directory. We use the temp path.
FilePath temp_directory;
Expand Down Expand Up @@ -97,5 +92,6 @@ bool InitializeCrashReporting() {
}

bool ShutdownCrashReporting() {
ExceptionBarrierConfig::set_enabled(false);
return ShutdownVectoredCrashReporting();
}
39 changes: 6 additions & 33 deletions chrome_frame/crash_reporting/crash_report.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,36 +47,6 @@ static void veh_segment_end() {}
#pragma code_seg(push, ".text$vm")
#include "chrome_frame/crash_reporting/vectored_handler-impl.h"

// Use Win32 API; use breakpad for dumps; checks for single (current) module.
class CrashHandlerTraits : public Win32VEHTraits,
public ModuleOfInterestWithExcludedRegion {
public:
CrashHandlerTraits() {}

// Note that breakpad_lock must be held when this is called.
void Init(google_breakpad::ExceptionHandler* breakpad, Lock* breakpad_lock) {
DCHECK(breakpad);
DCHECK(breakpad_lock);
breakpad_lock->AssertAcquired();

Win32VEHTraits::InitializeIgnoredBlocks();
ModuleOfInterestWithExcludedRegion::SetCurrentModule();
// Pointers to static (non-extern) functions take the address of the
// function's first byte, as opposed to an entry in the compiler generated
// JMP table. In release builds /OPT:REF wipes away the JMP table, but debug
// builds are not so lucky.
ModuleOfInterestWithExcludedRegion::SetExcludedRegion(&veh_segment_start,
&veh_segment_end);
}

void Shutdown() {
}

inline bool WriteDump(EXCEPTION_POINTERS* p) {
return WriteMinidumpForException(p);
}
};

class CrashHandler {
public:
CrashHandler() : veh_id_(NULL), handler_(&crash_api_) {}
Expand Down Expand Up @@ -115,14 +85,17 @@ bool CrashHandler::Init(google_breakpad::ExceptionHandler* breakpad,
if (veh_id_)
return true;

crash_api_.Init(&veh_segment_start, &veh_segment_end,
&WriteMinidumpForException);

void* id = ::AddVectoredExceptionHandler(FALSE, &VectoredHandlerEntryPoint);
if (id != NULL) {
veh_id_ = id;
crash_api_.Init(breakpad, breakpad_lock);
return true;
} else {
crash_api_.Shutdown();
return false;
}

return false;
}

void CrashHandler::Shutdown() {
Expand Down
42 changes: 42 additions & 0 deletions chrome_frame/crash_reporting/vectored_handler-impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#ifndef CHROME_FRAME_CRASH_REPORTING_VECTORED_HANDLER_IMPL_H_
#define CHROME_FRAME_CRASH_REPORTING_VECTORED_HANDLER_IMPL_H_

#include "base/logging.h"
#include "chrome_frame/crash_reporting/vectored_handler.h"
#include "chrome_frame/crash_reporting/nt_loader.h"

Expand Down Expand Up @@ -262,4 +264,44 @@ Win32VEHTraits::IgnoreExceptions[kIgnoreEntries] = {
{ "kernel32.dll", "IsBadStringPtrW", 0, 100, NULL },
};

// Use Win32 API; checks for single (current) module. Will call a specified
// CrashHandlerTraits::DumpHandler when taking a dump.
class CrashHandlerTraits : public Win32VEHTraits,
public ModuleOfInterestWithExcludedRegion {
public:

typedef bool (*DumpHandler)(EXCEPTION_POINTERS* p);

CrashHandlerTraits() : dump_handler_(NULL) {}

// Note that breakpad_lock must be held when this is called.
void Init(const void* veh_segment_start, const void* veh_segment_end,
DumpHandler dump_handler) {
DCHECK(dump_handler);
dump_handler_ = dump_handler;
Win32VEHTraits::InitializeIgnoredBlocks();
ModuleOfInterestWithExcludedRegion::SetCurrentModule();
// Pointers to static (non-extern) functions take the address of the
// function's first byte, as opposed to an entry in the compiler generated
// JMP table. In release builds /OPT:REF wipes away the JMP table, but debug
// builds are not so lucky.
ModuleOfInterestWithExcludedRegion::SetExcludedRegion(veh_segment_start,
veh_segment_end);
}

void Shutdown() {
}

inline bool WriteDump(EXCEPTION_POINTERS* p) {
if (dump_handler_) {
return dump_handler_(p);
} else {
return false;
}
}

private:
DumpHandler dump_handler_;
};

#endif // CHROME_FRAME_CRASH_REPORTING_VECTORED_HANDLER_IMPL_H_
64 changes: 52 additions & 12 deletions chrome_frame/exception_barrier.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,18 @@
// get crash reports of exceptions that pass over same.
#include "chrome_frame/exception_barrier.h"

#include "chrome_frame/crash_reporting/vectored_handler-impl.h"
#include "chrome_frame/crash_reporting/crash_report.h"

enum {
// Flag set by exception handling machinery when unwinding
EH_UNWINDING = 0x00000002
};

ExceptionBarrier::ExceptionHandler ExceptionBarrier::s_handler_ = NULL;
bool ExceptionBarrierConfig::s_enabled_ = false;

ExceptionBarrierCustomHandler::CustomExceptionHandler
ExceptionBarrierCustomHandler::s_custom_handler_ = NULL;

// This function must be extern "C" to match up with the SAFESEH
// declaration in our corresponding ASM file
Expand All @@ -20,19 +26,53 @@ ExceptionBarrierHandler(struct _EXCEPTION_RECORD* exception_record,
void* establisher_frame,
struct _CONTEXT* context,
void* reserved) {
establisher_frame; // unreferenced formal parameter
reserved;
if (!(exception_record->ExceptionFlags & EH_UNWINDING)) {
// When the exception is really propagating through us, we'd like to be
// called before the state of the program has been modified by the stack
// unwinding. In the absence of an exception handler, the unhandled
// exception filter gets called between the first chance and the second
// chance exceptions, so Windows pops either the JIT debugger or WER UI.
// This is not desirable in most of the cases.
ExceptionBarrier::ExceptionHandler handler = ExceptionBarrier::handler();
// When the exception is really propagating through us, we'd like to be
// called before the state of the program has been modified by the stack
// unwinding. In the absence of an exception handler, the unhandled
// exception filter gets called between the first chance and the second
// chance exceptions, so Windows pops either the JIT debugger or WER UI.
// This is not desirable in most of the cases.
EXCEPTION_POINTERS ptrs = { exception_record, context };

if (ExceptionBarrierConfig::enabled() &&
IS_DISPATCHING(exception_record->ExceptionFlags)) {
WriteMinidumpForException(&ptrs);
}

return ExceptionContinueSearch;
}

extern "C" EXCEPTION_DISPOSITION __cdecl
ExceptionBarrierReportOnlyModuleHandler(
struct _EXCEPTION_RECORD* exception_record,
void* establisher_frame,
struct _CONTEXT* context,
void* reserved) {
EXCEPTION_POINTERS ptrs = { exception_record, context };

if (ExceptionBarrierConfig::enabled() &&
IS_DISPATCHING(exception_record->ExceptionFlags)) {
CrashHandlerTraits traits;
traits.Init(0, 0, &WriteMinidumpForException);
if (traits.IsOurModule(exception_record->ExceptionAddress)) {
traits.WriteDump(&ptrs);
}
}

return ExceptionContinueSearch;
}

extern "C" EXCEPTION_DISPOSITION __cdecl
ExceptionBarrierCallCustomHandler(struct _EXCEPTION_RECORD* exception_record,
void* establisher_frame,
struct _CONTEXT* context,
void* reserved) {
if (ExceptionBarrierConfig::enabled() &&
IS_DISPATCHING(exception_record->ExceptionFlags)) {
ExceptionBarrierCustomHandler::CustomExceptionHandler handler =
ExceptionBarrierCustomHandler::custom_handler();
if (handler) {
EXCEPTION_POINTERS ptrs = { exception_record, context };

handler(&ptrs);
}
}
Expand Down
Loading

0 comments on commit 7235431

Please sign in to comment.