Improve filtering for false positive crashes...

chrome_frame/crash_reporting/vectored_handler-impl.h

 // Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
@@ skipping 65 matching lines @@
   exceptions_seen_++;
 
   // If the exception code is STATUS_STACK_OVERFLOW then proceed as usual -
   // we want to report it. Otherwise check whether guard page in stack is gone -
   // i.e. stack overflow has been already observed and most probably we are
   // seeing the follow-up STATUS_ACCESS_VIOLATION(s). See bug 32441.
   if (exceptionCode != STATUS_STACK_OVERFLOW && api_->CheckForStackOverflow()) {
     return ExceptionContinueSearch;
   }
 
-  // Check whether exception address is inbetween
-  // [IsBadReadPtr, IsBadReadPtr + 0xXX]
-  if (api_->ShouldIgnoreException(exceptionInfo)) {
+  // Check whether exception will be handled by the module that cuased it.
+  // This should automatically handle the case of IsBadReadPtr and family.
+  const EXCEPTION_REGISTRATION_RECORD* seh = api_->RtlpGetExceptionList();
+  if (api_->ShouldIgnoreException(exceptionInfo, seh)) {
     return ExceptionContinueSearch;
   }
 
   const DWORD exceptionFlags = exceptionInfo->ExceptionRecord->ExceptionFlags;
   // I don't think VEH is called on unwind. Just to be safe.
   if (IS_DISPATCHING(exceptionFlags)) {
     if (ModuleHasInstalledSEHFilter())
       return ExceptionContinueSearch;
 
     if (api_->IsOurModule(exceptionInfo->ExceptionRecord->ExceptionAddress)) {
@@ skipping 63 matching lines @@
   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) {
+  static bool ShouldIgnoreException(const EXCEPTION_POINTERS* exceptionInfo,
+      const EXCEPTION_REGISTRATION_RECORD* seh_record) {
     const void* address = exceptionInfo->ExceptionRecord->ExceptionAddress;
-    for (int i = 0; i < kIgnoreEntries; i++) {
-      const CodeBlock& code_block = IgnoreExceptions[i];
-      DCHECK(code_block.code) << "Win32VEHTraits::CodeBlocks not initialized!";
-      if ((CodeOffset(code_block.code, code_block.begin_offset) <= address) &&
-          (address < CodeOffset(code_block.code, code_block.end_offset))) {
-        return true;
-      }
-    }
+    const DWORD flags = GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT |
+        GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS;
+    HMODULE crashing_module = GetModuleHandleFromAddress(address);
+    if (!crashing_module)
+      return false;
+
+    HMODULE top_seh_module = NULL;
+    if (EXCEPTION_CHAIN_END != seh_record)
+      top_seh_module = GetModuleHandleFromAddress(seh_record->Handler);
+
+    // check if the crash address belongs in a module that's expecting
+    // and handling it. This should address cases like kernel32!IsBadXXX
+    // and urlmon!IsValidInterface
+    if (crashing_module == top_seh_module)
+      return true;
 
     // 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)) {
+    if (nt_loader::OwnsLoaderLock()) {
       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;
   }
 
   static bool CheckForStackOverflow() {
-    MEMORY_BASIC_INFORMATION mi;
+    MEMORY_BASIC_INFORMATION mi = {0};
     const DWORD kPageSize = 0x1000;
     void* stack_top = GetStackTopLimit() - kPageSize;
     ::VirtualQuery(stack_top, &mi, sizeof(mi));
     // The above call may result in moving the top of the stack.
     // Check once more.
     void* stack_top2 = GetStackTopLimit() - kPageSize;
     if (stack_top2 != stack_top)
       ::VirtualQuery(stack_top2, &mi, sizeof(mi));
     return !(mi.Protect & PAGE_GUARD);
   }
 
-  static 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();
-      }
-    }
-  }
-
  private:
   static inline const void* CodeOffset(const void* code, int offset) {
     return reinterpret_cast<const char*>(code) + offset;
   }
 
-  // Block of code to be ignored for exceptions
-  struct CodeBlock {
-    char* module;
-    char* function;
-    int begin_offset;
-    int end_offset;
-    const void* code;
-  };
+  static HMODULE GetModuleHandleFromAddress(const void* p) {
+    HMODULE module_at_address = NULL;
+    MEMORY_BASIC_INFORMATION mi = {0};
+    if (::VirtualQuery(p, &mi, sizeof(mi)) && (mi.Type & MEM_IMAGE)) {
+      module_at_address = reinterpret_cast<HMODULE>(mi.AllocationBase);
+    }
 
-  static const int kIgnoreEntries = 4;
-  static CodeBlock IgnoreExceptions[kIgnoreEntries];
+    return module_at_address;
+  }
 };
 
-DECLSPEC_SELECTANY 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 },
-};
 
 // 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_