Suppress crash dump generation due to IsBadXXX functions.

IsBad[Read/Write]Ptr etc functions cause access violations
which we catch and take crash dump if we are on stack. Adding
code to suppress these false positives.

BUG=none
TEST=added new tests cases to ChromeFrame.ExceptionReport

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36403 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 127 insertions, 44 deletions

diff --git a/chrome_frame/crash_reporting/crash_report.cc b/chrome_frame/crash_reporting/crash_report.cc
index 43d602d..2a1b88b 100644
--- a/chrome_frame/crash_reporting/crash_report.cc
+++ b/chrome_frame/crash_reporting/crash_report.cc
@@ -7,57 +7,20 @@
 #include "chrome_frame/crash_reporting/crash_report.h"
 
 #include "base/basictypes.h"
-#include "base/logging.h"
 #include "breakpad/src/client/windows/handler/exception_handler.h"
 #include "chrome_frame/crash_reporting/vectored_handler.h"
-#include "chrome_frame/crash_reporting/vectored_handler-impl.h"
 
-namespace {
 // TODO(joshia): factor out common code with chrome used for crash reporting
 const wchar_t kGoogleUpdatePipeName[] = L"\\\\.\\pipe\\GoogleCrashServices\\";
-
 google_breakpad::ExceptionHandler* g_breakpad = NULL;
 
-__declspec(naked)
-static EXCEPTION_REGISTRATION_RECORD* InternalRtlpGetExceptionList() {
-  __asm {
-    mov eax, fs:0
-    ret
-  }
-}
-}  // end of namespace
-
-// Class which methods simply forwards to Win32 API and uses breakpad to write
-// a minidump. Used as template (external interface) of VectoredHandlerT<E>.
-class Win32VEHTraits : public VEHTraitsBase {
- public:
-  static inline void* Register(PVECTORED_EXCEPTION_HANDLER func,
-      const void* module_start, const void* module_end) {
-    VEHTraitsBase::SetModule(module_start, module_end);
-    return ::AddVectoredExceptionHandler(1, func);
-  }
-
-  static inline ULONG Unregister(void* handle) {
-    return ::RemoveVectoredExceptionHandler(handle);
-  }
-
-  static inline bool WriteDump(EXCEPTION_POINTERS* p) {
-    return g_breakpad->WriteMinidumpForException(p);
-  }
-
-  static inline EXCEPTION_REGISTRATION_RECORD* RtlpGetExceptionList() {
-    return InternalRtlpGetExceptionList();
-  }
-
-  static inline WORD RtlCaptureStackBackTrace(DWORD FramesToSkip,
-      DWORD FramesToCapture, void** BackTrace, DWORD* BackTraceHash) {
-    return ::RtlCaptureStackBackTrace(FramesToSkip, FramesToCapture,
-                                      BackTrace, BackTraceHash);
-  }
+Win32VEHTraits::CodeBlock Win32VEHTraits::IgnoreExceptions[kIgnoreEntries] = {
+  { "kernel32.dll", "IsBadReadPtr", 0, 100, NULL },
+  { "kernel32.dll", "IsBadWritePtr", 0, 100, NULL },
+  { "kernel32.dll", "IsBadStringPtrA", 0, 100, NULL },
+  { "kernel32.dll", "IsBadStringPtrW", 0, 100, NULL },
 };
 
-extern "C" IMAGE_DOS_HEADER __ImageBase;
-
 std::wstring GetCrashServerPipeName(const std::wstring& user_sid) {
   std::wstring pipe_name = kGoogleUpdatePipeName;
   pipe_name += user_sid;
diff --git a/chrome_frame/crash_reporting/crash_report.h b/chrome_frame/crash_reporting/crash_report.h
index e98fd05..829dce2 100644
--- a/chrome_frame/crash_reporting/crash_report.h
+++ b/chrome_frame/crash_reporting/crash_report.h
@@ -9,7 +9,12 @@
 
 #include <string>
 
+#include "base/logging.h"
 #include "breakpad/src/client/windows/handler/exception_handler.h"
+#include "chrome_frame/crash_reporting/vectored_handler-impl.h"
+
+extern google_breakpad::ExceptionHandler* g_breakpad;
+extern "C" IMAGE_DOS_HEADER __ImageBase;
 
 bool InitializeVectoredCrashReporting(
     bool full_dump,
@@ -25,4 +30,89 @@ bool InitializeVectoredCrashReportingWithPipeName(
 
 bool ShutdownVectoredCrashReporting();
 
+namespace {
+__declspec(naked)
+static EXCEPTION_REGISTRATION_RECORD* InternalRtlpGetExceptionList() {
+  __asm {
+    mov eax, fs:0
+    ret
+  }
+}
+}  // end of namespace
+
+// Class which methods simply forwards to Win32 API and uses breakpad to write
+// a minidump. Used as template (external interface) of VectoredHandlerT<E>.
+class Win32VEHTraits : public VEHTraitsBase {
+ public:
+  static inline void* Register(PVECTORED_EXCEPTION_HANDLER func,
+      const void* module_start, const void* module_end) {
+    InitializeIgnoredBlocks();
+    VEHTraitsBase::SetModule(module_start, module_end);
+    return ::AddVectoredExceptionHandler(1, func);
+  }
+
+  static inline ULONG Unregister(void* handle) {
+    return ::RemoveVectoredExceptionHandler(handle);
+  }
+
+  static inline bool WriteDump(EXCEPTION_POINTERS* p) {
+    return g_breakpad->WriteMinidumpForException(p);
+  }
+
+  static inline EXCEPTION_REGISTRATION_RECORD* RtlpGetExceptionList() {
+    return InternalRtlpGetExceptionList();
+  }
+
+  static inline WORD RtlCaptureStackBackTrace(DWORD FramesToSkip,
+      DWORD FramesToCapture, void** BackTrace, DWORD* BackTraceHash) {
+    return ::RtlCaptureStackBackTrace(FramesToSkip, FramesToCapture,
+                                      BackTrace, BackTraceHash);
+  }
+
+  static bool ShouldIgnoreException(const EXCEPTION_POINTERS* exceptionInfo) {
+    const void* address = exceptionInfo->ExceptionRecord->ExceptionAddress;
+    for (int i = 0; i < kIgnoreEntries; i++) {
+      const CodeBlock& code_block = IgnoreExceptions[i];
+      if (code_block.code &&
+         (CodeOffset(code_block.code, code_block.begin_offset) <= address) &&
+         (address < CodeOffset(code_block.code, code_block.end_offset))) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  static inline void InitializeIgnoredBlocks() {
+    // Initialize ignored exception list
+    for (int i = 0; i < kIgnoreEntries; i++) {
+      CodeBlock& code_block = IgnoreExceptions[i];
+      if (!code_block.code) {
+        HMODULE module = GetModuleHandleA(code_block.module);
+        DCHECK(module) << "GetModuleHandle error: " << GetLastError();
+        code_block.code = GetProcAddress(module, code_block.function);
+        DCHECK(code_block.code) << "GetProcAddress error: "<< GetLastError();
+      }
+    }
+  }
+
+  static inline const void* CodeOffset(const void* code, int offset) {
+    return reinterpret_cast<const char*>(code) + offset;
+  }
+
+ private:
+  // Block of code to be ignored for exceptions
+  struct CodeBlock {
+    char* module;
+    char* function;
+    int begin_offset;
+    int end_offset;
+    const void* code;
+  };
+
+  static const int kIgnoreEntries = 4;
+  static CodeBlock IgnoreExceptions[kIgnoreEntries];
+};
+
+
 #endif  // CHROME_FRAME_CRASH_REPORTING_CRASH_REPORT_H_
diff --git a/chrome_frame/crash_reporting/crash_reporting.gyp b/chrome_frame/crash_reporting/crash_reporting.gyp
index 7d75184..6cd712c 100644
--- a/chrome_frame/crash_reporting/crash_reporting.gyp
+++ b/chrome_frame/crash_reporting/crash_reporting.gyp
@@ -42,6 +42,7 @@
         '../../testing/gmock.gyp:gmock',
         '../../testing/gtest.gyp:gtest',
         '../../testing/gtest.gyp:gtestmain',
+        '../../breakpad/breakpad.gyp:breakpad_handler',
       ],
     },
   ],
diff --git a/chrome_frame/crash_reporting/vectored_handler-impl.h b/chrome_frame/crash_reporting/vectored_handler-impl.h
index f4ae117..c6d7d8e 100644
--- a/chrome_frame/crash_reporting/vectored_handler-impl.h
+++ b/chrome_frame/crash_reporting/vectored_handler-impl.h
@@ -65,8 +65,11 @@ LONG WINAPI VectoredHandlerT<E>::VectoredHandler(
 
   ++VectoredHandlerT<E>::g_exceptions_seen;
 
-  // TODO(stoyan): Check whether exception address is inbetween
+  // Check whether exception address is inbetween
   // [IsBadReadPtr, IsBadReadPtr + 0xXX]
+  if (E::ShouldIgnoreException(exceptionInfo)) {
+    return ExceptionContinueSearch;
+  }
 
   const DWORD exceptionFlags = exceptionInfo->ExceptionRecord->ExceptionFlags;
   // I don't think VEH is called on unwind. Just to be safe.
diff --git a/chrome_frame/crash_reporting/vectored_handler.h b/chrome_frame/crash_reporting/vectored_handler.h
index f3e08b0..7447bba 100644
--- a/chrome_frame/crash_reporting/vectored_handler.h
+++ b/chrome_frame/crash_reporting/vectored_handler.h
@@ -77,6 +77,10 @@ class VEHTraitsBase {
     g_module_start = module_start;
     g_module_end = module_end;
   }
+
+  static bool ShouldIgnoreException(const EXCEPTION_POINTERS* exceptionInfo) {
+    return false;
+  }
 };
 
 DECLSPEC_SELECTANY const void* VEHTraitsBase::g_module_start;
diff --git a/chrome_frame/crash_reporting/vectored_handler_unittest.cc b/chrome_frame/crash_reporting/vectored_handler_unittest.cc
index 52c730a..0b87f04 100644
--- a/chrome_frame/crash_reporting/vectored_handler_unittest.cc
+++ b/chrome_frame/crash_reporting/vectored_handler_unittest.cc
@@ -6,11 +6,12 @@
 
 #include "base/logging.h"
 #include "chrome_frame/crash_reporting/vectored_handler-impl.h"
+#include "chrome_frame/crash_reporting/crash_report.h"
 #include "gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 // Class that mocks external call from VectoredHandlerT for testing purposes.
-class EMock : public VEHTraitsBase {
+class EMock : public Win32VEHTraits {
  public:
   static inline bool WriteDump(EXCEPTION_POINTERS* p) {
     g_dump_made = true;
@@ -20,6 +21,7 @@ class EMock : public VEHTraitsBase {
   static inline void* Register(PVECTORED_EXCEPTION_HANDLER func,
                                const void* module_start,
                                const void* module_end) {
+    InitializeIgnoredBlocks();
     VEHTraitsBase::SetModule(module_start, module_end);
     // Return some arbitrary number, expecting to get the same on Unregister()
     return reinterpret_cast<void*>(4);
@@ -188,5 +190,25 @@ TEST(ChromeFrame, ExceptionReport) {
   EXPECT_TRUE(EMock::g_dump_made);
   EMock::g_dump_made = false;
 
+  // Exception, in IsBadStringPtrA, we are on the stack.
+  char* ignore_address = reinterpret_cast<char*>(GetProcAddress(
+      GetModuleHandleA("kernel32.dll"), "IsBadStringPtrA")) + 10;
+  ex.Set(STATUS_ACCESS_VIOLATION, ignore_address + 10, 0);
+  EMock::SetNoSEHFilter();
+  EMock::SetOnStack();
+  EXPECT_EQ(ExceptionContinueSearch, VectoredHandlerMock::VectoredHandler(&ex));
+  EXPECT_EQ(5, VectoredHandlerMock::g_exceptions_seen);
+  EXPECT_FALSE(EMock::g_dump_made);
+  EMock::g_dump_made = false;
+
+  // Exception, in IsBadStringPtrA, we are not in stack.
+  ex.Set(STATUS_ACCESS_VIOLATION, ignore_address + 10, 0);
+  EMock::SetNoSEHFilter();
+  EMock::SetNotOnStack();
+  EXPECT_EQ(ExceptionContinueSearch, VectoredHandlerMock::VectoredHandler(&ex));
+  EXPECT_EQ(6, VectoredHandlerMock::g_exceptions_seen);
+  EXPECT_FALSE(EMock::g_dump_made);
+  EMock::g_dump_made = false;
+
   VectoredHandlerMock::Unregister();
 }