3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2024-12-27 15:32:10 +08:00
Code Issues Projects Releases Wiki Activity

win: Improve child crash location test

Browse Source 
In setting up the gn build, slightly different optimization settings
were applied for release builds. This caused a couple things to happen,
1) the sketchy noinline declspec was ignored, and 2) the distance
between reading the IP and the actual crash exceeded the tolerance of 64
bytes in the parent.

To make the test more robust to this, use CaptureContext() (I think our
improved version didn't exist at the time the tests was originally
written). Also, switch from crashpad::CheckedWriteFile to Windows'
WriteFile(), which avoids inlining a whole lot of code at that point.
The return value is not checked, but the next thing that happens is that
the function crashes unconditionally, so this does not seem like a huge
problem.

Bug: crashpad:79
Change-Id: I8193d8ce8b01e1533c16b207813c36d6d6113d89
Reviewed-on: https://chromium-review.googlesource.com/902693
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This commit is contained in:
Scott Graham 2018-02-06 13:21:45 -08:00 committed by Commit Bot
parent 36679d572b
commit 9ab4fbf1e1
4 changed files with 49 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
client/simulate_crash_win.h
View File

@ -23,11 +23,12 @@
//! \file
//! \brief Captures the CPU context and captures a dump without an exception.
#define CRASHPAD_SIMULATE_CRASH() \
do { \
CONTEXT context; \
crashpad::CaptureContext(&context); \
crashpad::CrashpadClient::DumpWithoutCrash(context); \
#define CRASHPAD_SIMULATE_CRASH() \
do { \
/* Not "context" to avoid variable shadowing warnings. */ \
CONTEXT simulate_crash_cpu_context; \
crashpad::CaptureContext(&simulate_crash_cpu_context); \
crashpad::CrashpadClient::DumpWithoutCrash(simulate_crash_cpu_context); \
} while (false)
#endif // CRASHPAD_CLIENT_SIMULATE_CRASH_WIN_H_

34
snapshot/win/crashpad_snapshot_test_crashing_child.cc
View File

@ -15,20 +15,11 @@
#include <intrin.h>
#include <windows.h>
#include "base/files/file_path.h"
#include "base/logging.h"
#include "build/build_config.h"
#include "client/crashpad_client.h"
#include "util/file/file_io.h"
#include "util/misc/from_pointer_cast.h"
#include "util/win/address_types.h"
namespace {
__declspec(noinline) crashpad::WinVMAddress CurrentAddress() {
return crashpad::FromPointerCast<crashpad::WinVMAddress>(_ReturnAddress());
}
} // namespace
#include "util/win/capture_context.h"
int wmain(int argc, wchar_t* argv[]) {
CHECK_EQ(argc, 2);
@ -38,8 +29,25 @@ int wmain(int argc, wchar_t* argv[]) {
HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE);
PCHECK(out != INVALID_HANDLE_VALUE) << "GetStdHandle";
crashpad::WinVMAddress break_address = CurrentAddress();
crashpad::CheckedWriteFile(out, &break_address, sizeof(break_address));
CONTEXT context;
crashpad::CaptureContext(&context);
#if defined(ARCH_CPU_64_BITS)
crashpad::WinVMAddress break_address = context.Rip;
#else
crashpad::WinVMAddress break_address = context.Eip;
#endif
// This does not used CheckedWriteFile() because at high optimization
// settings, a lot of logging code can be inlined, causing there to be a large
// number of instructions between where the IP is captured and the actual
// __debugbreak(). Instead call Windows' WriteFile() to minimize the amount of
// code here. Because the next line is going to crash in any case, there's
// minimal difference in behavior aside from an indication of what broke when
// the other end experiences a ReadFile() error.
DWORD bytes_written;
WriteFile(
out, &break_address, sizeof(break_address), &bytes_written, nullptr);
__debugbreak();

32
snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc
View File

@ -18,17 +18,8 @@
#include "base/logging.h"
#include "client/crashpad_client.h"
#include "client/simulate_crash.h"
#include "util/file/file_io.h"
#include "util/misc/from_pointer_cast.h"
#include "util/win/address_types.h"
namespace {
__declspec(noinline) crashpad::WinVMAddress CurrentAddress() {
return crashpad::FromPointerCast<crashpad::WinVMAddress>(_ReturnAddress());
}
} // namespace
#include "util/win/capture_context.h"
int wmain(int argc, wchar_t* argv[]) {
CHECK_EQ(argc, 2);
@ -38,8 +29,25 @@ int wmain(int argc, wchar_t* argv[]) {
HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE);
PCHECK(out != INVALID_HANDLE_VALUE) << "GetStdHandle";
crashpad::WinVMAddress current_address = CurrentAddress();
crashpad::CheckedWriteFile(out, &current_address, sizeof(current_address));
CONTEXT context;
crashpad::CaptureContext(&context);
#if defined(ARCH_CPU_64_BITS)
crashpad::WinVMAddress break_address = context.Rip;
#else
crashpad::WinVMAddress break_address = context.Eip;
#endif
// This does not used CheckedWriteFile() because at high optimization
// settings, a lot of logging code can be inlined, causing there to be a large
// number of instructions between where the IP is captured and the actual
// __debugbreak(). Instead call Windows' WriteFile() to minimize the amount of
// code here. Because the next line is going to crash in any case, there's
// minimal difference in behavior aside from an indication of what broke when
// the other end experiences a ReadFile() error.
DWORD bytes_written;
WriteFile(
out, &break_address, sizeof(break_address), &bytes_written, nullptr);
CRASHPAD_SIMULATE_CRASH();

4
snapshot/win/exception_snapshot_win_test.cc
View File

@ -103,7 +103,7 @@ class CrashingDelegate : public ExceptionHandlerServer::Delegate {
// Verify the exception happened at the expected location with a bit of
// slop space to allow for reading the current PC before the exception
// happens. See TestCrashingChild().
constexpr uint64_t kAllowedOffset = 64;
constexpr uint64_t kAllowedOffset = 100;
EXPECT_GT(snapshot.Exception()->ExceptionAddress(), break_near_);
EXPECT_LT(snapshot.Exception()->ExceptionAddress(),
break_near_ + kAllowedOffset);
@ -204,7 +204,7 @@ class SimulateDelegate : public ExceptionHandlerServer::Delegate {
// Verify the dump was captured at the expected location with some slop
// space.
constexpr uint64_t kAllowedOffset = 64;
constexpr uint64_t kAllowedOffset = 100;
EXPECT_GT(snapshot.Exception()->Context()->InstructionPointer(),
dump_near_);
EXPECT_LT(snapshot.Exception()->Context()->InstructionPointer(),