From 1f82c6cc8a35e80ee26c265c506e2334f2da2005 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 8 Feb 2017 22:03:54 -0500 Subject: [PATCH 01/10] Ensure Content-Length does not appear with Transfer-Encoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the HTTPTransport test, verify the requirement of RFC 7230 §3.3.2 that Content-Length not appear if Transfer-Encoding is present. TEST=crashpad_util_test HTTPTransport.* BUG=crashpad:159 Change-Id: I51eafff9659443e1d9bb67d1213c8cecc757ded6 Reviewed-on: https://chromium-review.googlesource.com/439984 Reviewed-by: Robert Sesek --- util/net/http_transport_test_server.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/util/net/http_transport_test_server.py b/util/net/http_transport_test_server.py index 3d3c55a4..e79a428e 100755 --- a/util/net/http_transport_test_server.py +++ b/util/net/http_transport_test_server.py @@ -81,6 +81,8 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): self.rfile.buffer = '' if self.headers.get('Transfer-Encoding', '').lower() == 'chunked': + if 'Content-Length' in self.headers: + raise AssertionError body = self.handle_chunked_encoding() else: length = int(self.headers.get('Content-Length', -1)) From c1b305244aba9c8ea3265cadba9ecaca0dbb9461 Mon Sep 17 00:00:00 2001 From: Erik Chen Date: Fri, 10 Feb 2017 09:43:39 -0800 Subject: [PATCH 02/10] Update mig.py to take an explicit sdk argument. BUG=chromium:690734 > Review-Url: https://codereview.chromium.org/2685233002 > Cr-Commit-Position: refs/heads/master@{#449550} > Message-Id: Merged from chromium 53f2146935506b4f382705b605dffec41b5519eb Change-Id: I1b3176a4a62078f1e27184ad589c9c3f4b548674 Reviewed-on: https://chromium-review.googlesource.com/440847 Reviewed-by: Mark Mentovai --- util/mach/mig.py | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/util/mach/mig.py b/util/mach/mig.py index eca2348a..a41c4f89 100755 --- a/util/mach/mig.py +++ b/util/mach/mig.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import argparse import os import re import subprocess @@ -107,23 +108,32 @@ extern "C" { file.close() def main(args): - if len(args) == 5: - (defs_file, user_c, server_c, user_h, server_h) = args - elif len(args) == 6: - (defs_file, user_c, server_c, user_h, server_h, dev_dir) = args - os.environ['DEVELOPER_DIR'] = dev_dir - else: - assert False, "Wrong number of arguments" - subprocess.check_call(['mig', - '-user', user_c, - '-server', server_c, - '-header', user_h, - '-sheader', server_h, - defs_file]) - FixUserImplementation(user_c) - server_declarations = FixServerImplementation(server_c) - FixHeader(user_h) - FixHeader(server_h, server_declarations) + parser = argparse.ArgumentParser() + parser.add_argument('--developer-dir', help='Path to Xcode') + parser.add_argument('--sdk', help='Path to SDK') + parser.add_argument('defs') + parser.add_argument('user_c') + parser.add_argument('server_c') + parser.add_argument('user_h') + parser.add_argument('server_h') + parsed = parser.parse_args(args) + + command = ['mig', + '-user', parsed.user_c, + '-server', parsed.server_c, + '-header', parsed.user_h, + '-sheader', parsed.server_h, + ] + if parsed.sdk is not None: + command.extend(['-isysroot', parsed.sdk]) + if parsed.developer_dir is not None: + os.environ['DEVELOPER_DIR'] = parsed.developer_dir + command.append(parsed.defs) + subprocess.check_call(command) + FixUserImplementation(parsed.user_c) + server_declarations = FixServerImplementation(parsed.server_c) + FixHeader(parsed.user_h) + FixHeader(parsed.server_h, server_declarations) if __name__ == '__main__': sys.exit(main(sys.argv[1:])) From d98a4de718d9d03fdd270dc7a6add8596894ed40 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Tue, 14 Feb 2017 16:27:22 -0500 Subject: [PATCH 03/10] win: support native x86 CONTEXT structures with x87 but no SSE context When no SSE (fxsave) context is available but x87 (fsave) context is, use the x87 context. This also embeds the x87 FPU opcode from the fxsave fop field in bits 16-26 of the fsave error_selector field, true to the layout of the fsave structure. See Intel SDM volume 1 (253665-061) 8.1.10 and figure 8-9. BUG=crashpad:161 TEST=crashpad_snapshot_test CPUContextX86.*:CPUContextWin.* Change-Id: I0bf7ed995c152f124166eaa20104d228d3468f76 Reviewed-on: https://chromium-review.googlesource.com/442144 Reviewed-by: Scott Graham --- minidump/minidump_context_writer.cc | 12 ++- minidump/test/minidump_context_test_util.cc | 3 +- snapshot/cpu_context.cc | 29 +++-- snapshot/cpu_context.h | 12 +++ snapshot/cpu_context_test.cc | 10 ++ snapshot/win/cpu_context_win.cc | 77 +++++++++++--- snapshot/win/cpu_context_win_test.cc | 112 ++++++++++++++++---- 7 files changed, 208 insertions(+), 47 deletions(-) diff --git a/minidump/minidump_context_writer.cc b/minidump/minidump_context_writer.cc index 4d3bf8a4..cc18c34a 100644 --- a/minidump/minidump_context_writer.cc +++ b/minidump/minidump_context_writer.cc @@ -100,14 +100,18 @@ void MinidumpContextX86Writer::InitializeFromSnapshot( context_snapshot->fxsave.ftw, context_snapshot->fxsave.st_mm); context_.float_save.error_offset = context_snapshot->fxsave.fpu_ip; - context_.float_save.error_selector = context_snapshot->fxsave.fpu_cs; + context_.float_save.error_selector = + (context_snapshot->fxsave.fop << 16) | context_snapshot->fxsave.fpu_cs; context_.float_save.data_offset = context_snapshot->fxsave.fpu_dp; context_.float_save.data_selector = context_snapshot->fxsave.fpu_ds; - for (size_t index = 0, offset = 0; + CPUContextX86::X87Register* context_float_save_st = + reinterpret_cast( + context_.float_save.register_area); + for (size_t index = 0; index < arraysize(context_snapshot->fxsave.st_mm); - offset += sizeof(context_snapshot->fxsave.st_mm[index].st), ++index) { - memcpy(&context_.float_save.register_area[offset], + ++index) { + memcpy(&context_float_save_st[index], &context_snapshot->fxsave.st_mm[index].st, sizeof(context_snapshot->fxsave.st_mm[index].st)); } diff --git a/minidump/test/minidump_context_test_util.cc b/minidump/test/minidump_context_test_util.cc index 730c2b79..a4d1d26d 100644 --- a/minidump/test/minidump_context_test_util.cc +++ b/minidump/test/minidump_context_test_util.cc @@ -73,7 +73,8 @@ void InitializeMinidumpContextX86(MinidumpContextX86* context, uint32_t seed) { context->float_save.tag_word = CPUContextX86::FxsaveToFsaveTagWord( context->fxsave.fsw, context->fxsave.ftw, context->fxsave.st_mm); context->float_save.error_offset = context->fxsave.fpu_ip; - context->float_save.error_selector = context->fxsave.fpu_cs; + context->float_save.error_selector = + (context->fxsave.fop << 16) | context->fxsave.fpu_cs; context->float_save.data_offset = context->fxsave.fpu_dp; context->float_save.data_selector = context->fxsave.fpu_ds; for (size_t st_mm_index = 0; diff --git a/snapshot/cpu_context.cc b/snapshot/cpu_context.cc index c11a031b..c846fd1f 100644 --- a/snapshot/cpu_context.cc +++ b/snapshot/cpu_context.cc @@ -19,18 +19,22 @@ namespace crashpad { +namespace { + +enum { + kX87TagValid = 0, + kX87TagZero, + kX87TagSpecial, + kX87TagEmpty, +}; + +} // namespace + // static uint16_t CPUContextX86::FxsaveToFsaveTagWord( uint16_t fsw, uint8_t fxsave_tag, const CPUContextX86::X87OrMMXRegister st_mm[8]) { - enum { - kX87TagValid = 0, - kX87TagZero, - kX87TagSpecial, - kX87TagEmpty, - }; - // The x87 tag word (in both abridged and full form) identifies physical // registers, but |st_mm| is arranged in logical stack order. In order to map // physical tag word bits to the logical stack registers they correspond to, @@ -85,6 +89,17 @@ uint16_t CPUContextX86::FxsaveToFsaveTagWord( return fsave_tag; } +// static +uint8_t CPUContextX86::FsaveToFxsaveTagWord(uint16_t fsave_tag) { + uint8_t fxsave_tag = 0; + for (int physical_index = 0; physical_index < 8; ++physical_index) { + const uint8_t fsave_bits = (fsave_tag >> (physical_index * 2)) & 0x3; + const bool fxsave_bit = fsave_bits != kX87TagEmpty; + fxsave_tag |= fxsave_bit << physical_index; + } + return fxsave_tag; +} + uint64_t CPUContext::InstructionPointer() const { switch (architecture) { case kCPUArchitectureX86: diff --git a/snapshot/cpu_context.h b/snapshot/cpu_context.h index 67b298e8..e1989141 100644 --- a/snapshot/cpu_context.h +++ b/snapshot/cpu_context.h @@ -75,6 +75,8 @@ struct CPUContextX86 { //! Manual, Volume 2: System Programming (24593-3.24), “FXSAVE Format for x87 //! Tag Word”. //! + //! \sa FsaveToFxsaveTagWord() + //! //! \param[in] fsw The FPU status word, used to map logical \a st_mm registers //! to their physical counterparts. This can be taken from //! CPUContextX86::Fxsave::fsw. @@ -87,6 +89,16 @@ struct CPUContextX86 { static uint16_t FxsaveToFsaveTagWord( uint16_t fsw, uint8_t fxsave_tag, const X87OrMMXRegister st_mm[8]); + //! \breif Converts x87 floating-point tag words from `fsave` (full, 16-bit) + //! to `fxsave` (abridged, 8-bit) form. + //! + //! This function performs the inverse operation of FxsaveToFsaveTagWord(). + //! + //! \param[in] fsave_tag The full FPU tag word. + //! + //! \return The abridged FPU tag word. + static uint8_t FsaveToFxsaveTagWord(uint16_t fsave_tag); + // Integer registers. uint32_t eax; uint32_t ebx; diff --git a/snapshot/cpu_context_test.cc b/snapshot/cpu_context_test.cc index 808ba615..042b14e7 100644 --- a/snapshot/cpu_context_test.cc +++ b/snapshot/cpu_context_test.cc @@ -161,6 +161,16 @@ TEST(CPUContextX86, FxsaveToFsaveTagWord) { CPUContextX86::FxsaveToFsaveTagWord(fsw, fxsave_tag, st_mm)); } +TEST(CPUContextX86, FsaveToFxsaveTagWord) { + // The register sets that these x87 tag words might apply to are given in the + // FxsaveToFsaveTagWord test above. + EXPECT_EQ(0x0f, CPUContextX86::FsaveToFxsaveTagWord(0xff22)); + EXPECT_EQ(0xf0, CPUContextX86::FsaveToFxsaveTagWord(0xa9ff)); + EXPECT_EQ(0x5a, CPUContextX86::FsaveToFxsaveTagWord(0xeebb)); + EXPECT_EQ(0x1f, CPUContextX86::FsaveToFxsaveTagWord(0xfe90)); + EXPECT_EQ(0x00, CPUContextX86::FsaveToFxsaveTagWord(0xffff)); +} + } // namespace } // namespace test } // namespace crashpad diff --git a/snapshot/win/cpu_context_win.cc b/snapshot/win/cpu_context_win.cc index 2450ef25..900fbda8 100644 --- a/snapshot/win/cpu_context_win.cc +++ b/snapshot/win/cpu_context_win.cc @@ -24,16 +24,43 @@ namespace crashpad { namespace { +template +bool HasContextPart(const T& context, uint32_t bits) { + return (context.ContextFlags & bits) == bits; +} + template void CommonInitializeX86Context(const T& context, CPUContextX86* out) { - LOG_IF(ERROR, !(context.ContextFlags & WOW64_CONTEXT_i386)) - << "non-x86 context"; + // This function assumes that the WOW64_CONTEXT_* and x86 CONTEXT_* values + // for ContextFlags are identical. This can be tested when targeting 32-bit + // x86. +#if defined(ARCH_CPU_X86) + static_assert(sizeof(CONTEXT) == sizeof(WOW64_CONTEXT), + "type mismatch: CONTEXT"); +#define ASSERT_WOW64_EQUIVALENT(x) \ + do { \ + static_assert(x == WOW64_##x, "value mismatch: " #x); \ + } while (false) + ASSERT_WOW64_EQUIVALENT(CONTEXT_i386); + ASSERT_WOW64_EQUIVALENT(CONTEXT_i486); + ASSERT_WOW64_EQUIVALENT(CONTEXT_CONTROL); + ASSERT_WOW64_EQUIVALENT(CONTEXT_INTEGER); + ASSERT_WOW64_EQUIVALENT(CONTEXT_SEGMENTS); + ASSERT_WOW64_EQUIVALENT(CONTEXT_FLOATING_POINT); + ASSERT_WOW64_EQUIVALENT(CONTEXT_DEBUG_REGISTERS); + ASSERT_WOW64_EQUIVALENT(CONTEXT_EXTENDED_REGISTERS); + ASSERT_WOW64_EQUIVALENT(CONTEXT_FULL); + ASSERT_WOW64_EQUIVALENT(CONTEXT_ALL); + ASSERT_WOW64_EQUIVALENT(CONTEXT_XSTATE); +#undef ASSERT_WOW64_EQUIVALENT +#endif // ARCH_CPU_X86 + memset(out, 0, sizeof(*out)); - // We assume in this function that the WOW64_CONTEXT_* and x86 CONTEXT_* - // values for ContextFlags are identical. + LOG_IF(ERROR, !HasContextPart(context, WOW64_CONTEXT_i386)) + << "non-x86 context"; - if (context.ContextFlags & WOW64_CONTEXT_CONTROL) { + if (HasContextPart(context, WOW64_CONTEXT_CONTROL)) { out->ebp = context.Ebp; out->eip = context.Eip; out->cs = static_cast(context.SegCs); @@ -42,7 +69,7 @@ void CommonInitializeX86Context(const T& context, CPUContextX86* out) { out->ss = static_cast(context.SegSs); } - if (context.ContextFlags & WOW64_CONTEXT_INTEGER) { + if (HasContextPart(context, WOW64_CONTEXT_INTEGER)) { out->eax = context.Eax; out->ebx = context.Ebx; out->ecx = context.Ecx; @@ -51,14 +78,14 @@ void CommonInitializeX86Context(const T& context, CPUContextX86* out) { out->esi = context.Esi; } - if (context.ContextFlags & WOW64_CONTEXT_SEGMENTS) { + if (HasContextPart(context, WOW64_CONTEXT_SEGMENTS)) { out->ds = static_cast(context.SegDs); out->es = static_cast(context.SegEs); out->fs = static_cast(context.SegFs); out->gs = static_cast(context.SegGs); } - if (context.ContextFlags & WOW64_CONTEXT_DEBUG_REGISTERS) { + if (HasContextPart(context, WOW64_CONTEXT_DEBUG_REGISTERS)) { out->dr0 = context.Dr0; out->dr1 = context.Dr1; out->dr2 = context.Dr2; @@ -71,12 +98,28 @@ void CommonInitializeX86Context(const T& context, CPUContextX86* out) { out->dr7 = context.Dr7; } - if (context.ContextFlags & WOW64_CONTEXT_EXTENDED_REGISTERS) { + if (HasContextPart(context, WOW64_CONTEXT_EXTENDED_REGISTERS)) { static_assert(sizeof(out->fxsave) == sizeof(context.ExtendedRegisters), "types must be equivalent"); memcpy(&out->fxsave, &context.ExtendedRegisters, sizeof(out->fxsave)); - } else if (context.ContextFlags & WOW64_CONTEXT_FLOATING_POINT) { - CHECK(false) << "TODO(scottmg): extract x87 data"; + } else if (HasContextPart(context, WOW64_CONTEXT_FLOATING_POINT)) { + out->fxsave.fcw = static_cast(context.FloatSave.ControlWord); + out->fxsave.fsw = static_cast(context.FloatSave.StatusWord); + out->fxsave.ftw = CPUContextX86::FsaveToFxsaveTagWord( + static_cast(context.FloatSave.TagWord)); + out->fxsave.fop = context.FloatSave.ErrorSelector >> 16; + out->fxsave.fpu_ip = context.FloatSave.ErrorOffset; + out->fxsave.fpu_cs = static_cast(context.FloatSave.ErrorSelector); + out->fxsave.fpu_dp = context.FloatSave.DataOffset; + out->fxsave.fpu_ds = static_cast(context.FloatSave.DataSelector); + const CPUContextX86::X87Register* context_floatsave_st = + reinterpret_cast( + context.FloatSave.RegisterArea); + for (size_t index = 0; index < arraysize(out->fxsave.st_mm); ++index) { + memcpy(out->fxsave.st_mm[index].st, + context_floatsave_st[index], + sizeof(out->fxsave.st_mm[index].st)); + } } } @@ -91,9 +134,9 @@ void InitializeX86Context(const WOW64_CONTEXT& context, CPUContextX86* out) { void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { memset(out, 0, sizeof(*out)); - LOG_IF(ERROR, !(context.ContextFlags & CONTEXT_AMD64)) << "non-x64 context"; + LOG_IF(ERROR, !HasContextPart(context, CONTEXT_AMD64)) << "non-x64 context"; - if (context.ContextFlags & CONTEXT_CONTROL) { + if (HasContextPart(context, CONTEXT_CONTROL)) { out->cs = context.SegCs; out->rflags = context.EFlags; out->rip = context.Rip; @@ -101,7 +144,7 @@ void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { // SegSs ignored. } - if (context.ContextFlags & CONTEXT_INTEGER) { + if (HasContextPart(context, CONTEXT_INTEGER)) { out->rax = context.Rax; out->rbx = context.Rbx; out->rcx = context.Rcx; @@ -119,14 +162,14 @@ void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { out->r15 = context.R15; } - if (context.ContextFlags & CONTEXT_SEGMENTS) { + if (HasContextPart(context, CONTEXT_SEGMENTS)) { out->fs = context.SegFs; out->gs = context.SegGs; // SegDs ignored. // SegEs ignored. } - if (context.ContextFlags & CONTEXT_DEBUG_REGISTERS) { + if (HasContextPart(context, CONTEXT_DEBUG_REGISTERS)) { out->dr0 = context.Dr0; out->dr1 = context.Dr1; out->dr2 = context.Dr2; @@ -139,7 +182,7 @@ void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { out->dr7 = context.Dr7; } - if (context.ContextFlags & CONTEXT_FLOATING_POINT) { + if (HasContextPart(context, CONTEXT_FLOATING_POINT)) { static_assert(sizeof(out->fxsave) == sizeof(context.FltSave), "types must be equivalent"); memcpy(&out->fxsave, &context.FltSave.ControlWord, sizeof(out->fxsave)); diff --git a/snapshot/win/cpu_context_win_test.cc b/snapshot/win/cpu_context_win_test.cc index 0c7484b7..1f8bfe98 100644 --- a/snapshot/win/cpu_context_win_test.cc +++ b/snapshot/win/cpu_context_win_test.cc @@ -16,6 +16,7 @@ #include +#include "base/macros.h" #include "build/build_config.h" #include "gtest/gtest.h" #include "snapshot/cpu_context.h" @@ -24,6 +25,84 @@ namespace crashpad { namespace test { namespace { +template +void TestInitializeX86Context() { + T context = {0}; + context.ContextFlags = WOW64_CONTEXT_INTEGER | + WOW64_CONTEXT_DEBUG_REGISTERS | + WOW64_CONTEXT_EXTENDED_REGISTERS; + context.Eax = 1; + context.Dr0 = 3; + context.ExtendedRegisters[4] = 2; // FTW + + // Test the simple case, where everything in the CPUContextX86 argument is set + // directly from the supplied thread, float, and debug state parameters. + { + CPUContextX86 cpu_context_x86 = {}; + InitializeX86Context(context, &cpu_context_x86); + EXPECT_EQ(1u, cpu_context_x86.eax); + EXPECT_EQ(2u, cpu_context_x86.fxsave.ftw); + EXPECT_EQ(3u, cpu_context_x86.dr0); + } +} + +template +void TestInitializeX86Context_FsaveWithoutFxsave() { + T context = {0}; + context.ContextFlags = WOW64_CONTEXT_INTEGER | + WOW64_CONTEXT_FLOATING_POINT | + WOW64_CONTEXT_DEBUG_REGISTERS; + context.Eax = 1; + + // In fields that are wider than they need to be, set the high bits to ensure + // that they’re masked off appropriately in the output. + context.FloatSave.ControlWord = 0xffff027f; + context.FloatSave.StatusWord = 0xffff0004; + context.FloatSave.TagWord = 0xffffa9ff; + context.FloatSave.ErrorOffset = 0x01234567; + context.FloatSave.ErrorSelector = 0x0bad0003; + context.FloatSave.DataOffset = 0x89abcdef; + context.FloatSave.DataSelector = 0xffff0007; + context.FloatSave.RegisterArea[77] = 0x80; + context.FloatSave.RegisterArea[78] = 0xff; + context.FloatSave.RegisterArea[79] = 0x7f; + + context.Dr0 = 3; + + { + CPUContextX86 cpu_context_x86 = {}; + InitializeX86Context(context, &cpu_context_x86); + + EXPECT_EQ(1u, cpu_context_x86.eax); + + EXPECT_EQ(0x027f, cpu_context_x86.fxsave.fcw); + EXPECT_EQ(0x0004, cpu_context_x86.fxsave.fsw); + EXPECT_EQ(0x00f0, cpu_context_x86.fxsave.ftw); + EXPECT_EQ(0x0bad, cpu_context_x86.fxsave.fop); + EXPECT_EQ(0x01234567, cpu_context_x86.fxsave.fpu_ip); + EXPECT_EQ(0x0003, cpu_context_x86.fxsave.fpu_cs); + EXPECT_EQ(0x89abcdef, cpu_context_x86.fxsave.fpu_dp); + EXPECT_EQ(0x0007, cpu_context_x86.fxsave.fpu_ds); + for (size_t st_mm = 0; st_mm < 7; ++st_mm) { + for (size_t byte = 0; + byte < arraysize(cpu_context_x86.fxsave.st_mm[st_mm].st); + ++byte) { + EXPECT_EQ(0x00, cpu_context_x86.fxsave.st_mm[st_mm].st[byte]); + } + } + for (size_t byte = 0; byte < 7; ++byte) { + EXPECT_EQ(0x00, cpu_context_x86.fxsave.st_mm[7].st[byte]); + } + EXPECT_EQ(0x80, cpu_context_x86.fxsave.st_mm[7].st[7]); + EXPECT_EQ(0xff, cpu_context_x86.fxsave.st_mm[7].st[8]); + EXPECT_EQ(0x7f, cpu_context_x86.fxsave.st_mm[7].st[9]); + + EXPECT_EQ(3u, cpu_context_x86.dr0); + } +} + +#if defined(ARCH_CPU_X86_FAMILY) + #if defined(ARCH_CPU_X86_64) TEST(CPUContextWin, InitializeX64Context) { @@ -45,28 +124,25 @@ TEST(CPUContextWin, InitializeX64Context) { } } -#else +#endif // ARCH_CPU_X86_64 TEST(CPUContextWin, InitializeX86Context) { - CONTEXT context = {0}; - context.ContextFlags = - CONTEXT_INTEGER | CONTEXT_EXTENDED_REGISTERS | CONTEXT_DEBUG_REGISTERS; - context.Eax = 1; - context.ExtendedRegisters[4] = 2; // FTW. - context.Dr0 = 3; - - // Test the simple case, where everything in the CPUContextX86 argument is - // set directly from the supplied thread, float, and debug state parameters. - { - CPUContextX86 cpu_context_x86 = {}; - InitializeX86Context(context, &cpu_context_x86); - EXPECT_EQ(1u, cpu_context_x86.eax); - EXPECT_EQ(2u, cpu_context_x86.fxsave.ftw); - EXPECT_EQ(3u, cpu_context_x86.dr0); - } +#if defined(ARCH_CPU_X86) + TestInitializeX86Context(); +#else // ARCH_CPU_X86 + TestInitializeX86Context(); +#endif // ARCH_CPU_X86 } -#endif // ARCH_CPU_X86_64 +TEST(CPUContextWin, InitializeX86Context_FsaveWithoutFxsave) { +#if defined(ARCH_CPU_X86) + TestInitializeX86Context_FsaveWithoutFxsave(); +#else // ARCH_CPU_X86 + TestInitializeX86Context_FsaveWithoutFxsave(); +#endif // ARCH_CPU_X86 +} + +#endif // ARCH_CPU_X86_FAMILY } // namespace } // namespace test From 0c322ecc3f711c34fbf85b2cbe69f38b8dbccf05 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 15 Feb 2017 19:54:19 -0500 Subject: [PATCH 04/10] Use zlib to gzip-compress uploads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds zlib to Crashpad. By default in standalone Crashpad builds, the system zlib will be used where available. A copy of Chromium’s zlib (currently a slightly patched 1.2.11) is checked out via DEPS into third_party for use on Windows, which does not have a system zlib. zlib is used to produce gzip streams for HTTP upload request bodies sent by crashpad_handler by default. The Content-Encoding: gzip header is set for these compressed request bodies. Compression can be disabled for upload to servers without corresponding decompression support by starting crashpad_handler with the --no-upload-gzip option. Most minidumps compress quite well with zlib. A size reduction of 90% is not uncommon. BUG=crashpad:157 TEST=crashpad_util_test GzipHTTPBodyStream.*:HTTPTransport.* Change-Id: I99b86db3952c3685cd78f5dc858a60b54399c513 Reviewed-on: https://chromium-review.googlesource.com/438585 Reviewed-by: Robert Sesek --- .gitignore | 1 + DEPS | 3 + handler/crash_report_upload_thread.cc | 15 +- handler/crash_report_upload_thread.h | 5 +- handler/crashpad_handler.md | 8 ++ handler/handler_main.cc | 11 +- third_party/zlib/README.crashpad | 18 +++ third_party/zlib/zlib.gyp | 137 ++++++++++++++++++ third_party/zlib/zlib_crashpad.h | 32 +++++ util/misc/zlib.cc | 37 +++++ util/misc/zlib.h | 42 ++++++ util/net/http_body_gzip.cc | 126 +++++++++++++++++ util/net/http_body_gzip.h | 67 +++++++++ util/net/http_body_gzip_test.cc | 178 ++++++++++++++++++++++++ util/net/http_headers.cc | 2 +- util/net/http_headers.h | 3 + util/net/http_multipart_builder.cc | 28 +++- util/net/http_multipart_builder.h | 17 ++- util/net/http_multipart_builder_test.cc | 3 + util/net/http_transport_test.cc | 16 ++- util/net/http_transport_test_server.py | 8 ++ util/util.gyp | 5 + util/util_test.gyp | 2 + 23 files changed, 748 insertions(+), 16 deletions(-) create mode 100644 third_party/zlib/README.crashpad create mode 100644 third_party/zlib/zlib.gyp create mode 100644 third_party/zlib/zlib_crashpad.h create mode 100644 util/misc/zlib.cc create mode 100644 util/misc/zlib.h create mode 100644 util/net/http_body_gzip.cc create mode 100644 util/net/http_body_gzip.h create mode 100644 util/net/http_body_gzip_test.cc diff --git a/.gitignore b/.gitignore index 65bb4418..43ab9c0a 100644 --- a/.gitignore +++ b/.gitignore @@ -14,5 +14,6 @@ /third_party/gyp/gyp /third_party/llvm /third_party/mini_chromium/mini_chromium +/third_party/zlib/zlib /xcodebuild tags diff --git a/DEPS b/DEPS index 0da988b7..4aa8dd5a 100644 --- a/DEPS +++ b/DEPS @@ -39,6 +39,9 @@ deps = { 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + 'f65519e442d23498937251e680a3b113927613b0', + 'crashpad/third_party/zlib/zlib': + Var('chromium_git') + '/chromium/src/third_party/zlib@' + + '13dc246a58e4b72104d35f9b1809af95221ebda7', } hooks = [ diff --git a/handler/crash_report_upload_thread.cc b/handler/crash_report_upload_thread.cc index 5bd1fbf9..2c2b69b9 100644 --- a/handler/crash_report_upload_thread.cc +++ b/handler/crash_report_upload_thread.cc @@ -139,7 +139,8 @@ class CallRecordUploadAttempt { CrashReportUploadThread::CrashReportUploadThread(CrashReportDatabase* database, const std::string& url, - bool rate_limit) + bool rate_limit, + bool upload_gzip) : url_(url), // Check for pending reports every 15 minutes, even in the absence of a // signal from the handler thread. This allows for failed uploads to be @@ -147,7 +148,8 @@ CrashReportUploadThread::CrashReportUploadThread(CrashReportDatabase* database, // processes to be recognized. thread_(15 * 60, this), database_(database), - rate_limit_(rate_limit) { + rate_limit_(rate_limit), + upload_gzip_(upload_gzip) { } CrashReportUploadThread::~CrashReportUploadThread() { @@ -308,6 +310,7 @@ CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport( } HTTPMultipartBuilder http_multipart_builder; + http_multipart_builder.SetGzipEnabled(upload_gzip_); const char kMinidumpKey[] = "upload_file_minidump"; @@ -332,9 +335,11 @@ CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport( std::unique_ptr http_transport(HTTPTransport::Create()); http_transport->SetURL(url_); - HTTPHeaders::value_type content_type = - http_multipart_builder.GetContentType(); - http_transport->SetHeader(content_type.first, content_type.second); + HTTPHeaders content_headers; + http_multipart_builder.PopulateContentHeaders(&content_headers); + for (const auto& content_header : content_headers) { + http_transport->SetHeader(content_header.first, content_header.second); + } http_transport->SetBodyStream(http_multipart_builder.GetBodyStream()); // TODO(mark): The timeout should be configurable by the client. http_transport->SetTimeout(60.0); // 1 minute. diff --git a/handler/crash_report_upload_thread.h b/handler/crash_report_upload_thread.h index a9601d14..14debacd 100644 --- a/handler/crash_report_upload_thread.h +++ b/handler/crash_report_upload_thread.h @@ -45,9 +45,11 @@ class CrashReportUploadThread : public WorkerThread::Delegate { //! \param[in] url The URL of the server to upload crash reports to. //! \param[in] rate_limit Whether uploads should be throttled to a (currently //! hardcoded) rate. + //! \param[in] upload_gzip Whether uploads should use `gzip` compression. CrashReportUploadThread(CrashReportDatabase* database, const std::string& url, - bool rate_limit); + bool rate_limit, + bool upload_gzip); ~CrashReportUploadThread(); //! \brief Starts a dedicated upload thread, which executes ThreadMain(). @@ -139,6 +141,7 @@ class CrashReportUploadThread : public WorkerThread::Delegate { WorkerThread thread_; CrashReportDatabase* database_; // weak bool rate_limit_; + bool upload_gzip_; DISALLOW_COPY_AND_ASSIGN(CrashReportUploadThread); }; diff --git a/handler/crashpad_handler.md b/handler/crashpad_handler.md index 31c0b347..30dacfe7 100644 --- a/handler/crashpad_handler.md +++ b/handler/crashpad_handler.md @@ -144,6 +144,14 @@ establish the Crashpad client environment before running a program. throttled to one per hour. Using this option disables that behavior, and Crashpad will attempt to upload all captured reports. + * **--no-upload-gzip** + + Do not use `gzip` compression for uploaded crash reports. Normally, the + entire request body is compressed into a `gzip` stream and transmitted with + `Content-Encoding: gzip`. This option disables compression, and is intended + for use with collection servers that don’t accept uploads compressed in this + way. + * **--pipe-name**=_PIPE_ Listen on the given pipe name for connections from clients. _PIPE_ must be of diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 9433a6af..97322576 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -93,6 +93,7 @@ void Usage(const base::FilePath& me) { #endif // OS_MACOSX " --metrics-dir=DIR store metrics files in DIR (only in Chromium)\n" " --no-rate-limit don't rate limit crash uploads\n" +" --no-upload-gzip don't use gzip compression when uploading\n" #if defined(OS_MACOSX) " --reset-own-crash-exception-port-to-system-default\n" " reset the server's exception handler to default\n" @@ -291,6 +292,7 @@ int HandlerMain(int argc, char* argv[]) { #endif // OS_MACOSX kOptionMetrics, kOptionNoRateLimit, + kOptionNoUploadGzip, #if defined(OS_MACOSX) kOptionResetOwnCrashExceptionPortToSystemDefault, #elif defined(OS_WIN) @@ -317,11 +319,13 @@ int HandlerMain(int argc, char* argv[]) { InitialClientData initial_client_data; #endif // OS_MACOSX bool rate_limit; + bool upload_gzip; } options = {}; #if defined(OS_MACOSX) options.handshake_fd = -1; #endif options.rate_limit = true; + options.upload_gzip = true; const option long_options[] = { {"annotation", required_argument, nullptr, kOptionAnnotation}, @@ -340,6 +344,7 @@ int HandlerMain(int argc, char* argv[]) { #endif // OS_MACOSX {"metrics-dir", required_argument, nullptr, kOptionMetrics}, {"no-rate-limit", no_argument, nullptr, kOptionNoRateLimit}, + {"no-upload-gzip", no_argument, nullptr, kOptionNoUploadGzip}, #if defined(OS_MACOSX) {"reset-own-crash-exception-port-to-system-default", no_argument, @@ -407,6 +412,10 @@ int HandlerMain(int argc, char* argv[]) { options.rate_limit = false; break; } + case kOptionNoUploadGzip: { + options.upload_gzip = false; + break; + } #if defined(OS_MACOSX) case kOptionResetOwnCrashExceptionPortToSystemDefault: { options.reset_own_crash_exception_port_to_system_default = true; @@ -555,7 +564,7 @@ int HandlerMain(int argc, char* argv[]) { // configurable database setting to control upload limiting. // See https://crashpad.chromium.org/bug/23. CrashReportUploadThread upload_thread( - database.get(), options.url, options.rate_limit); + database.get(), options.url, options.rate_limit, options.upload_gzip); upload_thread.Start(); PruneCrashReportThread prune_thread(database.get(), diff --git a/third_party/zlib/README.crashpad b/third_party/zlib/README.crashpad new file mode 100644 index 00000000..83f47e31 --- /dev/null +++ b/third_party/zlib/README.crashpad @@ -0,0 +1,18 @@ +Name: zlib +Short Name: zlib +URL: http://zlib.net/ +Revision: See zlib/README.chromium +License: zlib +License File: zlib/LICENSE +Security Critical: yes + +Description: +“A massively spiffy yet delicately unobtrusive compression library.” + +zlib is a free, general-purpose, legally unencumbered lossless data-compression +library. zlib implements the “deflate” compression algorithm described by RFC +1951, which combines the LZ77 (Lempel-Ziv) algorithm with Huffman coding. zlib +also implements the zlib (RFC 1950) and gzip (RFC 1952) wrapper formats. + +Local Modifications: +See zlib/README.chromium. diff --git a/third_party/zlib/zlib.gyp b/third_party/zlib/zlib.gyp new file mode 100644 index 00000000..f92cdf26 --- /dev/null +++ b/third_party/zlib/zlib.gyp @@ -0,0 +1,137 @@ +# Copyright 2017 The Crashpad Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +{ + 'variables': { + 'conditions': [ + # Use the system zlib by default where available, as it is on most + # platforms. Windows does not have a system zlib, so use “embedded” which + # directs the build to use the source code in the zlib subdirectory. + ['OS!="win"', { + 'zlib_source%': 'system', + }, { + 'zlib_source%': 'embedded', + }], + ], + }, + 'targets': [ + { + 'target_name': 'zlib', + 'conditions': [ + ['zlib_source=="system"', { + 'type': 'none', + 'direct_dependent_settings': { + 'defines': [ + 'CRASHPAD_ZLIB_SOURCE_SYSTEM', + ], + }, + 'link_settings': { + 'conditions': [ + ['OS=="mac"', { + 'libraries': [ + '$(SDKROOT)/usr/lib/libz.dylib', + ], + }, { + 'libraries': [ + '-lz', + ], + }], + ], + }, + }], + ['zlib_source=="embedded"', { + 'type': 'static_library', + 'include_dirs': [ + 'zlib', + ], + 'defines': [ + 'CRASHPAD_ZLIB_SOURCE_EMBEDDED', + 'HAVE_STDARG_H', + ], + 'direct_dependent_settings': { + 'include_dirs': [ + 'zlib', + ], + 'defines': [ + 'CRASHPAD_ZLIB_SOURCE_EMBEDDED', + ], + }, + 'sources': [ + 'zlib/adler32.c', + 'zlib/compress.c', + 'zlib/crc32.c', + 'zlib/crc32.h', + 'zlib/crc_folding.c', + 'zlib/deflate.c', + 'zlib/deflate.h', + 'zlib/fill_window_sse.c', + 'zlib/gzclose.c', + 'zlib/gzguts.h', + 'zlib/gzlib.c', + 'zlib/gzread.c', + 'zlib/gzwrite.c', + 'zlib/infback.c', + 'zlib/inffast.c', + 'zlib/inffast.h', + 'zlib/inffixed.h', + 'zlib/inflate.c', + 'zlib/inflate.h', + 'zlib/inftrees.c', + 'zlib/inftrees.h', + 'zlib/names.h', + 'zlib/simd_stub.c', + 'zlib/trees.c', + 'zlib/trees.h', + 'zlib/uncompr.c', + 'zlib/x86.c', + 'zlib/x86.h', + 'zlib/zconf.h', + 'zlib/zlib.h', + 'zlib/zutil.c', + 'zlib/zutil.h', + 'zlib_crashpad.h', + ], + 'conditions': [ + ['target_arch=="x86" or target_arch=="amd64"', { + 'sources!': [ + 'zlib/simd_stub.c', + ], + }, { + 'sources!': [ + 'zlib/crc_folding.c', + 'zlib/fill_window_sse.c', + 'zlib/x86.c', + 'zlib/x86.h', + ], + }], + ['OS!="win"', { + 'defines': [ + 'HAVE_HIDDEN', + 'HAVE_UNISTD_H', + ], + }, { + 'msvs_disabled_warnings': [ + 4131, # uses old-style declarator + 4244, # conversion from 't1' to 't2', possible loss of data + 4245, # conversion from 't1' to 't2', signed/unsigned mismatch + 4267, # conversion from 'size_t' to 't', possible loss of data + 4324, # structure was padded due to alignment specifier + ], + }], + ], + }], + ], + }, + ], +} diff --git a/third_party/zlib/zlib_crashpad.h b/third_party/zlib/zlib_crashpad.h new file mode 100644 index 00000000..2ab542e0 --- /dev/null +++ b/third_party/zlib/zlib_crashpad.h @@ -0,0 +1,32 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_THIRD_PARTY_ZLIB_ZLIB_CRASHPAD_H_ +#define CRASHPAD_THIRD_PARTY_ZLIB_ZLIB_CRASHPAD_H_ + +// #include this file instead of the system version of or equivalent +// available at any other location in the source tree. It will #include the +// proper depending on how the build has been configured. + +#if defined(CRASHPAD_ZLIB_SOURCE_SYSTEM) +#include +#elif defined(CRASHPAD_ZLIB_SOURCE_EMBEDDED) +#include "third_party/zlib/zlib/zlib.h" +#elif defined(CRASHPAD_ZLIB_SOURCE_CHROMIUM) +#include "third_party/zlib/zlib.h" +#else +#error Unknown zlib source +#endif + +#endif // CRASHPAD_THIRD_PARTY_ZLIB_ZLIB_CRASHPAD_H_ diff --git a/util/misc/zlib.cc b/util/misc/zlib.cc new file mode 100644 index 00000000..b26f9c90 --- /dev/null +++ b/util/misc/zlib.cc @@ -0,0 +1,37 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/misc/zlib.h" + +#include "base/logging.h" +#include "base/strings/stringprintf.h" +#include "third_party/zlib/zlib_crashpad.h" + +namespace crashpad { + +int ZlibWindowBitsWithGzipWrapper(int window_bits) { + // See the documentation for deflateInit2() and inflateInit2() in . 0 + // is only valid during decompression. + + DCHECK(window_bits == 0 || (window_bits >= 8 && window_bits <= 15)) + << window_bits; + + return 16 + window_bits; +} + +std::string ZlibErrorString(int zr) { + return base::StringPrintf("%s (%d)", zError(zr), zr); +} + +} // namespace crashpad diff --git a/util/misc/zlib.h b/util/misc/zlib.h new file mode 100644 index 00000000..e3da6438 --- /dev/null +++ b/util/misc/zlib.h @@ -0,0 +1,42 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_UTIL_MISC_ZLIB_H_ +#define CRASHPAD_UTIL_MISC_ZLIB_H_ + +#include + +namespace crashpad { + +//! \brief Obtain a \a window_bits parameter to pass to `deflateInit2()` or +//! `inflateInit2()` that specifies a `gzip` wrapper instead of the default +//! zlib wrapper. +//! +//! \param[in] A \a window_bits value that only specifies the base-2 logarithm +//! of the deflate sliding window size. +//! +//! \return \a window_bits adjusted to specify a `gzip` wrapper, to be passed to +//! `deflateInit2()` or `inflateInit2()`. +int ZlibWindowBitsWithGzipWrapper(int window_bits); + +//! \brief Formats a string for an error received from the zlib library. +//! +//! \param[in] zr A zlib result code, such as `Z_STREAM_ERROR`. +//! +//! \return A formatted string. +std::string ZlibErrorString(int zr); + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_MISC_ZLIB_H_ diff --git a/util/net/http_body_gzip.cc b/util/net/http_body_gzip.cc new file mode 100644 index 00000000..70f4db35 --- /dev/null +++ b/util/net/http_body_gzip.cc @@ -0,0 +1,126 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/net/http_body_gzip.h" + +#include + +#include "base/logging.h" +#include "base/numerics/safe_conversions.h" +#include "third_party/zlib/zlib_crashpad.h" +#include "util/misc/zlib.h" + +namespace crashpad { + +GzipHTTPBodyStream::GzipHTTPBodyStream(std::unique_ptr source) + : input_(), + source_(std::move(source)), + z_stream_(new z_stream()), + state_(State::kUninitialized) {} + +GzipHTTPBodyStream::~GzipHTTPBodyStream() { + DCHECK(state_ == State::kUninitialized || + state_ == State::kFinished || + state_ == State::kError); +} + +FileOperationResult GzipHTTPBodyStream::GetBytesBuffer(uint8_t* buffer, + size_t max_len) { + if (state_ == State::kError) { + return -1; + } + + if (state_ == State::kFinished) { + return 0; + } + + if (state_ == State::kUninitialized) { + z_stream_->zalloc = Z_NULL; + z_stream_->zfree = Z_NULL; + z_stream_->opaque = Z_NULL; + + // The default values for zlib’s internal MAX_WBITS and DEF_MEM_LEVEL. These + // are the values that deflateInit() would use, but they’re not exported + // from zlib. deflateInit2() is used instead of deflateInit() to get the + // gzip wrapper. + const int kZlibMaxWindowBits = 15; + const int kZlibDefaultMemoryLevel = 8; + + int zr = deflateInit2(z_stream_.get(), + Z_DEFAULT_COMPRESSION, + Z_DEFLATED, + ZlibWindowBitsWithGzipWrapper(kZlibMaxWindowBits), + kZlibDefaultMemoryLevel, + Z_DEFAULT_STRATEGY); + if (zr != Z_OK) { + LOG(ERROR) << "deflateInit2: " << ZlibErrorString(zr); + state_ = State::kError; + return -1; + } + + state_ = State::kOperating; + } + + z_stream_->next_out = buffer; + z_stream_->avail_out = base::saturated_cast(max_len); + + while (state_ != State::kFinished && z_stream_->avail_out > 0) { + if (state_ != State::kInputEOF && z_stream_->avail_in == 0) { + FileOperationResult input_bytes = + source_->GetBytesBuffer(input_, sizeof(input_)); + if (input_bytes == -1) { + Done(State::kError); + return -1; + } + + if (input_bytes == 0) { + state_ = State::kInputEOF; + } + + z_stream_->next_in = input_; + z_stream_->avail_in = base::checked_cast(input_bytes); + } + + int zr = deflate(z_stream_.get(), + state_ == State::kInputEOF ? Z_FINISH : Z_NO_FLUSH); + if (state_ == State::kInputEOF && zr == Z_STREAM_END) { + Done(State::kFinished); + if (state_ == State::kError) { + return -1; + } + } else if (zr != Z_OK) { + LOG(ERROR) << "deflate: " << ZlibErrorString(zr); + Done(State::kError); + return -1; + } + } + + DCHECK_LE(z_stream_->avail_out, max_len); + return max_len - z_stream_->avail_out; +} + +void GzipHTTPBodyStream::Done(State state) { + DCHECK(state_ == State::kOperating || state_ == State::kInputEOF) << state_; + DCHECK(state == State::kFinished || state == State::kError) << state; + + int zr = deflateEnd(z_stream_.get()); + if (zr != Z_OK) { + LOG(ERROR) << "deflateEnd: " << ZlibErrorString(zr); + state_ = State::kError; + } else { + state_ = state; + } +} + +} // namespace crashpad diff --git a/util/net/http_body_gzip.h b/util/net/http_body_gzip.h new file mode 100644 index 00000000..da3a5f24 --- /dev/null +++ b/util/net/http_body_gzip.h @@ -0,0 +1,67 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_UTIL_NET_HTTP_BODY_GZIP_H_ +#define CRASHPAD_UTIL_NET_HTTP_BODY_GZIP_H_ + +#include +#include + +#include + +#include "base/macros.h" +#include "util/file/file_io.h" +#include "util/net/http_body.h" + +extern "C" { +typedef struct z_stream_s z_stream; +} // extern "C" + +namespace crashpad { + +//! \brief An implementation of HTTPBodyStream that `gzip`-compresses another +//! HTTPBodyStream. +class GzipHTTPBodyStream : public HTTPBodyStream { + public: + explicit GzipHTTPBodyStream(std::unique_ptr source); + + ~GzipHTTPBodyStream() override; + + // HTTPBodyStream: + FileOperationResult GetBytesBuffer(uint8_t* buffer, size_t max_len) override; + + private: + enum State : int { + kUninitialized, + kOperating, + kInputEOF, + kFinished, + kError, + }; + + // Calls deflateEnd() and transitions state_ to state. If deflateEnd() fails, + // logs a message and transitions state_ to State::kError. + void Done(State state); + + uint8_t input_[4096]; + std::unique_ptr source_; + std::unique_ptr z_stream_; + State state_; + + DISALLOW_COPY_AND_ASSIGN(GzipHTTPBodyStream); +}; + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_NET_HTTP_BODY_GZIP_H_ diff --git a/util/net/http_body_gzip_test.cc b/util/net/http_body_gzip_test.cc new file mode 100644 index 00000000..a7b97b93 --- /dev/null +++ b/util/net/http_body_gzip_test.cc @@ -0,0 +1,178 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/net/http_body_gzip.h" + +#include + +#include +#include +#include +#include + +#include "base/macros.h" +#include "base/rand_util.h" +#include "base/numerics/safe_conversions.h" +#include "gtest/gtest.h" +#include "third_party/zlib/zlib_crashpad.h" +#include "util/misc/zlib.h" +#include "util/net/http_body.h" + +namespace crashpad { +namespace test { +namespace { + +class ScopedZlibInflateStream { + public: + explicit ScopedZlibInflateStream(z_stream* zlib) : zlib_(zlib) {} + ~ScopedZlibInflateStream() { + int zr = inflateEnd(zlib_); + EXPECT_EQ(Z_OK, zr) << "inflateEnd: " << ZlibErrorString(zr); + } + + private: + z_stream* zlib_; // weak + DISALLOW_COPY_AND_ASSIGN(ScopedZlibInflateStream); +}; + +void GzipInflate(const std::string& compressed, + std::string* decompressed, + size_t buf_size) { + decompressed->clear(); + + // There’s got to be at least a small buffer. + buf_size = std::max(buf_size, static_cast(1)); + + std::unique_ptr buf(new uint8_t[buf_size]); + z_stream zlib = {}; + zlib.zalloc = Z_NULL; + zlib.zfree = Z_NULL; + zlib.opaque = Z_NULL; + zlib.next_in = reinterpret_cast(const_cast(&compressed[0])); + zlib.avail_in = base::checked_cast(compressed.size()); + zlib.next_out = buf.get(); + zlib.avail_out = base::checked_cast(buf_size); + + int zr = inflateInit2(&zlib, ZlibWindowBitsWithGzipWrapper(0)); + ASSERT_EQ(Z_OK, zr) << "inflateInit2: " << ZlibErrorString(zr); + ScopedZlibInflateStream zlib_inflate(&zlib); + + zr = inflate(&zlib, Z_FINISH); + ASSERT_EQ(Z_STREAM_END, zr) << "inflate: " << ZlibErrorString(zr); + + ASSERT_LE(zlib.avail_out, buf_size); + decompressed->assign(reinterpret_cast(buf.get()), + buf_size - zlib.avail_out); +} + +void TestGzipDeflateInflate(const std::string& string) { + std::unique_ptr string_stream( + new StringHTTPBodyStream(string)); + GzipHTTPBodyStream gzip_stream(std::move(string_stream)); + + // The minimum size of a gzip wrapper per RFC 1952: a 10-byte header and an + // 8-byte trailer. + const size_t kGzipHeaderSize = 18; + + // Per http://www.zlib.net/zlib_tech.html, in the worst case, zlib will store + // uncompressed data as-is, at an overhead of 5 bytes per 16384-byte block. + // Zero-length input will “compress” to a 2-byte zlib stream. Add the overhead + // of the gzip wrapper, assuming no optional fields are present. + size_t buf_size = + string.size() + kGzipHeaderSize + + (string.empty() ? 2 : (((string.size() + 16383) / 16384) * 5)); + std::unique_ptr buf(new uint8_t[buf_size]); + FileOperationResult compressed_bytes = + gzip_stream.GetBytesBuffer(buf.get(), buf_size); + ASSERT_NE(compressed_bytes, -1); + ASSERT_LE(static_cast(compressed_bytes), buf_size); + + // Make sure that the stream is really at EOF. + uint8_t eof_buf[16]; + ASSERT_EQ(0, gzip_stream.GetBytesBuffer(eof_buf, sizeof(eof_buf))); + + std::string compressed(reinterpret_cast(buf.get()), compressed_bytes); + + ASSERT_GE(compressed.size(), kGzipHeaderSize); + EXPECT_EQ('\37', compressed[0]); + EXPECT_EQ('\213', compressed[1]); + EXPECT_EQ(Z_DEFLATED, compressed[2]); + + std::string decompressed; + ASSERT_NO_FATAL_FAILURE( + GzipInflate(compressed, &decompressed, string.size())); + + EXPECT_EQ(string, decompressed); + + // In block mode, compression should be identical. + string_stream.reset(new StringHTTPBodyStream(string)); + GzipHTTPBodyStream block_gzip_stream(std::move(string_stream)); + uint8_t block_buf[4096]; + std::string block_compressed; + FileOperationResult block_compressed_bytes; + while ((block_compressed_bytes = block_gzip_stream.GetBytesBuffer( + block_buf, sizeof(block_buf))) > 0) { + block_compressed.append(reinterpret_cast(block_buf), + block_compressed_bytes); + } + ASSERT_EQ(0, block_compressed_bytes); + EXPECT_EQ(compressed, block_compressed); +} + +std::string MakeString(size_t size) { + std::string string; + for (size_t i = 0; i < size; ++i) { + string.append(1, (i % 256) ^ ((i >> 8) % 256)); + } + return string; +} + +constexpr size_t kFourKBytes = 4096; +constexpr size_t kManyBytes = 375017; + +TEST(GzipHTTPBodyStream, Empty) { + TestGzipDeflateInflate(std::string()); +} + +TEST(GzipHTTPBodyStream, OneByte) { + TestGzipDeflateInflate(std::string("Z")); +} + +TEST(GzipHTTPBodyStream, FourKBytes_NUL) { + TestGzipDeflateInflate(std::string(kFourKBytes, '\0')); +} + +TEST(GzipHTTPBodyStream, ManyBytes_NUL) { + TestGzipDeflateInflate(std::string(kManyBytes, '\0')); +} + +TEST(GzipHTTPBodyStream, FourKBytes_Deterministic) { + TestGzipDeflateInflate(MakeString(kFourKBytes)); +} + +TEST(GzipHTTPBodyStream, ManyBytes_Deterministic) { + TestGzipDeflateInflate(MakeString(kManyBytes)); +} + +TEST(GzipHTTPBodyStream, FourKBytes_Random) { + TestGzipDeflateInflate(base::RandBytesAsString(kFourKBytes)); +} + +TEST(GzipHTTPBodyStream, ManyBytes_Random) { + TestGzipDeflateInflate(base::RandBytesAsString(kManyBytes)); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/net/http_headers.cc b/util/net/http_headers.cc index 09d61b3e..37d84c67 100644 --- a/util/net/http_headers.cc +++ b/util/net/http_headers.cc @@ -17,7 +17,7 @@ namespace crashpad { const char kContentType[] = "Content-Type"; - const char kContentLength[] = "Content-Length"; +const char kContentEncoding[] = "Content-Encoding"; } // namespace crashpad diff --git a/util/net/http_headers.h b/util/net/http_headers.h index 3633cb2d..851ff31c 100644 --- a/util/net/http_headers.h +++ b/util/net/http_headers.h @@ -29,6 +29,9 @@ extern const char kContentType[]; //! \brief The header name `"Content-Length"`. extern const char kContentLength[]; +//! \brief The header name `"Content-Encoding"`. +extern const char kContentEncoding[]; + } // namespace crashpad #endif // CRASHPAD_UTIL_NET_HTTP_HEADERS_H_ diff --git a/util/net/http_multipart_builder.cc b/util/net/http_multipart_builder.cc index 46f6f090..640d540a 100644 --- a/util/net/http_multipart_builder.cc +++ b/util/net/http_multipart_builder.cc @@ -23,6 +23,7 @@ #include "base/rand_util.h" #include "base/strings/stringprintf.h" #include "util/net/http_body.h" +#include "util/net/http_body_gzip.h" namespace crashpad { @@ -116,12 +117,18 @@ void AssertSafeMIMEType(const std::string& string) { } // namespace HTTPMultipartBuilder::HTTPMultipartBuilder() - : boundary_(GenerateBoundaryString()), form_data_(), file_attachments_() { -} + : boundary_(GenerateBoundaryString()), + form_data_(), + file_attachments_(), + gzip_enabled_(false) {} HTTPMultipartBuilder::~HTTPMultipartBuilder() { } +void HTTPMultipartBuilder::SetGzipEnabled(bool gzip_enabled) { + gzip_enabled_ = gzip_enabled; +} + void HTTPMultipartBuilder::SetFormData(const std::string& key, const std::string& value) { EraseKey(key); @@ -179,13 +186,24 @@ std::unique_ptr HTTPMultipartBuilder::GetBodyStream() { streams.push_back( new StringHTTPBodyStream("--" + boundary_ + "--" + kCRLF)); - return std::unique_ptr(new CompositeHTTPBodyStream(streams)); + auto composite = + std::unique_ptr(new CompositeHTTPBodyStream(streams)); + if (gzip_enabled_) { + return std::unique_ptr( + new GzipHTTPBodyStream(std::move(composite))); + } + return composite; } -HTTPHeaders::value_type HTTPMultipartBuilder::GetContentType() const { +void HTTPMultipartBuilder::PopulateContentHeaders( + HTTPHeaders* http_headers) const { std::string content_type = base::StringPrintf("multipart/form-data; boundary=%s", boundary_.c_str()); - return std::make_pair(kContentType, content_type); + (*http_headers)[kContentType] = content_type; + + if (gzip_enabled_) { + (*http_headers)[kContentEncoding] = "gzip"; + } } void HTTPMultipartBuilder::EraseKey(const std::string& key) { diff --git a/util/net/http_multipart_builder.h b/util/net/http_multipart_builder.h index 65602c71..e0c98fa4 100644 --- a/util/net/http_multipart_builder.h +++ b/util/net/http_multipart_builder.h @@ -34,6 +34,15 @@ class HTTPMultipartBuilder { HTTPMultipartBuilder(); ~HTTPMultipartBuilder(); + //! \brief Enables or disables `gzip` compression. + //! + //! \param[in] gzip_enabled Whether to enable or disable `gzip` compression. + //! + //! When `gzip` compression is enabled, the body stream returned by + //! GetBodyStream() will be `gzip`-compressed, and the content headers set by + //! PopulateContentHeaders() will contain `Content-Encoding: gzip`. + void SetGzipEnabled(bool gzip_enabled); + //! \brief Sets a `Content-Disposition: form-data` key-value pair. //! //! \param[in] key The key of the form data, specified as the `name` in the @@ -64,8 +73,11 @@ class HTTPMultipartBuilder { //! \return A caller-owned HTTPBodyStream object. std::unique_ptr GetBodyStream(); - //! \brief Gets the header pair for `"Content-Type"`. - HTTPHeaders::value_type GetContentType() const; + //! \brief Adds the appropriate content headers to \a http_headers. + //! + //! Any headers that this method adds will replace existing headers by the + //! same name in \a http_headers. + void PopulateContentHeaders(HTTPHeaders* http_headers) const; private: struct FileAttachment { @@ -81,6 +93,7 @@ class HTTPMultipartBuilder { std::string boundary_; std::map form_data_; std::map file_attachments_; + bool gzip_enabled_; DISALLOW_COPY_AND_ASSIGN(HTTPMultipartBuilder); }; diff --git a/util/net/http_multipart_builder_test.cc b/util/net/http_multipart_builder_test.cc index d019d058..fa20abe4 100644 --- a/util/net/http_multipart_builder_test.cc +++ b/util/net/http_multipart_builder_test.cc @@ -71,6 +71,7 @@ TEST(HTTPMultipartBuilder, ThreeStringFields) { ASSERT_TRUE(body.get()); std::string contents = ReadStreamToString(body.get()); auto lines = SplitCRLF(contents); + ASSERT_EQ(13u, lines.size()); auto lines_it = lines.begin(); // The first line is the boundary. All subsequent boundaries must match this. @@ -164,6 +165,7 @@ TEST(HTTPMultipartBuilder, OverwriteFormDataWithEscapedKey) { ASSERT_TRUE(body.get()); std::string contents = ReadStreamToString(body.get()); auto lines = SplitCRLF(contents); + ASSERT_EQ(5u, lines.size()); auto lines_it = lines.begin(); const std::string& boundary = *lines_it++; @@ -253,6 +255,7 @@ TEST(HTTPMultipartBuilder, SharedFormDataAndAttachmentKeyNamespace) { ASSERT_TRUE(body.get()); std::string contents = ReadStreamToString(body.get()); auto lines = SplitCRLF(contents); + ASSERT_EQ(9u, lines.size()); auto lines_it = lines.begin(); const std::string& boundary = *lines_it++; diff --git a/util/net/http_transport_test.cc b/util/net/http_transport_test.cc index e8495308..8dfa68a4 100644 --- a/util/net/http_transport_test.cc +++ b/util/net/http_transport_test.cc @@ -221,7 +221,21 @@ TEST(HTTPTransport, ValidFormData) { builder.SetFormData("key2", "--abcdefg123"); HTTPHeaders headers; - EXPECT_TRUE(headers.insert(builder.GetContentType()).second); + builder.PopulateContentHeaders(&headers); + + HTTPTransportTestFixture test( + headers, builder.GetBodyStream(), 200, &ValidFormData); + test.Run(); +} + +TEST(HTTPTransport, ValidFormData_Gzip) { + HTTPMultipartBuilder builder; + builder.SetGzipEnabled(true); + builder.SetFormData("key1", "test"); + builder.SetFormData("key2", "--abcdefg123"); + + HTTPHeaders headers; + builder.PopulateContentHeaders(&headers); HTTPTransportTestFixture test(headers, builder.GetBodyStream(), 200, &ValidFormData); diff --git a/util/net/http_transport_test_server.py b/util/net/http_transport_test_server.py index e79a428e..7ea15719 100755 --- a/util/net/http_transport_test_server.py +++ b/util/net/http_transport_test_server.py @@ -33,6 +33,7 @@ This could easily have been written in C++ instead. import BaseHTTPServer import struct import sys +import zlib class BufferedReadFile(object): """A File-like object that stores all read contents into a buffer.""" @@ -88,6 +89,13 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): length = int(self.headers.get('Content-Length', -1)) body = self.rfile.read(length) + if self.headers.get('Content-Encoding', '').lower() == 'gzip': + # 15 is the value of |wbits|, which should be at the maximum possible + # value to ensure that any gzip stream can be decoded. The offset of 16 + # specifies that the stream to decompress will be formatted with a gzip + # wrapper. + body = zlib.decompress(body, 16 + 15) + RequestHandler.raw_request += body self.send_response(self.response_code) diff --git a/util/util.gyp b/util/util.gyp index abf0bfdc..b2354d28 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -23,6 +23,7 @@ 'dependencies': [ '../compat/compat.gyp:crashpad_compat', '../third_party/mini_chromium/mini_chromium.gyp:base', + '../third_party/zlib/zlib.gyp:zlib', ], 'include_dirs': [ '..', @@ -106,8 +107,12 @@ 'misc/tri_state.h', 'misc/uuid.cc', 'misc/uuid.h', + 'misc/zlib.cc', + 'misc/zlib.h', 'net/http_body.cc', 'net/http_body.h', + 'net/http_body_gzip.cc', + 'net/http_body_gzip.h', 'net/http_headers.cc', 'net/http_headers.h', 'net/http_multipart_builder.cc', diff --git a/util/util_test.gyp b/util/util_test.gyp index e6bf5635..7636941b 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -29,6 +29,7 @@ '../third_party/gtest/gmock.gyp:gmock_main', '../third_party/gtest/gtest.gyp:gtest', '../third_party/mini_chromium/mini_chromium.gyp:base', + '../third_party/zlib/zlib.gyp:zlib', ], 'include_dirs': [ '..', @@ -62,6 +63,7 @@ 'misc/scoped_forbid_return_test.cc', 'misc/random_string_test.cc', 'misc/uuid_test.cc', + 'net/http_body_gzip_test.cc', 'net/http_body_test.cc', 'net/http_body_test_util.cc', 'net/http_body_test_util.h', From 546e64cd0b1bee8c959c0c443fde8f13df271784 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Thu, 16 Feb 2017 13:25:29 -0500 Subject: [PATCH 05/10] =?UTF-8?q?Centrally=20define=20CPUContextX86::Fsave?= =?UTF-8?q?=20and=20fsave=E2=86=94=EF=B8=8Efxsave=20conversions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As I was finishing d98a4de718d9, it became evident that fsave proliferation was becoming a problem. Especially considering tests, there was much duplicated conversion code. This ties everything up together in a central location. test::BytesToHexString() is a new function to ease testing of byte arrays like x87 registers, without having to loop over each byte. Some static_asserts are added to verify that complex structures that need to maintain interoperability don’t grow or shrink. This is used to check the size of the fxsave and fsave structures, as well as the MinidumpCPUContext* structures. BUG=crashpad:162 Change-Id: I1a1be18096ee9be250cbfb2e006adfd08eba8753 Reviewed-on: https://chromium-review.googlesource.com/444004 Reviewed-by: Scott Graham --- minidump/minidump_context.h | 24 +- minidump/minidump_context_writer.cc | 53 ++-- minidump/test/minidump_context_test_util.cc | 102 +++----- snapshot/cpu_context.cc | 57 +++++ snapshot/cpu_context.h | 57 ++++- snapshot/cpu_context_test.cc | 256 ++++++++++++++++---- snapshot/win/cpu_context_win.cc | 40 +-- snapshot/win/cpu_context_win_test.cc | 21 +- test/hex_string.cc | 35 +++ test/hex_string.h | 40 +++ test/hex_string_test.cc | 34 +++ test/test.gyp | 2 + test/test_test.gyp | 1 + 13 files changed, 541 insertions(+), 181 deletions(-) create mode 100644 test/hex_string.cc create mode 100644 test/hex_string.h create mode 100644 test/hex_string_test.cc diff --git a/minidump/minidump_context.h b/minidump/minidump_context.h index 4eb3acfa..b71b57cc 100644 --- a/minidump/minidump_context.h +++ b/minidump/minidump_context.h @@ -92,7 +92,8 @@ enum MinidumpContextX86Flags : uint32_t { //! \brief Indicates the validity of floating-point state //! (`CONTEXT_FLOATING_POINT`). //! - //! The `float_save` field is valid. + //! The `fsave` field is valid. The `float_save` field is included in this + //! definition, but its members have no practical use asdie from `fsave`. kMinidumpContextX86FloatingPoint = kMinidumpContextX86 | 0x00000008, //! \brief Indicates the validity of debug registers @@ -130,8 +131,9 @@ enum MinidumpContextX86Flags : uint32_t { //! \brief A 32-bit x86 CPU context (register state) carried in a minidump file. //! //! This is analogous to the `CONTEXT` structure on Windows when targeting -//! 32-bit x86. This structure is used instead of `CONTEXT` to make it available -//! when targeting other architectures. +//! 32-bit x86, and the `WOW64_CONTEXT` structure when targeting an x86-family +//! CPU, either 32- or 64-bit. This structure is used instead of `CONTEXT` or +//! `WOW64_CONTEXT` to make it available when targeting other architectures. //! //! \note This structure doesn’t carry `dr4` or `dr5`, which are obsolete and //! normally alias `dr6` and `dr7`, respectively. See Intel Software @@ -152,16 +154,12 @@ struct MinidumpContextX86 { uint32_t dr6; uint32_t dr7; - struct { - uint32_t control_word; - uint32_t status_word; - uint32_t tag_word; - uint32_t error_offset; - uint32_t error_selector; - uint32_t data_offset; - uint32_t data_selector; - uint8_t register_area[80]; - uint32_t spare_0; + // CPUContextX86::Fsave has identical layout to what the x86 CONTEXT + // structure places here. + CPUContextX86::Fsave fsave; + union { + uint32_t spare_0; // As in the native x86 CONTEXT structure since Windows 8 + uint32_t cr0_npx_state; // As in WOW64_CONTEXT and older SDKs’ x86 CONTEXT } float_save; uint32_t gs; diff --git a/minidump/minidump_context_writer.cc b/minidump/minidump_context_writer.cc index cc18c34a..c6c29d96 100644 --- a/minidump/minidump_context_writer.cc +++ b/minidump/minidump_context_writer.cc @@ -14,6 +14,8 @@ #include "minidump/minidump_context_writer.h" +#include +#include #include #include @@ -24,6 +26,28 @@ namespace crashpad { +namespace { + +// Sanity-check complex structures to ensure interoperability. +static_assert(sizeof(MinidumpContextX86) == 716, "MinidumpContextX86 size"); +static_assert(sizeof(MinidumpContextAMD64) == 1232, + "MinidumpContextAMD64 size"); + +// These structures can also be checked against definitions in the Windows SDK. +#if defined(OS_WIN) +#if defined(ARCH_CPU_X86_FAMILY) +static_assert(sizeof(MinidumpContextX86) == sizeof(WOW64_CONTEXT), + "WOW64_CONTEXT size"); +#if defined(ARCH_CPU_X86) +static_assert(sizeof(MinidumpContextX86) == sizeof(CONTEXT), "CONTEXT size"); +#elif defined(ARCH_CPU_X86_64) +static_assert(sizeof(MinidumpContextAMD64) == sizeof(CONTEXT), "CONTEXT size"); +#endif +#endif // ARCH_CPU_X86_FAMILY +#endif // OS_WIN + +} // namespace + MinidumpContextWriter::~MinidumpContextWriter() { } @@ -89,32 +113,11 @@ void MinidumpContextX86Writer::InitializeFromSnapshot( context_.dr6 = context_snapshot->dr6; context_.dr7 = context_snapshot->dr7; - // The contents of context_.float_save effectively alias everything in - // context_.fxsave that’s related to x87 FPU state. context_.float_save - // doesn’t carry state specific to SSE (or later), such as mxcsr and the xmm + // The contents of context_.fsave effectively alias everything in + // context_.fxsave that’s related to x87 FPU state. context_.fsave doesn’t + // carry state specific to SSE (or later), such as mxcsr and the xmm // registers. - context_.float_save.control_word = context_snapshot->fxsave.fcw; - context_.float_save.status_word = context_snapshot->fxsave.fsw; - context_.float_save.tag_word = - CPUContextX86::FxsaveToFsaveTagWord(context_snapshot->fxsave.fsw, - context_snapshot->fxsave.ftw, - context_snapshot->fxsave.st_mm); - context_.float_save.error_offset = context_snapshot->fxsave.fpu_ip; - context_.float_save.error_selector = - (context_snapshot->fxsave.fop << 16) | context_snapshot->fxsave.fpu_cs; - context_.float_save.data_offset = context_snapshot->fxsave.fpu_dp; - context_.float_save.data_selector = context_snapshot->fxsave.fpu_ds; - - CPUContextX86::X87Register* context_float_save_st = - reinterpret_cast( - context_.float_save.register_area); - for (size_t index = 0; - index < arraysize(context_snapshot->fxsave.st_mm); - ++index) { - memcpy(&context_float_save_st[index], - &context_snapshot->fxsave.st_mm[index].st, - sizeof(context_snapshot->fxsave.st_mm[index].st)); - } + CPUContextX86::FxsaveToFsave(context_snapshot->fxsave, &context_.fsave); context_.gs = context_snapshot->gs; context_.fs = context_snapshot->fs; diff --git a/minidump/test/minidump_context_test_util.cc b/minidump/test/minidump_context_test_util.cc index a4d1d26d..8999abe0 100644 --- a/minidump/test/minidump_context_test_util.cc +++ b/minidump/test/minidump_context_test_util.cc @@ -23,6 +23,7 @@ #include "gtest/gtest.h" #include "snapshot/cpu_context.h" #include "snapshot/test/test_cpu_context.h" +#include "test/hex_string.h" namespace crashpad { namespace test { @@ -56,6 +57,7 @@ void InitializeMinidumpContextX86(MinidumpContextX86* context, uint32_t seed) { context->ss = value++ & 0xffff; InitializeCPUContextX86Fxsave(&context->fxsave, &value); + CPUContextX86::FxsaveToFsave(context->fxsave, &context->fsave); context->dr0 = value++; context->dr1 = value++; @@ -65,31 +67,6 @@ void InitializeMinidumpContextX86(MinidumpContextX86* context, uint32_t seed) { context->dr6 = value++; context->dr7 = value++; - // Copy the values that are aliased between the fxsave area - // (context->extended_registers) and the floating-point save area - // (context->float_save). - context->float_save.control_word = context->fxsave.fcw; - context->float_save.status_word = context->fxsave.fsw; - context->float_save.tag_word = CPUContextX86::FxsaveToFsaveTagWord( - context->fxsave.fsw, context->fxsave.ftw, context->fxsave.st_mm); - context->float_save.error_offset = context->fxsave.fpu_ip; - context->float_save.error_selector = - (context->fxsave.fop << 16) | context->fxsave.fpu_cs; - context->float_save.data_offset = context->fxsave.fpu_dp; - context->float_save.data_selector = context->fxsave.fpu_ds; - for (size_t st_mm_index = 0; - st_mm_index < arraysize(context->fxsave.st_mm); - ++st_mm_index) { - for (size_t byte = 0; - byte < arraysize(context->fxsave.st_mm[st_mm_index].st); - ++byte) { - size_t st_index = - st_mm_index * arraysize(context->fxsave.st_mm[st_mm_index].st) + byte; - context->float_save.register_area[st_index] = - context->fxsave.st_mm[st_mm_index].st[byte]; - } - } - // Set this field last, because it has no analogue in CPUContextX86. context->float_save.spare_0 = value++; } @@ -189,37 +166,31 @@ void ExpectMinidumpContextFxsave(const FxsaveType* expected, st_mm_index < arraysize(expected->st_mm); ++st_mm_index) { SCOPED_TRACE(base::StringPrintf("st_mm_index %" PRIuS, st_mm_index)); - for (size_t byte = 0; - byte < arraysize(expected->st_mm[st_mm_index].st); - ++byte) { - EXPECT_EQ(expected->st_mm[st_mm_index].st[byte], - observed->st_mm[st_mm_index].st[byte]) << "byte " << byte; - } - for (size_t byte = 0; - byte < arraysize(expected->st_mm[st_mm_index].st_reserved); - ++byte) { - EXPECT_EQ(expected->st_mm[st_mm_index].st_reserved[byte], - observed->st_mm[st_mm_index].st_reserved[byte]) - << "byte " << byte; - } + EXPECT_EQ(BytesToHexString(expected->st_mm[st_mm_index].st, + arraysize(expected->st_mm[st_mm_index].st)), + BytesToHexString(observed->st_mm[st_mm_index].st, + arraysize(observed->st_mm[st_mm_index].st))); + EXPECT_EQ( + BytesToHexString(expected->st_mm[st_mm_index].st_reserved, + arraysize(expected->st_mm[st_mm_index].st_reserved)), + BytesToHexString(observed->st_mm[st_mm_index].st_reserved, + arraysize(observed->st_mm[st_mm_index].st_reserved))); } for (size_t xmm_index = 0; xmm_index < arraysize(expected->xmm); ++xmm_index) { - SCOPED_TRACE(base::StringPrintf("xmm_index %" PRIuS, xmm_index)); - for (size_t byte = 0; byte < arraysize(expected->xmm[xmm_index]); ++byte) { - EXPECT_EQ(expected->xmm[xmm_index][byte], observed->xmm[xmm_index][byte]) - << "byte " << byte; - } - } - for (size_t byte = 0; byte < arraysize(expected->reserved_4); ++byte) { - EXPECT_EQ(expected->reserved_4[byte], observed->reserved_4[byte]) - << "byte " << byte; - } - for (size_t byte = 0; byte < arraysize(expected->available); ++byte) { - EXPECT_EQ(expected->available[byte], observed->available[byte]) - << "byte " << byte; + EXPECT_EQ(BytesToHexString(expected->xmm[xmm_index], + arraysize(expected->xmm[xmm_index])), + BytesToHexString(observed->xmm[xmm_index], + arraysize(observed->xmm[xmm_index]))) + << "xmm_index " << xmm_index; } + EXPECT_EQ( + BytesToHexString(expected->reserved_4, arraysize(expected->reserved_4)), + BytesToHexString(observed->reserved_4, arraysize(observed->reserved_4))); + EXPECT_EQ( + BytesToHexString(expected->available, arraysize(expected->available)), + BytesToHexString(observed->available, arraysize(observed->available))); } } // namespace @@ -237,22 +208,19 @@ void ExpectMinidumpContextX86( EXPECT_EQ(expected.dr6, observed->dr6); EXPECT_EQ(expected.dr7, observed->dr7); - EXPECT_EQ(expected.float_save.control_word, - observed->float_save.control_word); - EXPECT_EQ(expected.float_save.status_word, observed->float_save.status_word); - EXPECT_EQ(expected.float_save.tag_word, observed->float_save.tag_word); - EXPECT_EQ(expected.float_save.error_offset, - observed->float_save.error_offset); - EXPECT_EQ(expected.float_save.error_selector, - observed->float_save.error_selector); - EXPECT_EQ(expected.float_save.data_offset, observed->float_save.data_offset); - EXPECT_EQ(expected.float_save.data_selector, - observed->float_save.data_selector); - for (size_t index = 0; - index < arraysize(expected.float_save.register_area); - ++index) { - EXPECT_EQ(expected.float_save.register_area[index], - observed->float_save.register_area[index]) << "index " << index; + EXPECT_EQ(expected.fsave.fcw, observed->fsave.fcw); + EXPECT_EQ(expected.fsave.fsw, observed->fsave.fsw); + EXPECT_EQ(expected.fsave.ftw, observed->fsave.ftw); + EXPECT_EQ(expected.fsave.fpu_ip, observed->fsave.fpu_ip); + EXPECT_EQ(expected.fsave.fpu_cs, observed->fsave.fpu_cs); + EXPECT_EQ(expected.fsave.fpu_dp, observed->fsave.fpu_dp); + EXPECT_EQ(expected.fsave.fpu_ds, observed->fsave.fpu_ds); + for (size_t index = 0; index < arraysize(expected.fsave.st); ++index) { + EXPECT_EQ(BytesToHexString(expected.fsave.st[index], + arraysize(expected.fsave.st[index])), + BytesToHexString(observed->fsave.st[index], + arraysize(observed->fsave.st[index]))) + << "index " << index; } if (snapshot) { EXPECT_EQ(0u, observed->float_save.spare_0); diff --git a/snapshot/cpu_context.cc b/snapshot/cpu_context.cc index c846fd1f..c3887a2c 100644 --- a/snapshot/cpu_context.cc +++ b/snapshot/cpu_context.cc @@ -14,13 +14,23 @@ #include "snapshot/cpu_context.h" +#include + #include "base/logging.h" +#include "base/macros.h" #include "util/misc/implicit_cast.h" namespace crashpad { namespace { +// Sanity-check complex structures to ensure interoperability. +static_assert(sizeof(CPUContextX86::Fsave) == 108, "CPUContextX86::Fsave size"); +static_assert(sizeof(CPUContextX86::Fxsave) == 512, + "CPUContextX86::Fxsave size"); +static_assert(sizeof(CPUContextX86_64::Fxsave) == 512, + "CPUContextX86_64::Fxsave size"); + enum { kX87TagValid = 0, kX87TagZero, @@ -30,6 +40,53 @@ enum { } // namespace +// static +void CPUContextX86::FxsaveToFsave(const Fxsave& fxsave, Fsave* fsave) { + fsave->fcw = fxsave.fcw; + fsave->reserved_1 = 0; + fsave->fsw = fxsave.fsw; + fsave->reserved_2 = 0; + fsave->ftw = FxsaveToFsaveTagWord(fxsave.fsw, fxsave.ftw, fxsave.st_mm); + fsave->reserved_3 = 0; + fsave->fpu_ip = fxsave.fpu_ip; + fsave->fpu_cs = fxsave.fpu_cs; + fsave->fop = fxsave.fop; + fsave->fpu_dp = fxsave.fpu_dp; + fsave->fpu_ds = fxsave.fpu_ds; + fsave->reserved_4 = 0; + static_assert(arraysize(fsave->st) == arraysize(fxsave.st_mm), + "FPU stack registers must be equivalent"); + for (size_t index = 0; index < arraysize(fsave->st); ++index) { + memcpy(fsave->st[index], fxsave.st_mm[index].st, sizeof(fsave->st[index])); + } +} + +// static +void CPUContextX86::FsaveToFxsave(const Fsave& fsave, Fxsave* fxsave) { + fxsave->fcw = fsave.fcw; + fxsave->fsw = fsave.fsw; + fxsave->ftw = FsaveToFxsaveTagWord(fsave.ftw); + fxsave->reserved_1 = 0; + fxsave->fop = fsave.fop; + fxsave->fpu_ip = fsave.fpu_ip; + fxsave->fpu_cs = fsave.fpu_cs; + fxsave->reserved_2 = 0; + fxsave->fpu_dp = fsave.fpu_dp; + fxsave->fpu_ds = fsave.fpu_ds; + fxsave->reserved_3 = 0; + fxsave->mxcsr = 0; + fxsave->mxcsr_mask = 0; + static_assert(arraysize(fxsave->st_mm) == arraysize(fsave.st), + "FPU stack registers must be equivalent"); + for (size_t index = 0; index < arraysize(fsave.st); ++index) { + memcpy(fxsave->st_mm[index].st, fsave.st[index], sizeof(fsave.st[index])); + memset(fxsave->st_mm[index].st_reserved, + 0, + sizeof(fxsave->st_mm[index].st_reserved)); + } + memset(fxsave->xmm, 0, sizeof(*fxsave) - offsetof(Fxsave, xmm)); +} + // static uint16_t CPUContextX86::FxsaveToFsaveTagWord( uint16_t fsw, diff --git a/snapshot/cpu_context.h b/snapshot/cpu_context.h index e1989141..f5bc8bf1 100644 --- a/snapshot/cpu_context.h +++ b/snapshot/cpu_context.h @@ -25,6 +25,22 @@ namespace crashpad { struct CPUContextX86 { using X87Register = uint8_t[10]; + struct Fsave { + uint16_t fcw; // FPU control word + uint16_t reserved_1; + uint16_t fsw; // FPU status word + uint16_t reserved_2; + uint16_t ftw; // full FPU tag word + uint16_t reserved_3; + uint32_t fpu_ip; // FPU instruction pointer offset + uint16_t fpu_cs; // FPU instruction pointer segment selector + uint16_t fop; // FPU opcode + uint32_t fpu_dp; // FPU data pointer offset + uint16_t fpu_ds; // FPU data pointer segment selector + uint16_t reserved_4; + X87Register st[8]; + }; + union X87OrMMXRegister { struct { X87Register st; @@ -58,14 +74,49 @@ struct CPUContextX86 { uint8_t available[48]; }; + //! \brief Converts an `fxsave` area to an `fsave` area. + //! + //! `fsave` state is restricted to the x87 FPU, while `fxsave` state includes + //! state related to the x87 FPU as well as state specific to SSE. + //! + //! As the `fxsave` format is a superset of the `fsave` format, this operation + //! fully populates the `fsave` area. `fsave` uses the full 16-bit form for + //! the x87 floating-point tag word, so FxsaveToFsaveTagWord() is used to + //! derive Fsave::ftw from the abridged 8-bit form used by `fxsave`. Reserved + //! fields in \a fsave are set to `0`. + //! + //! \param[in] fxsave The `fxsave` area to convert. + //! \param[out] fsave The `fsave` area to populate. + //! + //! \sa FsaveToFxsave() + static void FxsaveToFsave(const Fxsave& fxsave, Fsave* fsave); + + //! \brief Converts an `fsave` area to an `fxsave` area. + //! + //! `fsave` state is restricted to the x87 FPU, while `fxsave` state includes + //! state related to the x87 FPU as well as state specific to SSE. + //! + //! As the `fsave` format is a subset of the `fxsave` format, this operation + //! cannot fully populate the `fxsave` area. Fields in \a fxsave that have no + //! equivalent in \a fsave are set to `0`, including Fxsave::mxcsr, + //! Fxsave::mxcsr_mask, Fxsave::xmm, and Fxsave::available. + //! FsaveToFxsaveTagWord() is used to derive Fxsave::ftw from the full 16-bit + //! form used by `fsave`. Reserved fields in \a fxsave are set to `0`. + //! + //! \param[in] fsave The `fsave` area to convert. + //! \param[out] fxsave The `fxsave` area to populate. + //! + //! \sa FxsaveToFsave() + static void FsaveToFxsave(const Fsave& fsave, Fxsave* fxsave); + //! \brief Converts x87 floating-point tag words from `fxsave` (abridged, //! 8-bit) to `fsave` (full, 16-bit) form. //! //! `fxsave` stores the x87 floating-point tag word in abridged 8-bit form, //! and `fsave` stores it in full 16-bit form. Some users, notably - //! MinidumpContextX86::float_save::tag_word, require the full 16-bit form, - //! where most other contemporary code uses `fxsave` and thus the abridged - //! 8-bit form found in CPUContextX86::Fxsave::ftw. + //! CPUContextX86::Fsave::ftw, require the full 16-bit form, where most other + //! contemporary code uses `fxsave` and thus the abridged 8-bit form found in + //! CPUContextX86::Fxsave::ftw. //! //! This function converts an abridged tag word to the full version by using //! the abridged tag word and the contents of the registers it describes. See diff --git a/snapshot/cpu_context_test.cc b/snapshot/cpu_context_test.cc index 042b14e7..ffc18cfe 100644 --- a/snapshot/cpu_context_test.cc +++ b/snapshot/cpu_context_test.cc @@ -19,6 +19,7 @@ #include "base/macros.h" #include "gtest/gtest.h" +#include "test/hex_string.h" namespace crashpad { namespace test { @@ -48,37 +49,188 @@ enum FractionValue { //! \param[in] j_bit The value to use for the “J bit” (“integer bit”). //! \param[in] fraction_value If kFractionAllZero, the fraction will be zeroed //! out. If kFractionNormal, the fraction will not be all zeroes. -void SetX87Register(CPUContextX86::X87OrMMXRegister* st_mm, +void SetX87Register(CPUContextX86::X87Register* st, ExponentValue exponent_value, bool j_bit, FractionValue fraction_value) { switch (exponent_value) { case kExponentAllZero: - st_mm->st[9] = 0x80; - st_mm->st[8] = 0; + (*st)[9] = 0x80; + (*st)[8] = 0; break; case kExponentAllOne: - st_mm->st[9] = 0x7f; - st_mm->st[8] = 0xff; + (*st)[9] = 0x7f; + (*st)[8] = 0xff; break; case kExponentNormal: - st_mm->st[9] = 0x55; - st_mm->st[8] = 0x55; + (*st)[9] = 0x55; + (*st)[8] = 0x55; break; } uint8_t fraction_pattern = fraction_value == kFractionAllZero ? 0 : 0x55; - memset(&st_mm->st[0], fraction_pattern, 8); + memset(st, fraction_pattern, 8); if (j_bit) { - st_mm->st[7] |= 0x80; + (*st)[7] |= 0x80; } else { - st_mm->st[7] &= ~0x80; + (*st)[7] &= ~0x80; } +} +//! \brief Initializes an x87 register to a known bit pattern. +//! +//! This behaves as SetX87Register() but also clears the reserved portion of the +//! field as used in the `fxsave` format. +void SetX87OrMMXRegister(CPUContextX86::X87OrMMXRegister* st_mm, + ExponentValue exponent_value, + bool j_bit, + FractionValue fraction_value) { + SetX87Register(&st_mm->st, exponent_value, j_bit, fraction_value); memset(st_mm->st_reserved, 0, sizeof(st_mm->st_reserved)); } +TEST(CPUContextX86, FxsaveToFsave) { + // Establish a somewhat plausible fxsave state. Use nonzero values for + // reserved fields and things that aren’t present in fsave. + CPUContextX86::Fxsave fxsave; + fxsave.fcw = 0x027f; // mask exceptions, 53-bit precision, round to nearest + fxsave.fsw = 1 << 11; // top = 1: logical 0-7 maps to physical 1-7, 0 + fxsave.ftw = 0x1f; // physical 5-7 (logical 4-6) empty + fxsave.reserved_1 = 0x5a; + fxsave.fop = 0x1fe; // fsin + fxsave.fpu_ip = 0x76543210; + fxsave.fpu_cs = 0x0007; + fxsave.reserved_2 = 0x5a5a; + fxsave.fpu_dp = 0xfedcba98; + fxsave.fpu_ds = 0x000f; + fxsave.reserved_3 = 0x5a5a; + fxsave.mxcsr = 0x1f80; + fxsave.mxcsr_mask = 0xffff; + SetX87Register( + &fxsave.st_mm[0].st, kExponentNormal, true, kFractionAllZero); // valid + SetX87Register( + &fxsave.st_mm[1].st, kExponentAllZero, false, kFractionAllZero); // zero + SetX87Register( + &fxsave.st_mm[2].st, kExponentAllOne, true, kFractionAllZero); // spec. + SetX87Register( + &fxsave.st_mm[3].st, kExponentAllOne, true, kFractionNormal); // spec. + SetX87Register( + &fxsave.st_mm[4].st, kExponentAllZero, false, kFractionAllZero); + SetX87Register( + &fxsave.st_mm[5].st, kExponentAllZero, false, kFractionAllZero); + SetX87Register( + &fxsave.st_mm[6].st, kExponentAllZero, false, kFractionAllZero); + SetX87Register( + &fxsave.st_mm[7].st, kExponentNormal, true, kFractionNormal); // valid + for (size_t index = 0; index < arraysize(fxsave.st_mm); ++index) { + memset(&fxsave.st_mm[index].st_reserved, + 0x5a, + sizeof(fxsave.st_mm[index].st_reserved)); + } + memset(&fxsave.xmm, 0x5a, sizeof(fxsave) - offsetof(decltype(fxsave), xmm)); + + CPUContextX86::Fsave fsave; + CPUContextX86::FxsaveToFsave(fxsave, &fsave); + + // Everything should have come over from fxsave. Reserved fields should be + // zero. + EXPECT_EQ(fxsave.fcw, fsave.fcw); + EXPECT_EQ(0, fsave.reserved_1); + EXPECT_EQ(fxsave.fsw, fsave.fsw); + EXPECT_EQ(0, fsave.reserved_2); + EXPECT_EQ(0xfe90, fsave.ftw); // FxsaveToFsaveTagWord + EXPECT_EQ(0, fsave.reserved_3); + EXPECT_EQ(fxsave.fpu_ip, fsave.fpu_ip); + EXPECT_EQ(fxsave.fpu_cs, fsave.fpu_cs); + EXPECT_EQ(fxsave.fop, fsave.fop); + EXPECT_EQ(fxsave.fpu_dp, fsave.fpu_dp); + EXPECT_EQ(fxsave.fpu_ds, fsave.fpu_ds); + EXPECT_EQ(0, fsave.reserved_4); + for (size_t index = 0; index < arraysize(fsave.st); ++index) { + EXPECT_EQ(BytesToHexString(fxsave.st_mm[index].st, + arraysize(fxsave.st_mm[index].st)), + BytesToHexString(fsave.st[index], arraysize(fsave.st[index]))) + << "index " << index; + } +} + +TEST(CPUContextX86, FsaveToFxsave) { + // Establish a somewhat plausible fsave state. Use nonzero values for + // reserved fields. + CPUContextX86::Fsave fsave; + fsave.fcw = 0x0300; // unmask exceptions, 64-bit precision, round to nearest + fsave.reserved_1 = 0xa5a5; + fsave.fsw = 2 << 11; // top = 2: logical 0-7 maps to physical 2-7, 0-1 + fsave.reserved_2 = 0xa5a5; + fsave.ftw = 0xa9ff; // physical 0-3 (logical 6-7, 0-1) empty; physical 4 + // (logical 2) zero; physical 5-7 (logical 3-5) special + fsave.reserved_3 = 0xa5a5; + fsave.fpu_ip = 0x456789ab; + fsave.fpu_cs = 0x1013; + fsave.fop = 0x01ee; // fldz + fsave.fpu_dp = 0x0123cdef; + fsave.fpu_ds = 0x2017; + fsave.reserved_4 = 0xa5a5; + SetX87Register(&fsave.st[0], kExponentAllZero, false, kFractionNormal); + SetX87Register(&fsave.st[1], kExponentAllZero, true, kFractionNormal); + SetX87Register( + &fsave.st[2], kExponentAllZero, false, kFractionAllZero); // zero + SetX87Register( + &fsave.st[3], kExponentAllZero, true, kFractionAllZero); // spec. + SetX87Register( + &fsave.st[4], kExponentAllZero, false, kFractionNormal); // spec. + SetX87Register( + &fsave.st[5], kExponentAllZero, true, kFractionNormal); // spec. + SetX87Register(&fsave.st[6], kExponentAllZero, false, kFractionAllZero); + SetX87Register(&fsave.st[7], kExponentAllZero, true, kFractionAllZero); + + CPUContextX86::Fxsave fxsave; + CPUContextX86::FsaveToFxsave(fsave, &fxsave); + + // Everything in fsave should have come over from there. Fields not present in + // fsave and reserved fields should be zero. + EXPECT_EQ(fsave.fcw, fxsave.fcw); + EXPECT_EQ(fsave.fsw, fxsave.fsw); + EXPECT_EQ(0xf0, fxsave.ftw); // FsaveToFxsaveTagWord + EXPECT_EQ(0, fxsave.reserved_1); + EXPECT_EQ(fsave.fop, fxsave.fop); + EXPECT_EQ(fsave.fpu_ip, fxsave.fpu_ip); + EXPECT_EQ(fsave.fpu_cs, fxsave.fpu_cs); + EXPECT_EQ(0, fxsave.reserved_2); + EXPECT_EQ(fsave.fpu_dp, fxsave.fpu_dp); + EXPECT_EQ(fsave.fpu_ds, fxsave.fpu_ds); + EXPECT_EQ(0, fxsave.reserved_3); + EXPECT_EQ(0u, fxsave.mxcsr); + EXPECT_EQ(0u, fxsave.mxcsr_mask); + for (size_t index = 0; index < arraysize(fxsave.st_mm); ++index) { + EXPECT_EQ(BytesToHexString(fsave.st[index], arraysize(fsave.st[index])), + BytesToHexString(fxsave.st_mm[index].st, + arraysize(fxsave.st_mm[index].st))) + << "index " << index; + EXPECT_EQ(std::string(arraysize(fxsave.st_mm[index].st_reserved) * 2, '0'), + BytesToHexString(fxsave.st_mm[index].st_reserved, + arraysize(fxsave.st_mm[index].st_reserved))) + << "index " << index; + } + size_t unused_len = sizeof(fxsave) - offsetof(decltype(fxsave), xmm); + EXPECT_EQ(std::string(unused_len * 2, '0'), + BytesToHexString(fxsave.xmm, unused_len)); + + // Since the fsave format is a subset of the fxsave format, fsave-fxsave-fsave + // should round-trip cleanly. + CPUContextX86::Fsave fsave_2; + CPUContextX86::FxsaveToFsave(fxsave, &fsave_2); + + // Clear the reserved fields in the original fsave structure, since they’re + // expected to be clear in the copy. + fsave.reserved_1 = 0; + fsave.reserved_2 = 0; + fsave.reserved_3 = 0; + fsave.reserved_4 = 0; + EXPECT_EQ(0, memcmp(&fsave, &fsave_2, sizeof(fsave))); +} + TEST(CPUContextX86, FxsaveToFsaveTagWord) { // The fsave tag word uses bit pattern 00 for valid, 01 for zero, 10 for // “special”, and 11 for empty. Like the fxsave tag word, it is arranged by @@ -93,40 +245,52 @@ TEST(CPUContextX86, FxsaveToFsaveTagWord) { uint16_t fsw = 0 << 11; // top = 0: logical 0-7 maps to physical 0-7 uint8_t fxsave_tag = 0x0f; // physical 4-7 (logical 4-7) empty CPUContextX86::X87OrMMXRegister st_mm[8]; - SetX87Register(&st_mm[0], kExponentNormal, false, kFractionNormal); // spec. - SetX87Register(&st_mm[1], kExponentNormal, true, kFractionNormal); // valid - SetX87Register(&st_mm[2], kExponentNormal, false, kFractionAllZero); // spec. - SetX87Register(&st_mm[3], kExponentNormal, true, kFractionAllZero); // valid - SetX87Register(&st_mm[4], kExponentNormal, false, kFractionNormal); - SetX87Register(&st_mm[5], kExponentNormal, true, kFractionNormal); - SetX87Register(&st_mm[6], kExponentNormal, false, kFractionAllZero); - SetX87Register(&st_mm[7], kExponentNormal, true, kFractionAllZero); + SetX87OrMMXRegister( + &st_mm[0], kExponentNormal, false, kFractionNormal); // spec. + SetX87OrMMXRegister( + &st_mm[1], kExponentNormal, true, kFractionNormal); // valid + SetX87OrMMXRegister( + &st_mm[2], kExponentNormal, false, kFractionAllZero); // spec. + SetX87OrMMXRegister( + &st_mm[3], kExponentNormal, true, kFractionAllZero); // valid + SetX87OrMMXRegister(&st_mm[4], kExponentNormal, false, kFractionNormal); + SetX87OrMMXRegister(&st_mm[5], kExponentNormal, true, kFractionNormal); + SetX87OrMMXRegister(&st_mm[6], kExponentNormal, false, kFractionAllZero); + SetX87OrMMXRegister(&st_mm[7], kExponentNormal, true, kFractionAllZero); EXPECT_EQ(0xff22, CPUContextX86::FxsaveToFsaveTagWord(fsw, fxsave_tag, st_mm)); fsw = 2 << 11; // top = 2: logical 0-7 maps to physical 2-7, 0-1 fxsave_tag = 0xf0; // physical 0-3 (logical 6-7, 0-1) empty - SetX87Register(&st_mm[0], kExponentAllZero, false, kFractionNormal); - SetX87Register(&st_mm[1], kExponentAllZero, true, kFractionNormal); - SetX87Register(&st_mm[2], kExponentAllZero, false, kFractionAllZero); // zero - SetX87Register(&st_mm[3], kExponentAllZero, true, kFractionAllZero); // spec. - SetX87Register(&st_mm[4], kExponentAllZero, false, kFractionNormal); // spec. - SetX87Register(&st_mm[5], kExponentAllZero, true, kFractionNormal); // spec. - SetX87Register(&st_mm[6], kExponentAllZero, false, kFractionAllZero); - SetX87Register(&st_mm[7], kExponentAllZero, true, kFractionAllZero); + SetX87OrMMXRegister(&st_mm[0], kExponentAllZero, false, kFractionNormal); + SetX87OrMMXRegister(&st_mm[1], kExponentAllZero, true, kFractionNormal); + SetX87OrMMXRegister( + &st_mm[2], kExponentAllZero, false, kFractionAllZero); // zero + SetX87OrMMXRegister( + &st_mm[3], kExponentAllZero, true, kFractionAllZero); // spec. + SetX87OrMMXRegister( + &st_mm[4], kExponentAllZero, false, kFractionNormal); // spec. + SetX87OrMMXRegister( + &st_mm[5], kExponentAllZero, true, kFractionNormal); // spec. + SetX87OrMMXRegister(&st_mm[6], kExponentAllZero, false, kFractionAllZero); + SetX87OrMMXRegister(&st_mm[7], kExponentAllZero, true, kFractionAllZero); EXPECT_EQ(0xa9ff, CPUContextX86::FxsaveToFsaveTagWord(fsw, fxsave_tag, st_mm)); fsw = 5 << 11; // top = 5: logical 0-7 maps to physical 5-7, 0-4 fxsave_tag = 0x5a; // physical 0, 2, 5, and 7 (logical 5, 0, 2, and 3) empty - SetX87Register(&st_mm[0], kExponentAllOne, false, kFractionNormal); - SetX87Register(&st_mm[1], kExponentAllOne, true, kFractionNormal); // spec. - SetX87Register(&st_mm[2], kExponentAllOne, false, kFractionAllZero); - SetX87Register(&st_mm[3], kExponentAllOne, true, kFractionAllZero); - SetX87Register(&st_mm[4], kExponentAllOne, false, kFractionNormal); // spec. - SetX87Register(&st_mm[5], kExponentAllOne, true, kFractionNormal); - SetX87Register(&st_mm[6], kExponentAllOne, false, kFractionAllZero); // spec. - SetX87Register(&st_mm[7], kExponentAllOne, true, kFractionAllZero); // spec. + SetX87OrMMXRegister(&st_mm[0], kExponentAllOne, false, kFractionNormal); + SetX87OrMMXRegister( + &st_mm[1], kExponentAllOne, true, kFractionNormal); // spec. + SetX87OrMMXRegister(&st_mm[2], kExponentAllOne, false, kFractionAllZero); + SetX87OrMMXRegister(&st_mm[3], kExponentAllOne, true, kFractionAllZero); + SetX87OrMMXRegister( + &st_mm[4], kExponentAllOne, false, kFractionNormal); // spec. + SetX87OrMMXRegister(&st_mm[5], kExponentAllOne, true, kFractionNormal); + SetX87OrMMXRegister( + &st_mm[6], kExponentAllOne, false, kFractionAllZero); // spec. + SetX87OrMMXRegister( + &st_mm[7], kExponentAllOne, true, kFractionAllZero); // spec. EXPECT_EQ(0xeebb, CPUContextX86::FxsaveToFsaveTagWord(fsw, fxsave_tag, st_mm)); @@ -134,14 +298,19 @@ TEST(CPUContextX86, FxsaveToFsaveTagWord) { // register file. fsw = 1 << 11; // top = 1: logical 0-7 maps to physical 1-7, 0 fxsave_tag = 0x1f; // physical 5-7 (logical 4-6) empty - SetX87Register(&st_mm[0], kExponentNormal, true, kFractionAllZero); // valid - SetX87Register(&st_mm[1], kExponentAllZero, false, kFractionAllZero); // zero - SetX87Register(&st_mm[2], kExponentAllOne, true, kFractionAllZero); // spec. - SetX87Register(&st_mm[3], kExponentAllOne, true, kFractionNormal); // spec. - SetX87Register(&st_mm[4], kExponentAllZero, false, kFractionAllZero); - SetX87Register(&st_mm[5], kExponentAllZero, false, kFractionAllZero); - SetX87Register(&st_mm[6], kExponentAllZero, false, kFractionAllZero); - SetX87Register(&st_mm[7], kExponentNormal, true, kFractionNormal); // valid + SetX87OrMMXRegister( + &st_mm[0], kExponentNormal, true, kFractionAllZero); // valid + SetX87OrMMXRegister( + &st_mm[1], kExponentAllZero, false, kFractionAllZero); // zero + SetX87OrMMXRegister( + &st_mm[2], kExponentAllOne, true, kFractionAllZero); // spec. + SetX87OrMMXRegister( + &st_mm[3], kExponentAllOne, true, kFractionNormal); // spec. + SetX87OrMMXRegister(&st_mm[4], kExponentAllZero, false, kFractionAllZero); + SetX87OrMMXRegister(&st_mm[5], kExponentAllZero, false, kFractionAllZero); + SetX87OrMMXRegister(&st_mm[6], kExponentAllZero, false, kFractionAllZero); + SetX87OrMMXRegister( + &st_mm[7], kExponentNormal, true, kFractionNormal); // valid EXPECT_EQ(0xfe90, CPUContextX86::FxsaveToFsaveTagWord(fsw, fxsave_tag, st_mm)); @@ -149,7 +318,7 @@ TEST(CPUContextX86, FxsaveToFsaveTagWord) { fsw = 0 << 11; // top = 0: logical 0-7 maps to physical 0-7 fxsave_tag = 0xff; // nothing empty for (size_t index = 0; index < arraysize(st_mm); ++index) { - SetX87Register(&st_mm[index], kExponentNormal, true, kFractionAllZero); + SetX87OrMMXRegister(&st_mm[index], kExponentNormal, true, kFractionAllZero); } EXPECT_EQ(0, CPUContextX86::FxsaveToFsaveTagWord(fsw, fxsave_tag, st_mm)); @@ -168,6 +337,7 @@ TEST(CPUContextX86, FsaveToFxsaveTagWord) { EXPECT_EQ(0xf0, CPUContextX86::FsaveToFxsaveTagWord(0xa9ff)); EXPECT_EQ(0x5a, CPUContextX86::FsaveToFxsaveTagWord(0xeebb)); EXPECT_EQ(0x1f, CPUContextX86::FsaveToFxsaveTagWord(0xfe90)); + EXPECT_EQ(0xff, CPUContextX86::FsaveToFxsaveTagWord(0x0000)); EXPECT_EQ(0x00, CPUContextX86::FsaveToFxsaveTagWord(0xffff)); } diff --git a/snapshot/win/cpu_context_win.cc b/snapshot/win/cpu_context_win.cc index 900fbda8..cc66e1cb 100644 --- a/snapshot/win/cpu_context_win.cc +++ b/snapshot/win/cpu_context_win.cc @@ -24,6 +24,16 @@ namespace crashpad { namespace { +// Validation for casts used with CPUContextX86::FsaveToFxsave(). +static_assert(sizeof(CPUContextX86::Fsave) == + offsetof(WOW64_FLOATING_SAVE_AREA, Cr0NpxState), + "WoW64 fsave types must be equivalent"); +#if defined(ARCH_CPU_X86) +static_assert(sizeof(CPUContextX86::Fsave) == + offsetof(FLOATING_SAVE_AREA, Spare0), + "fsave types must be equivalent"); +#endif // ARCH_CPU_X86 + template bool HasContextPart(const T& context, uint32_t bits) { return (context.ContextFlags & bits) == bits; @@ -90,36 +100,26 @@ void CommonInitializeX86Context(const T& context, CPUContextX86* out) { out->dr1 = context.Dr1; out->dr2 = context.Dr2; out->dr3 = context.Dr3; + // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see // https://en.wikipedia.org/wiki/X86_debug_register. out->dr4 = context.Dr6; out->dr5 = context.Dr7; + out->dr6 = context.Dr6; out->dr7 = context.Dr7; } if (HasContextPart(context, WOW64_CONTEXT_EXTENDED_REGISTERS)) { static_assert(sizeof(out->fxsave) == sizeof(context.ExtendedRegisters), - "types must be equivalent"); + "fxsave types must be equivalent"); memcpy(&out->fxsave, &context.ExtendedRegisters, sizeof(out->fxsave)); } else if (HasContextPart(context, WOW64_CONTEXT_FLOATING_POINT)) { - out->fxsave.fcw = static_cast(context.FloatSave.ControlWord); - out->fxsave.fsw = static_cast(context.FloatSave.StatusWord); - out->fxsave.ftw = CPUContextX86::FsaveToFxsaveTagWord( - static_cast(context.FloatSave.TagWord)); - out->fxsave.fop = context.FloatSave.ErrorSelector >> 16; - out->fxsave.fpu_ip = context.FloatSave.ErrorOffset; - out->fxsave.fpu_cs = static_cast(context.FloatSave.ErrorSelector); - out->fxsave.fpu_dp = context.FloatSave.DataOffset; - out->fxsave.fpu_ds = static_cast(context.FloatSave.DataSelector); - const CPUContextX86::X87Register* context_floatsave_st = - reinterpret_cast( - context.FloatSave.RegisterArea); - for (size_t index = 0; index < arraysize(out->fxsave.st_mm); ++index) { - memcpy(out->fxsave.st_mm[index].st, - context_floatsave_st[index], - sizeof(out->fxsave.st_mm[index].st)); - } + // The static_assert that validates this cast can’t be here because it + // relies on field names that vary based on the template parameter. + CPUContextX86::FsaveToFxsave( + *reinterpret_cast(&context.FloatSave), + &out->fxsave); } } @@ -174,10 +174,12 @@ void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { out->dr1 = context.Dr1; out->dr2 = context.Dr2; out->dr3 = context.Dr3; + // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see // https://en.wikipedia.org/wiki/X86_debug_register. out->dr4 = context.Dr6; out->dr5 = context.Dr7; + out->dr6 = context.Dr6; out->dr7 = context.Dr7; } @@ -185,7 +187,7 @@ void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { if (HasContextPart(context, CONTEXT_FLOATING_POINT)) { static_assert(sizeof(out->fxsave) == sizeof(context.FltSave), "types must be equivalent"); - memcpy(&out->fxsave, &context.FltSave.ControlWord, sizeof(out->fxsave)); + memcpy(&out->fxsave, &context.FltSave, sizeof(out->fxsave)); } } diff --git a/snapshot/win/cpu_context_win_test.cc b/snapshot/win/cpu_context_win_test.cc index 1f8bfe98..b2e6aae5 100644 --- a/snapshot/win/cpu_context_win_test.cc +++ b/snapshot/win/cpu_context_win_test.cc @@ -19,6 +19,7 @@ #include "base/macros.h" #include "build/build_config.h" #include "gtest/gtest.h" +#include "test/hex_string.h" #include "snapshot/cpu_context.h" namespace crashpad { @@ -84,18 +85,16 @@ void TestInitializeX86Context_FsaveWithoutFxsave() { EXPECT_EQ(0x89abcdef, cpu_context_x86.fxsave.fpu_dp); EXPECT_EQ(0x0007, cpu_context_x86.fxsave.fpu_ds); for (size_t st_mm = 0; st_mm < 7; ++st_mm) { - for (size_t byte = 0; - byte < arraysize(cpu_context_x86.fxsave.st_mm[st_mm].st); - ++byte) { - EXPECT_EQ(0x00, cpu_context_x86.fxsave.st_mm[st_mm].st[byte]); - } + EXPECT_EQ( + std::string(arraysize(cpu_context_x86.fxsave.st_mm[st_mm].st) * 2, + '0'), + BytesToHexString(cpu_context_x86.fxsave.st_mm[st_mm].st, + arraysize(cpu_context_x86.fxsave.st_mm[st_mm].st))) + << "st_mm " << st_mm; } - for (size_t byte = 0; byte < 7; ++byte) { - EXPECT_EQ(0x00, cpu_context_x86.fxsave.st_mm[7].st[byte]); - } - EXPECT_EQ(0x80, cpu_context_x86.fxsave.st_mm[7].st[7]); - EXPECT_EQ(0xff, cpu_context_x86.fxsave.st_mm[7].st[8]); - EXPECT_EQ(0x7f, cpu_context_x86.fxsave.st_mm[7].st[9]); + EXPECT_EQ("0000000000000080ff7f", + BytesToHexString(cpu_context_x86.fxsave.st_mm[7].st, + arraysize(cpu_context_x86.fxsave.st_mm[7].st))); EXPECT_EQ(3u, cpu_context_x86.dr0); } diff --git a/test/hex_string.cc b/test/hex_string.cc new file mode 100644 index 00000000..bb9b5f3a --- /dev/null +++ b/test/hex_string.cc @@ -0,0 +1,35 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "test/hex_string.h" + +#include "base/strings/stringprintf.h" + +namespace crashpad { +namespace test { + +std::string BytesToHexString(const void* bytes, size_t length) { + const unsigned char* bytes_c = reinterpret_cast(bytes); + + std::string hex_string; + hex_string.reserve(length * 2); + for (size_t index = 0; index < length; ++index) { + hex_string.append(base::StringPrintf("%02x", bytes_c[index])); + } + + return hex_string; +} + +} // namespace test +} // namespace crashpad diff --git a/test/hex_string.h b/test/hex_string.h new file mode 100644 index 00000000..08654880 --- /dev/null +++ b/test/hex_string.h @@ -0,0 +1,40 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_TEST_HEX_STRING_H_ +#define CRASHPAD_TEST_HEX_STRING_H_ + +#include + +#include + +namespace crashpad { +namespace test { + +//! \brief Returns a hexadecimal string corresponding to \a bytes and \a length. +//! +//! Example usage: +//! \code +//! uint8_t expected[10]; +//! uint8_t observed[10]; +//! // … +//! EXPECT_EQ(BytesToHexString(expected, arraysize(expected)), +//! BytesToHexString(observed, arraysize(observed))); +//! \endcode +std::string BytesToHexString(const void* bytes, size_t length); + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_TEST_HEX_STRING_H_ diff --git a/test/hex_string_test.cc b/test/hex_string_test.cc new file mode 100644 index 00000000..62b45c12 --- /dev/null +++ b/test/hex_string_test.cc @@ -0,0 +1,34 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "test/hex_string.h" + +#include "base/macros.h" +#include "gtest/gtest.h" + +namespace crashpad { +namespace test { +namespace { + +TEST(HexString, HexString) { + EXPECT_EQ("", BytesToHexString(nullptr, 0)); + + const char kBytes[] = "Abc123xyz \x0a\x7f\xf0\x9f\x92\xa9_"; + EXPECT_EQ("41626331323378797a200a7ff09f92a95f00", + BytesToHexString(kBytes, arraysize(kBytes))); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/test/test.gyp b/test/test.gyp index 666a68a1..d84ff640 100644 --- a/test/test.gyp +++ b/test/test.gyp @@ -35,6 +35,8 @@ 'file.cc', 'file.h', 'gtest_death_check.h', + 'hex_string.cc', + 'hex_string.h', 'mac/dyld.h', 'mac/mach_errors.cc', 'mac/mach_errors.h', diff --git a/test/test_test.gyp b/test/test_test.gyp index e4405fcc..2c58c170 100644 --- a/test/test_test.gyp +++ b/test/test_test.gyp @@ -34,6 +34,7 @@ '..', ], 'sources': [ + 'hex_string_test.cc', 'mac/mach_multiprocess_test.cc', 'multiprocess_exec_test.cc', 'multiprocess_posix_test.cc', From 4c6f6e52e2d7d4459fb03c352e90023615351352 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Thu, 16 Feb 2017 13:26:54 -0500 Subject: [PATCH 06/10] Remove vestigial support for in-Chromium GYP build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chromium’s GYP build has been removed. The support added to Crashpad’s GYP build to integrate with Chromium’s is now unused and unnecessary. Chromium builds Crashpad as part of its own GN build. https://groups.google.com/a/chromium.org/d/topic/chromium-dev/NZkPr-CXvQ0 Change-Id: I30f2d3453f4476037c9afe0714a780456f0bbcd6 Reviewed-on: https://chromium-review.googlesource.com/444044 Reviewed-by: Robert Sesek --- build/crashpad_dependencies.gypi | 22 +- build/gyp_crashpad.py | 11 +- handler/handler.gyp | 24 - third_party/gtest/gmock.gyp | 362 +++++++------- third_party/gtest/gtest.gyp | 498 ++++++++++---------- third_party/mini_chromium/mini_chromium.gyp | 17 +- 6 files changed, 428 insertions(+), 506 deletions(-) diff --git a/build/crashpad_dependencies.gypi b/build/crashpad_dependencies.gypi index 417d462b..7fd1cf66 100644 --- a/build/crashpad_dependencies.gypi +++ b/build/crashpad_dependencies.gypi @@ -13,16 +13,12 @@ # limitations under the License. { - # Crashpad can obtain dependencies in three different ways, directed by the - # crashpad_standalone GYP variable. It may have these values: + # Crashpad’s GYP build can obtain dependencies in two different ways, directed + # by the crashpad_standalone GYP variable. It may have these values: # standalone # A “standalone” Crashpad build, where the dependencies are in the # Crashpad tree. third_party/mini_chromium and third_party/gtest provide # the base and gtest libraries. - # chromium - # An in-Chromium build, where Crashpad is within the Chromium tree. - # Chromium provides its own base library and its copy of the gtest - # library. # external # A build with external dependencies. mini_chromium provides the base # library, but it’s located outside of the Crashpad tree, as is gtest. @@ -30,13 +26,15 @@ # In order for Crashpad’s .gyp files to reference the correct versions # depending on how dependencies are being provided, include this .gypi file # and reference the crashpad_dependencies variable. + # + # Note that Crashpad’s in-Chromium build uses GN instead of GYP, and + # Chromium’s GN build configures Crashpad to use Chromium’s own base library + # and its copy of the gtest library. 'variables': { - # When building as a standalone project or with external dependencies, - # build/gyp_crashpad.py sets crashpad_dependencies to "standalone" or - # "external", and this % assignment will not override it. The variable will - # not be set by anything else when building as part of Chromium, so in that - # case, this will define it with value "chromium". - 'crashpad_dependencies%': 'chromium', + # When with external dependencies, build/gyp_crashpad.py sets + # crashpad_dependencies to "external", and this % assignment will not + # override it. + 'crashpad_dependencies%': 'standalone', }, } diff --git a/build/gyp_crashpad.py b/build/gyp_crashpad.py index 741c8126..fe9da840 100755 --- a/build/gyp_crashpad.py +++ b/build/gyp_crashpad.py @@ -31,12 +31,12 @@ def ChooseDependencyPath(local_path, external_path): external_path: The external path to fall back to. Returns: - A 2-tuple. The first element is 'standalone' or 'external', depending on - whether local_path or external_path was chosen. The second element is the - chosen path. + A 2-tuple. The first element is None or 'external', depending on whether + local_path or external_path was chosen. The second element is the chosen + path. """ if os.path.exists(local_path) or not os.path.exists(external_path): - return ('standalone', local_path) + return (None, local_path) return ('external', external_path) @@ -64,7 +64,8 @@ def main(args): 'mini_chromium', 'build', 'common.gypi'), os.path.join(crashpad_dir, os.pardir, os.pardir, 'mini_chromium', 'mini_chromium', 'build', 'common.gypi'))) - args.extend(['-D', 'crashpad_dependencies=%s' % dependencies]) + if dependencies is not None: + args.extend(['-D', 'crashpad_dependencies=%s' % dependencies]) args.extend(['--include', mini_chromium_dir]) args.extend(['--depth', crashpad_dir_or_dot]) args.append(os.path.join(crashpad_dir, 'crashpad.gyp')) diff --git a/handler/handler.gyp b/handler/handler.gyp index e2d993df..6a51222c 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -15,7 +15,6 @@ { 'includes': [ '../build/crashpad.gypi', - '../build/crashpad_dependencies.gypi', ], 'targets': [ { @@ -66,29 +65,6 @@ ], 'conditions': [ - ['OS=="mac"', { - # In an in-Chromium build with component=shared_library, - # crashpad_handler will depend on shared libraries such as - # libbase.dylib located in out/{Debug,Release} via the @rpath - # mechanism. When crashpad_handler is copied to its home deep inside - # the Chromium app bundle, it needs to have an LC_RPATH command - # pointing back to the directory containing these dependency - # libraries. - 'variables': { - 'component%': 'static_library', - }, - 'conditions': [ - ['crashpad_dependencies=="chromium" and component=="shared_library"', { - 'xcode_settings': { - 'LD_RUNPATH_SEARCH_PATHS': [ # -Wl,-rpath - # Get back from - # Chromium.app/Contents/Versions/V/Framework.framework/Helpers - '@loader_path/../../../../../..', - ], - }, - }], - ], - }], ['OS=="win"', { 'msvs_settings': { 'VCLinkerTool': { diff --git a/third_party/gtest/gmock.gyp b/third_party/gtest/gmock.gyp index 683b89e1..466de6a9 100644 --- a/third_party/gtest/gmock.gyp +++ b/third_party/gtest/gmock.gyp @@ -17,212 +17,190 @@ '../../build/crashpad_dependencies.gypi', ], 'conditions': [ - ['crashpad_dependencies!="chromium"', { + ['1==1', { # Defer processing until crashpad_dependencies is set 'variables': { 'conditions': [ ['crashpad_dependencies=="standalone"', { 'gmock_dir': 'gtest/googlemock', - }, { + }], + ['crashpad_dependencies=="external"', { 'gmock_dir': '../../../../gmock', }], ], }, - 'target_defaults': { - # gmock relies heavily on objects with static storage duration. - 'xcode_settings': { - 'WARNING_CFLAGS!': [ - '-Wexit-time-destructors', - ], - }, - 'cflags!': [ - '-Wexit-time-destructors', + }], + ], + 'target_defaults': { + # gmock relies heavily on objects with static storage duration. + 'xcode_settings': { + 'WARNING_CFLAGS!': [ + '-Wexit-time-destructors', + ], + }, + 'cflags!': [ + '-Wexit-time-destructors', + ], + }, + + 'targets': [ + { + 'target_name': 'gmock', + 'type': 'static_library', + 'dependencies': [ + 'gtest.gyp:gtest', + ], + 'include_dirs': [ + '<(gmock_dir)', + '<(gmock_dir)/include', + ], + 'sources': [ + '<(gmock_dir)/include/gmock/gmock-actions.h', + '<(gmock_dir)/include/gmock/gmock-cardinalities.h', + '<(gmock_dir)/include/gmock/gmock-generated-actions.h', + '<(gmock_dir)/include/gmock/gmock-generated-function-mockers.h', + '<(gmock_dir)/include/gmock/gmock-generated-matchers.h', + '<(gmock_dir)/include/gmock/gmock-generated-nice-strict.h', + '<(gmock_dir)/include/gmock/gmock-matchers.h', + '<(gmock_dir)/include/gmock/gmock-more-actions.h', + '<(gmock_dir)/include/gmock/gmock-more-matchers.h', + '<(gmock_dir)/include/gmock/gmock-spec-builders.h', + '<(gmock_dir)/include/gmock/gmock.h', + '<(gmock_dir)/include/gmock/internal/custom/gmock-generated-actions.h', + '<(gmock_dir)/include/gmock/internal/custom/gmock-matchers.h', + '<(gmock_dir)/include/gmock/internal/custom/gmock-port.h', + '<(gmock_dir)/include/gmock/internal/gmock-generated-internal-utils.h', + '<(gmock_dir)/include/gmock/internal/gmock-internal-utils.h', + '<(gmock_dir)/include/gmock/internal/gmock-port.h', + '<(gmock_dir)/src/gmock-all.cc', + '<(gmock_dir)/src/gmock-cardinalities.cc', + '<(gmock_dir)/src/gmock-internal-utils.cc', + '<(gmock_dir)/src/gmock-matchers.cc', + '<(gmock_dir)/src/gmock-spec-builders.cc', + '<(gmock_dir)/src/gmock.cc', + ], + 'sources!': [ + '<(gmock_dir)/src/gmock-all.cc', + ], + + 'direct_dependent_settings': { + 'include_dirs': [ + '<(gmock_dir)/include', ], - }, - - 'targets': [ - { - 'target_name': 'gmock', - 'type': 'static_library', - 'dependencies': [ - 'gtest.gyp:gtest', - ], - 'include_dirs': [ - '<(gmock_dir)', - '<(gmock_dir)/include', - ], - 'sources': [ - '<(gmock_dir)/include/gmock/gmock-actions.h', - '<(gmock_dir)/include/gmock/gmock-cardinalities.h', - '<(gmock_dir)/include/gmock/gmock-generated-actions.h', - '<(gmock_dir)/include/gmock/gmock-generated-function-mockers.h', - '<(gmock_dir)/include/gmock/gmock-generated-matchers.h', - '<(gmock_dir)/include/gmock/gmock-generated-nice-strict.h', - '<(gmock_dir)/include/gmock/gmock-matchers.h', - '<(gmock_dir)/include/gmock/gmock-more-actions.h', - '<(gmock_dir)/include/gmock/gmock-more-matchers.h', - '<(gmock_dir)/include/gmock/gmock-spec-builders.h', - '<(gmock_dir)/include/gmock/gmock.h', - '<(gmock_dir)/include/gmock/internal/custom/gmock-generated-actions.h', - '<(gmock_dir)/include/gmock/internal/custom/gmock-matchers.h', - '<(gmock_dir)/include/gmock/internal/custom/gmock-port.h', - '<(gmock_dir)/include/gmock/internal/gmock-generated-internal-utils.h', - '<(gmock_dir)/include/gmock/internal/gmock-internal-utils.h', - '<(gmock_dir)/include/gmock/internal/gmock-port.h', - '<(gmock_dir)/src/gmock-all.cc', - '<(gmock_dir)/src/gmock-cardinalities.cc', - '<(gmock_dir)/src/gmock-internal-utils.cc', - '<(gmock_dir)/src/gmock-matchers.cc', - '<(gmock_dir)/src/gmock-spec-builders.cc', - '<(gmock_dir)/src/gmock.cc', - ], - 'sources!': [ - '<(gmock_dir)/src/gmock-all.cc', - ], - - 'direct_dependent_settings': { - 'include_dirs': [ - '<(gmock_dir)/include', - ], + 'conditions': [ + ['clang!=0', { + # The MOCK_METHODn() macros do not specify “override”, which + # triggers this warning in users: “error: 'Method' overrides a + # member function but is not marked 'override' + # [-Werror,-Winconsistent-missing-override]”. Suppress these + # warnings, and add -Wno-unknown-warning-option because only + # recent versions of clang (trunk r220703 and later, version + # 3.6 and later) recognize it. 'conditions': [ - ['clang!=0', { - # The MOCK_METHODn() macros do not specify “override”, which - # triggers this warning in users: “error: 'Method' overrides a - # member function but is not marked 'override' - # [-Werror,-Winconsistent-missing-override]”. Suppress these - # warnings, and add -Wno-unknown-warning-option because only - # recent versions of clang (trunk r220703 and later, version - # 3.6 and later) recognize it. - 'conditions': [ - ['OS=="mac"', { - 'xcode_settings': { - 'WARNING_CFLAGS': [ - '-Wno-inconsistent-missing-override', - '-Wno-unknown-warning-option', - ], - }, - }], - ['OS=="linux"', { - 'cflags': [ - '-Wno-inconsistent-missing-override', - '-Wno-unknown-warning-option', - ], - }], + ['OS=="mac"', { + 'xcode_settings': { + 'WARNING_CFLAGS': [ + '-Wno-inconsistent-missing-override', + '-Wno-unknown-warning-option', + ], + }, + }], + ['OS=="linux"', { + 'cflags': [ + '-Wno-inconsistent-missing-override', + '-Wno-unknown-warning-option', ], }], ], - }, - 'export_dependent_settings': [ - 'gtest.gyp:gtest', - ], - }, - { - 'target_name': 'gmock_main', - 'type': 'static_library', - 'dependencies': [ - 'gmock', - 'gtest.gyp:gtest', - ], - 'sources': [ - '<(gmock_dir)/src/gmock_main.cc', - ], - }, - { - 'target_name': 'gmock_test_executable', - 'type': 'none', - 'dependencies': [ - 'gmock', - 'gtest.gyp:gtest', - ], - 'direct_dependent_settings': { - 'type': 'executable', - 'include_dirs': [ - '<(gmock_dir)', - ], - }, - 'export_dependent_settings': [ - 'gmock', - 'gtest.gyp:gtest', - ], - }, - { - 'target_name': 'gmock_all_test', - 'dependencies': [ - 'gmock_test_executable', - 'gmock_main', - ], - 'include_dirs': [ - 'gtest/googletest', - ], - 'sources': [ - '<(gmock_dir)/test/gmock-actions_test.cc', - '<(gmock_dir)/test/gmock-cardinalities_test.cc', - '<(gmock_dir)/test/gmock-generated-actions_test.cc', - '<(gmock_dir)/test/gmock-generated-function-mockers_test.cc', - '<(gmock_dir)/test/gmock-generated-internal-utils_test.cc', - '<(gmock_dir)/test/gmock-generated-matchers_test.cc', - '<(gmock_dir)/test/gmock-internal-utils_test.cc', - '<(gmock_dir)/test/gmock-matchers_test.cc', - '<(gmock_dir)/test/gmock-more-actions_test.cc', - '<(gmock_dir)/test/gmock-nice-strict_test.cc', - '<(gmock_dir)/test/gmock-port_test.cc', - '<(gmock_dir)/test/gmock-spec-builders_test.cc', - '<(gmock_dir)/test/gmock_test.cc', - ], - }, - { - 'target_name': 'gmock_link_test', - 'dependencies': [ - 'gmock_test_executable', - 'gmock_main', - ], - 'sources': [ - '<(gmock_dir)/test/gmock_link_test.cc', - '<(gmock_dir)/test/gmock_link_test.h', - '<(gmock_dir)/test/gmock_link2_test.cc', - ], - }, - { - 'target_name': 'gmock_stress_test', - 'dependencies': [ - 'gmock_test_executable', - ], - 'sources': [ - '<(gmock_dir)/test/gmock_stress_test.cc', - ], - }, - { - 'target_name': 'gmock_all_tests', - 'type': 'none', - 'dependencies': [ - 'gmock_all_test', - 'gmock_link_test', - 'gmock_stress_test', - ], - }, + }], + ], + }, + 'export_dependent_settings': [ + 'gtest.gyp:gtest', ], - }, { # else: crashpad_dependencies=="chromium" - 'targets': [ - { - 'target_name': 'gmock', - 'type': 'none', - 'dependencies': [ - '<(DEPTH)/testing/gmock.gyp:gmock', - ], - 'export_dependent_settings': [ - '<(DEPTH)/testing/gmock.gyp:gmock', - ], - }, - { - 'target_name': 'gmock_main', - 'type': 'none', - 'dependencies': [ - '<(DEPTH)/testing/gmock.gyp:gmock_main', - ], - 'export_dependent_settings': [ - '<(DEPTH)/testing/gmock.gyp:gmock_main', - ], - }, + }, + { + 'target_name': 'gmock_main', + 'type': 'static_library', + 'dependencies': [ + 'gmock', + 'gtest.gyp:gtest', ], - }], + 'sources': [ + '<(gmock_dir)/src/gmock_main.cc', + ], + }, + { + 'target_name': 'gmock_test_executable', + 'type': 'none', + 'dependencies': [ + 'gmock', + 'gtest.gyp:gtest', + ], + 'direct_dependent_settings': { + 'type': 'executable', + 'include_dirs': [ + '<(gmock_dir)', + ], + }, + 'export_dependent_settings': [ + 'gmock', + 'gtest.gyp:gtest', + ], + }, + { + 'target_name': 'gmock_all_test', + 'dependencies': [ + 'gmock_test_executable', + 'gmock_main', + ], + 'include_dirs': [ + 'gtest/googletest', + ], + 'sources': [ + '<(gmock_dir)/test/gmock-actions_test.cc', + '<(gmock_dir)/test/gmock-cardinalities_test.cc', + '<(gmock_dir)/test/gmock-generated-actions_test.cc', + '<(gmock_dir)/test/gmock-generated-function-mockers_test.cc', + '<(gmock_dir)/test/gmock-generated-internal-utils_test.cc', + '<(gmock_dir)/test/gmock-generated-matchers_test.cc', + '<(gmock_dir)/test/gmock-internal-utils_test.cc', + '<(gmock_dir)/test/gmock-matchers_test.cc', + '<(gmock_dir)/test/gmock-more-actions_test.cc', + '<(gmock_dir)/test/gmock-nice-strict_test.cc', + '<(gmock_dir)/test/gmock-port_test.cc', + '<(gmock_dir)/test/gmock-spec-builders_test.cc', + '<(gmock_dir)/test/gmock_test.cc', + ], + }, + { + 'target_name': 'gmock_link_test', + 'dependencies': [ + 'gmock_test_executable', + 'gmock_main', + ], + 'sources': [ + '<(gmock_dir)/test/gmock_link_test.cc', + '<(gmock_dir)/test/gmock_link_test.h', + '<(gmock_dir)/test/gmock_link2_test.cc', + ], + }, + { + 'target_name': 'gmock_stress_test', + 'dependencies': [ + 'gmock_test_executable', + ], + 'sources': [ + '<(gmock_dir)/test/gmock_stress_test.cc', + ], + }, + { + 'target_name': 'gmock_all_tests', + 'type': 'none', + 'dependencies': [ + 'gmock_all_test', + 'gmock_link_test', + 'gmock_stress_test', + ], + }, ], } diff --git a/third_party/gtest/gtest.gyp b/third_party/gtest/gtest.gyp index e01adaf5..ad458936 100644 --- a/third_party/gtest/gtest.gyp +++ b/third_party/gtest/gtest.gyp @@ -17,274 +17,252 @@ '../../build/crashpad_dependencies.gypi', ], 'conditions': [ - ['crashpad_dependencies!="chromium"', { + ['1==1', { # Defer processing until crashpad_dependencies is set 'variables': { 'conditions': [ ['crashpad_dependencies=="standalone"', { 'gtest_dir': 'gtest/googletest', - }, { + }], + ['crashpad_dependencies=="external"', { 'gtest_dir': '../../../../gtest', }], ], }, - 'target_defaults': { - # gtest relies heavily on objects with static storage duration. - 'xcode_settings': { - 'WARNING_CFLAGS!': [ - '-Wexit-time-destructors', - ], - }, - 'cflags!': [ - '-Wexit-time-destructors', - ], - }, - - 'targets': [ - { - 'target_name': 'gtest', - 'type': 'static_library', - 'include_dirs': [ - '<(gtest_dir)', - '<(gtest_dir)/include', - ], - 'sources': [ - '<(gtest_dir)/include/gtest/gtest-death-test.h', - '<(gtest_dir)/include/gtest/gtest-message.h', - '<(gtest_dir)/include/gtest/gtest-param-test.h', - '<(gtest_dir)/include/gtest/gtest-printers.h', - '<(gtest_dir)/include/gtest/gtest-spi.h', - '<(gtest_dir)/include/gtest/gtest-test-part.h', - '<(gtest_dir)/include/gtest/gtest-typed-test.h', - '<(gtest_dir)/include/gtest/gtest.h', - '<(gtest_dir)/include/gtest/gtest_pred_impl.h', - '<(gtest_dir)/include/gtest/gtest_prod.h', - '<(gtest_dir)/include/gtest/internal/custom/gtest-port.h', - '<(gtest_dir)/include/gtest/internal/custom/gtest-printers.h', - '<(gtest_dir)/include/gtest/internal/custom/gtest.h', - '<(gtest_dir)/include/gtest/internal/gtest-death-test-internal.h', - '<(gtest_dir)/include/gtest/internal/gtest-filepath.h', - '<(gtest_dir)/include/gtest/internal/gtest-internal.h', - '<(gtest_dir)/include/gtest/internal/gtest-linked_ptr.h', - '<(gtest_dir)/include/gtest/internal/gtest-param-util-generated.h', - '<(gtest_dir)/include/gtest/internal/gtest-param-util.h', - '<(gtest_dir)/include/gtest/internal/gtest-port-arch.h', - '<(gtest_dir)/include/gtest/internal/gtest-port.h', - '<(gtest_dir)/include/gtest/internal/gtest-string.h', - '<(gtest_dir)/include/gtest/internal/gtest-tuple.h', - '<(gtest_dir)/include/gtest/internal/gtest-type-util.h', - '<(gtest_dir)/src/gtest-all.cc', - '<(gtest_dir)/src/gtest-death-test.cc', - '<(gtest_dir)/src/gtest-filepath.cc', - '<(gtest_dir)/src/gtest-internal-inl.h', - '<(gtest_dir)/src/gtest-port.cc', - '<(gtest_dir)/src/gtest-printers.cc', - '<(gtest_dir)/src/gtest-test-part.cc', - '<(gtest_dir)/src/gtest-typed-test.cc', - '<(gtest_dir)/src/gtest.cc', - ], - 'sources!': [ - '<(gtest_dir)/src/gtest-all.cc', - ], - 'direct_dependent_settings': { - 'include_dirs': [ - '<(gtest_dir)/include', - ], - }, - 'conditions': [ - ['crashpad_dependencies=="external"', { - 'include_dirs': [ - '<(gtest_dir)/../..', - ], - 'defines': [ - 'GUNIT_NO_GOOGLE3=1', - ], - 'direct_dependent_settings': { - 'include_dirs': [ - '<(gtest_dir)/../..', - ], - 'defines': [ - 'GUNIT_NO_GOOGLE3=1', - ], - }, - }], - ], - }, - { - 'target_name': 'gtest_main', - 'type': 'static_library', - 'dependencies': [ - 'gtest', - ], - 'sources': [ - '<(gtest_dir)/src/gtest_main.cc', - ], - }, - { - 'target_name': 'gtest_test_executable', - 'type': 'none', - 'dependencies': [ - 'gtest', - ], - 'direct_dependent_settings': { - 'type': 'executable', - 'include_dirs': [ - '<(gtest_dir)', - ], - }, - 'export_dependent_settings': [ - 'gtest', - ], - }, - { - 'target_name': 'gtest_all_test', - 'dependencies': [ - 'gtest_test_executable', - 'gtest_main', - ], - 'sources': [ - '<(gtest_dir)/test/gtest-death-test_test.cc', - '<(gtest_dir)/test/gtest-filepath_test.cc', - '<(gtest_dir)/test/gtest-linked_ptr_test.cc', - '<(gtest_dir)/test/gtest-message_test.cc', - '<(gtest_dir)/test/gtest-options_test.cc', - '<(gtest_dir)/test/gtest-port_test.cc', - '<(gtest_dir)/test/gtest-printers_test.cc', - '<(gtest_dir)/test/gtest-test-part_test.cc', - '<(gtest_dir)/test/gtest-typed-test2_test.cc', - '<(gtest_dir)/test/gtest-typed-test_test.cc', - '<(gtest_dir)/test/gtest-typed-test_test.h', - '<(gtest_dir)/test/gtest_main_unittest.cc', - '<(gtest_dir)/test/gtest_pred_impl_unittest.cc', - '<(gtest_dir)/test/gtest_prod_test.cc', - '<(gtest_dir)/test/gtest_unittest.cc', - '<(gtest_dir)/test/production.cc', - '<(gtest_dir)/test/production.h', - ], - }, - { - 'target_name': 'gtest_environment_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest_environment_test.cc', - ], - }, - { - 'target_name': 'gtest_listener_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest-listener_test.cc', - ], - }, - { - 'target_name': 'gtest_no_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest_no_test_unittest.cc', - ], - }, - { - 'target_name': 'gtest_param_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest-param-test2_test.cc', - '<(gtest_dir)/test/gtest-param-test_test.cc', - '<(gtest_dir)/test/gtest-param-test_test.h', - ], - }, - { - 'target_name': 'gtest_premature_exit_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest_premature_exit_test.cc', - ], - }, - { - 'target_name': 'gtest_repeat_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest_repeat_test.cc', - ], - }, - { - 'target_name': 'gtest_sole_header_test', - 'dependencies': [ - 'gtest_test_executable', - 'gtest_main', - ], - 'sources': [ - '<(gtest_dir)/test/gtest_sole_header_test.cc', - ], - }, - { - 'target_name': 'gtest_stress_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest_stress_test.cc', - ], - }, - { - 'target_name': 'gtest_unittest_api_test', - 'dependencies': [ - 'gtest_test_executable', - ], - 'sources': [ - '<(gtest_dir)/test/gtest-unittest-api_test.cc', - ], - }, - { - 'target_name': 'gtest_all_tests', - 'type': 'none', - 'dependencies': [ - 'gtest_all_test', - 'gtest_environment_test', - 'gtest_listener_test', - 'gtest_no_test', - 'gtest_param_test', - 'gtest_premature_exit_test', - 'gtest_repeat_test', - 'gtest_sole_header_test', - 'gtest_stress_test', - 'gtest_unittest_api_test', - ], - }, - ], - }, { # else: crashpad_dependencies=="chromium" - 'targets': [ - { - 'target_name': 'gtest', - 'type': 'none', - 'dependencies': [ - '<(DEPTH)/testing/gtest.gyp:gtest', - ], - 'export_dependent_settings': [ - '<(DEPTH)/testing/gtest.gyp:gtest', - ], - }, - { - 'target_name': 'gtest_main', - 'type': 'none', - 'dependencies': [ - '<(DEPTH)/testing/gtest.gyp:gtest_main', - ], - 'export_dependent_settings': [ - '<(DEPTH)/testing/gtest.gyp:gtest_main', - ], - }, - ], }], ], + 'target_defaults': { + # gtest relies heavily on objects with static storage duration. + 'xcode_settings': { + 'WARNING_CFLAGS!': [ + '-Wexit-time-destructors', + ], + }, + 'cflags!': [ + '-Wexit-time-destructors', + ], + }, + + 'targets': [ + { + 'target_name': 'gtest', + 'type': 'static_library', + 'include_dirs': [ + '<(gtest_dir)', + '<(gtest_dir)/include', + ], + 'sources': [ + '<(gtest_dir)/include/gtest/gtest-death-test.h', + '<(gtest_dir)/include/gtest/gtest-message.h', + '<(gtest_dir)/include/gtest/gtest-param-test.h', + '<(gtest_dir)/include/gtest/gtest-printers.h', + '<(gtest_dir)/include/gtest/gtest-spi.h', + '<(gtest_dir)/include/gtest/gtest-test-part.h', + '<(gtest_dir)/include/gtest/gtest-typed-test.h', + '<(gtest_dir)/include/gtest/gtest.h', + '<(gtest_dir)/include/gtest/gtest_pred_impl.h', + '<(gtest_dir)/include/gtest/gtest_prod.h', + '<(gtest_dir)/include/gtest/internal/custom/gtest-port.h', + '<(gtest_dir)/include/gtest/internal/custom/gtest-printers.h', + '<(gtest_dir)/include/gtest/internal/custom/gtest.h', + '<(gtest_dir)/include/gtest/internal/gtest-death-test-internal.h', + '<(gtest_dir)/include/gtest/internal/gtest-filepath.h', + '<(gtest_dir)/include/gtest/internal/gtest-internal.h', + '<(gtest_dir)/include/gtest/internal/gtest-linked_ptr.h', + '<(gtest_dir)/include/gtest/internal/gtest-param-util-generated.h', + '<(gtest_dir)/include/gtest/internal/gtest-param-util.h', + '<(gtest_dir)/include/gtest/internal/gtest-port-arch.h', + '<(gtest_dir)/include/gtest/internal/gtest-port.h', + '<(gtest_dir)/include/gtest/internal/gtest-string.h', + '<(gtest_dir)/include/gtest/internal/gtest-tuple.h', + '<(gtest_dir)/include/gtest/internal/gtest-type-util.h', + '<(gtest_dir)/src/gtest-all.cc', + '<(gtest_dir)/src/gtest-death-test.cc', + '<(gtest_dir)/src/gtest-filepath.cc', + '<(gtest_dir)/src/gtest-internal-inl.h', + '<(gtest_dir)/src/gtest-port.cc', + '<(gtest_dir)/src/gtest-printers.cc', + '<(gtest_dir)/src/gtest-test-part.cc', + '<(gtest_dir)/src/gtest-typed-test.cc', + '<(gtest_dir)/src/gtest.cc', + ], + 'sources!': [ + '<(gtest_dir)/src/gtest-all.cc', + ], + 'direct_dependent_settings': { + 'include_dirs': [ + '<(gtest_dir)/include', + ], + }, + 'conditions': [ + ['crashpad_dependencies=="external"', { + 'include_dirs': [ + '<(gtest_dir)/../..', + ], + 'defines': [ + 'GUNIT_NO_GOOGLE3=1', + ], + 'direct_dependent_settings': { + 'include_dirs': [ + '<(gtest_dir)/../..', + ], + 'defines': [ + 'GUNIT_NO_GOOGLE3=1', + ], + }, + }], + ], + }, + { + 'target_name': 'gtest_main', + 'type': 'static_library', + 'dependencies': [ + 'gtest', + ], + 'sources': [ + '<(gtest_dir)/src/gtest_main.cc', + ], + }, + { + 'target_name': 'gtest_test_executable', + 'type': 'none', + 'dependencies': [ + 'gtest', + ], + 'direct_dependent_settings': { + 'type': 'executable', + 'include_dirs': [ + '<(gtest_dir)', + ], + }, + 'export_dependent_settings': [ + 'gtest', + ], + }, + { + 'target_name': 'gtest_all_test', + 'dependencies': [ + 'gtest_test_executable', + 'gtest_main', + ], + 'sources': [ + '<(gtest_dir)/test/gtest-death-test_test.cc', + '<(gtest_dir)/test/gtest-filepath_test.cc', + '<(gtest_dir)/test/gtest-linked_ptr_test.cc', + '<(gtest_dir)/test/gtest-message_test.cc', + '<(gtest_dir)/test/gtest-options_test.cc', + '<(gtest_dir)/test/gtest-port_test.cc', + '<(gtest_dir)/test/gtest-printers_test.cc', + '<(gtest_dir)/test/gtest-test-part_test.cc', + '<(gtest_dir)/test/gtest-typed-test2_test.cc', + '<(gtest_dir)/test/gtest-typed-test_test.cc', + '<(gtest_dir)/test/gtest-typed-test_test.h', + '<(gtest_dir)/test/gtest_main_unittest.cc', + '<(gtest_dir)/test/gtest_pred_impl_unittest.cc', + '<(gtest_dir)/test/gtest_prod_test.cc', + '<(gtest_dir)/test/gtest_unittest.cc', + '<(gtest_dir)/test/production.cc', + '<(gtest_dir)/test/production.h', + ], + }, + { + 'target_name': 'gtest_environment_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest_environment_test.cc', + ], + }, + { + 'target_name': 'gtest_listener_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest-listener_test.cc', + ], + }, + { + 'target_name': 'gtest_no_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest_no_test_unittest.cc', + ], + }, + { + 'target_name': 'gtest_param_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest-param-test2_test.cc', + '<(gtest_dir)/test/gtest-param-test_test.cc', + '<(gtest_dir)/test/gtest-param-test_test.h', + ], + }, + { + 'target_name': 'gtest_premature_exit_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest_premature_exit_test.cc', + ], + }, + { + 'target_name': 'gtest_repeat_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest_repeat_test.cc', + ], + }, + { + 'target_name': 'gtest_sole_header_test', + 'dependencies': [ + 'gtest_test_executable', + 'gtest_main', + ], + 'sources': [ + '<(gtest_dir)/test/gtest_sole_header_test.cc', + ], + }, + { + 'target_name': 'gtest_stress_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest_stress_test.cc', + ], + }, + { + 'target_name': 'gtest_unittest_api_test', + 'dependencies': [ + 'gtest_test_executable', + ], + 'sources': [ + '<(gtest_dir)/test/gtest-unittest-api_test.cc', + ], + }, + { + 'target_name': 'gtest_all_tests', + 'type': 'none', + 'dependencies': [ + 'gtest_all_test', + 'gtest_environment_test', + 'gtest_listener_test', + 'gtest_no_test', + 'gtest_param_test', + 'gtest_premature_exit_test', + 'gtest_repeat_test', + 'gtest_sole_header_test', + 'gtest_stress_test', + 'gtest_unittest_api_test', + ], + }, + ], } diff --git a/third_party/mini_chromium/mini_chromium.gyp b/third_party/mini_chromium/mini_chromium.gyp index f063dd0a..e14a1fa5 100644 --- a/third_party/mini_chromium/mini_chromium.gyp +++ b/third_party/mini_chromium/mini_chromium.gyp @@ -18,11 +18,10 @@ ], 'targets': [ { - # To support Crashpad’s standalone build, its in-Chromium build, and its - # build depending on external libraries, Crashpad code depending on base - # should do so through this shim, which will either get base from - # mini_chromium, Chromium, or an external library depending on the build - # type. + # To support Crashpad’s standalone build and its build depending on + # external libraries, Crashpad code depending on base should do so through + # this shim, which will either get base from mini_chromium or an external + # library depending on the build type. 'target_name': 'base', 'type': 'none', 'conditions': [ @@ -34,14 +33,6 @@ 'mini_chromium/base/base.gyp:base', ], }], - ['crashpad_dependencies=="chromium"', { - 'dependencies': [ - '<(DEPTH)/base/base.gyp:base', - ], - 'export_dependent_settings': [ - '<(DEPTH)/base/base.gyp:base', - ], - }], ['crashpad_dependencies=="external"', { 'dependencies': [ '../../../../mini_chromium/mini_chromium/base/base.gyp:base', From f34ed66b93a0b42a58314af2784eee879a297ed5 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 22 Feb 2017 13:39:46 -0500 Subject: [PATCH 07/10] metrics: Record handler lifetime milestone events It could be useful to put our existing Crashpad.HandlerCrashed metrics into context by getting a sense of handler starts, clean exits, and other types of exits. BUG=crashpad:100 Change-Id: I8982075158ea6d210eb2ddad678302e339a42192 Reviewed-on: https://chromium-review.googlesource.com/444124 Reviewed-by: Scott Graham --- handler/handler_main.cc | 229 ++++++++++++++++++------- util/misc/metrics.cc | 8 + util/misc/metrics.h | 28 +++ util/thread/thread_posix.cc | 10 +- util/thread/thread_win.cc | 4 +- util/util.gyp | 3 + util/util_test.gyp | 2 + util/win/scoped_handle.cc | 2 +- util/win/session_end_watcher.cc | 243 +++++++++++++++++++++++++++ util/win/session_end_watcher.h | 79 +++++++++ util/win/session_end_watcher_test.cc | 62 +++++++ 11 files changed, 601 insertions(+), 69 deletions(-) create mode 100644 util/win/session_end_watcher.cc create mode 100644 util/win/session_end_watcher.h create mode 100644 util/win/session_end_watcher_test.cc diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 97322576..81d3de86 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -24,8 +24,10 @@ #include #include #include +#include #include "base/auto_reset.h" +#include "base/compiler_specific.h" #include "base/files/file_path.h" #include "base/files/scoped_file.h" #include "base/logging.h" @@ -64,6 +66,7 @@ #include "util/win/exception_handler_server.h" #include "util/win/handle.h" #include "util/win/initial_client_data.h" +#include "util/win/session_end_watcher.h" #endif // OS_MACOSX namespace crashpad { @@ -108,11 +111,65 @@ void Usage(const base::FilePath& me) { ToolSupport::UsageTail(me); } +// Calls Metrics::HandlerLifetimeMilestone, but only on the first call. This is +// to prevent multiple exit events from inadvertently being recorded, which +// might happen if a crash occurs during destruction in what would otherwise be +// a normal exit, or if a CallMetricsRecordNormalExit object is destroyed after +// something else logs an exit event. +void MetricsRecordExit(Metrics::LifetimeMilestone milestone) { + static bool once = [](Metrics::LifetimeMilestone milestone) { + Metrics::HandlerLifetimeMilestone(milestone); + return true; + }(milestone); + ALLOW_UNUSED_LOCAL(once); +} + +// Calls MetricsRecordExit() to record a failure, and returns EXIT_FAILURE for +// the convenience of callers in main() which can simply write “return +// ExitFailure();”. +int ExitFailure() { + MetricsRecordExit(Metrics::LifetimeMilestone::kFailed); + return EXIT_FAILURE; +} + +class CallMetricsRecordNormalExit { + public: + CallMetricsRecordNormalExit() {} + ~CallMetricsRecordNormalExit() { + MetricsRecordExit(Metrics::LifetimeMilestone::kExitedNormally); + } + + private: + DISALLOW_COPY_AND_ASSIGN(CallMetricsRecordNormalExit); +}; + #if defined(OS_MACOSX) -struct sigaction g_original_crash_sigaction[NSIG]; +void InstallSignalHandler(const std::vector& signals, + void (*handler)(int, siginfo_t*, void*)) { + struct sigaction sa = {}; + sigemptyset(&sa.sa_mask); + sa.sa_flags = SA_SIGINFO; + sa.sa_sigaction = handler; + + for (int sig : signals) { + int rv = sigaction(sig, &sa, nullptr); + PCHECK(rv == 0) << "sigaction " << sig; + } +} + +void RestoreDefaultSignalHandler(int sig) { + struct sigaction sa = {}; + sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + sa.sa_handler = SIG_DFL; + int rv = sigaction(sig, &sa, nullptr); + DPLOG_IF(ERROR, rv != 0) << "sigaction " << sig; +} void HandleCrashSignal(int sig, siginfo_t* siginfo, void* context) { + MetricsRecordExit(Metrics::LifetimeMilestone::kCrashed); + // Is siginfo->si_code useful? The only interesting values on macOS are 0 (not // useful, signals generated asynchronously such as by kill() or raise()) and // small positive numbers (useful, signal generated via a hardware fault). The @@ -144,22 +201,17 @@ void HandleCrashSignal(int sig, siginfo_t* siginfo, void* context) { } Metrics::HandlerCrashed(metrics_code); - // Restore the previous signal handler. - DCHECK_GT(sig, 0); - DCHECK_LT(static_cast(sig), arraysize(g_original_crash_sigaction)); - struct sigaction* osa = &g_original_crash_sigaction[sig]; - int rv = sigaction(sig, osa, nullptr); - DPLOG_IF(ERROR, rv != 0) << "sigaction " << sig; + RestoreDefaultSignalHandler(sig); // If the signal was received synchronously resulting from a hardware fault, // returning from the signal handler will cause the kernel to re-raise it, // because this handler hasn’t done anything to alleviate the condition that - // caused the signal to be raised in the first place. With the old signal - // handler in place (expected to be SIG_DFL), it will cause the same behavior - // to be taken as though this signal handler had never been installed at all - // (expected to be a crash). This is ideal, because the signal is re-raised - // with the same properties and from the same context that initially triggered - // it, providing the best debugging experience. + // caused the signal to be raised in the first place. With the default signal + // handler in place, it will cause the same behavior to be taken as though + // this signal handler had never been installed at all (expected to be a + // crash). This is ideal, because the signal is re-raised with the same + // properties and from the same context that initially triggered it, providing + // the best debugging experience. if ((sig != SIGILL && sig != SIGFPE && sig != SIGBUS && sig != SIGSEGV) || !si_code_valid) { @@ -167,8 +219,7 @@ void HandleCrashSignal(int sig, siginfo_t* siginfo, void* context) { // asynchronously via kill() and raise(), and those arising via hardware // traps such as int3 (resulting in SIGTRAP but advancing the instruction // pointer), will not reoccur on their own when returning from the signal - // handler. Re-raise them or call to the previous signal handler as - // appropriate. + // handler. Re-raise them. // // Unfortunately, when SIGBUS is received asynchronously via kill(), // siginfo->si_code makes it appear as though it was actually received via a @@ -181,49 +232,66 @@ void HandleCrashSignal(int sig, siginfo_t* siginfo, void* context) { // very valuable for debugging and are visible to a Mach exception handler. // Since SIGBUS is normally received synchronously in response to a hardware // fault, don’t sweat the unexpected asynchronous case. - if (osa->sa_handler == SIG_DFL) { - // Because this signal handler executes with the signal blocked, this - // raise() cannot immediately deliver the signal. Delivery is deferred - // until this signal handler returns and the signal becomes unblocked. The - // re-raised signal will appear with the same context as where it was - // initially triggered. - rv = raise(sig); - DPLOG_IF(ERROR, rv != 0) << "raise"; - } else if (osa->sa_handler != SIG_IGN) { - if (osa->sa_flags & SA_SIGINFO) { - osa->sa_sigaction(sig, siginfo, context); - } else { - osa->sa_handler(sig); - } - } + // + // Because this signal handler executes with the signal blocked, this + // raise() cannot immediately deliver the signal. Delivery is deferred until + // this signal handler returns and the signal becomes unblocked. The + // re-raised signal will appear with the same context as where it was + // initially triggered. + int rv = raise(sig); + DPLOG_IF(ERROR, rv != 0) << "raise"; } } -void InstallCrashHandler() { - struct sigaction sa = {}; - sigemptyset(&sa.sa_mask); - sa.sa_flags = SA_SIGINFO; - sa.sa_sigaction = HandleCrashSignal; +void HandleTerminateSignal(int sig, siginfo_t* siginfo, void* context) { + MetricsRecordExit(Metrics::LifetimeMilestone::kTerminated); + RestoreDefaultSignalHandler(sig); + + // Re-raise the signal. See the explanation in HandleCrashSignal(). Note that + // no checks for signals arising from synchronous hardware faults are made + // because termination signals never originate in that way. + int rv = raise(sig); + DPLOG_IF(ERROR, rv != 0) << "raise"; +} + +void InstallCrashHandler() { // These are the core-generating signals from 10.12.3 // xnu-3789.41.3/bsd/sys/signalvar.h sigprop: entries with SA_CORE are in the // set. - const int kSignals[] = {SIGQUIT, - SIGILL, - SIGTRAP, - SIGABRT, - SIGEMT, - SIGFPE, - SIGBUS, - SIGSEGV, - SIGSYS}; + const int kCrashSignals[] = {SIGQUIT, + SIGILL, + SIGTRAP, + SIGABRT, + SIGEMT, + SIGFPE, + SIGBUS, + SIGSEGV, + SIGSYS}; + InstallSignalHandler( + std::vector(&kCrashSignals[0], + &kCrashSignals[arraysize(kCrashSignals)]), + HandleCrashSignal); - for (int sig : kSignals) { - DCHECK_GT(sig, 0); - DCHECK_LT(static_cast(sig), arraysize(g_original_crash_sigaction)); - int rv = sigaction(sig, &sa, &g_original_crash_sigaction[sig]); - PCHECK(rv == 0) << "sigaction " << sig; - } + // Not a crash handler, but close enough. These are non-core-generating but + // terminating signals from 10.12.3 xnu-3789.41.3/bsd/sys/signalvar.h sigprop: + // entries with SA_KILL but not SA_CORE are in the set. SIGKILL is excluded + // because it is uncatchable. + const int kTerminateSignals[] = {SIGHUP, + SIGINT, + SIGPIPE, + SIGALRM, + SIGTERM, + SIGXCPU, + SIGXFSZ, + SIGVTALRM, + SIGPROF, + SIGUSR1, + SIGUSR2}; + InstallSignalHandler( + std::vector(&kTerminateSignals[0], + &kTerminateSignals[arraysize(kTerminateSignals)]), + HandleTerminateSignal); } struct ResetSIGTERMTraits { @@ -243,6 +311,9 @@ ExceptionHandlerServer* g_exception_handler_server; // This signal handler is only operative when being run from launchd. void HandleSIGTERM(int sig, siginfo_t* siginfo, void* context) { + // Don’t call MetricsRecordExit(). This is part of the normal exit path when + // running from launchd. + DCHECK(g_exception_handler_server); g_exception_handler_server->Stop(); } @@ -252,6 +323,7 @@ void HandleSIGTERM(int sig, siginfo_t* siginfo, void* context) { LONG(WINAPI* g_original_exception_filter)(EXCEPTION_POINTERS*) = nullptr; LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { + MetricsRecordExit(Metrics::LifetimeMilestone::kCrashed); Metrics::HandlerCrashed(exception_pointers->ExceptionRecord->ExceptionCode); if (g_original_exception_filter) @@ -260,9 +332,36 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { return EXCEPTION_CONTINUE_SEARCH; } +// Handles events like Control-C and Control-Break on a console. +BOOL WINAPI ConsoleHandler(DWORD console_event) { + MetricsRecordExit(Metrics::LifetimeMilestone::kTerminated); + return false; +} + +// Handles a WM_ENDSESSION message sent when the user session is ending. +class TerminateHandler final : public SessionEndWatcher { + public: + TerminateHandler() : SessionEndWatcher() {} + ~TerminateHandler() override {} + + private: + // SessionEndWatcher: + void SessionEnding() override { + MetricsRecordExit(Metrics::LifetimeMilestone::kTerminated); + } + + DISALLOW_COPY_AND_ASSIGN(TerminateHandler); +}; + void InstallCrashHandler() { g_original_exception_filter = SetUnhandledExceptionFilter(&UnhandledExceptionHandler); + + // These are termination handlers, not crash handlers, but that’s close + // enough. Note that destroying the TerminateHandler would wait for its thread + // to exit, which isn’t necessary or desirable. + SetConsoleCtrlHandler(ConsoleHandler, true); + static TerminateHandler* terminate_handler = new TerminateHandler(); } #endif // OS_MACOSX @@ -271,6 +370,7 @@ void InstallCrashHandler() { int HandlerMain(int argc, char* argv[]) { InstallCrashHandler(); + CallMetricsRecordNormalExit metrics_record_normal_exit; const base::FilePath argv0( ToolSupport::CommandLineArgumentToFilePathStringType(argv[0])); @@ -367,7 +467,7 @@ int HandlerMain(int argc, char* argv[]) { std::string value; if (!SplitStringFirst(optarg, '=', &key, &value)) { ToolSupport::UsageHint(me, "--annotation requires KEY=VALUE"); - return EXIT_FAILURE; + return ExitFailure(); } std::string old_value; if (!MapInsertOrReplace(&options.annotations, key, value, &old_value)) { @@ -386,7 +486,7 @@ int HandlerMain(int argc, char* argv[]) { options.handshake_fd < 0) { ToolSupport::UsageHint(me, "--handshake-fd requires a file descriptor"); - return EXIT_FAILURE; + return ExitFailure(); } break; } @@ -399,7 +499,7 @@ int HandlerMain(int argc, char* argv[]) { if (!options.initial_client_data.InitializeFromString(optarg)) { ToolSupport::UsageHint( me, "failed to parse --initial-client-data"); - return EXIT_FAILURE; + return ExitFailure(); } break; } @@ -433,15 +533,17 @@ int HandlerMain(int argc, char* argv[]) { } case kOptionHelp: { Usage(me); + MetricsRecordExit(Metrics::LifetimeMilestone::kExitedEarly); return EXIT_SUCCESS; } case kOptionVersion: { ToolSupport::Version(me); + MetricsRecordExit(Metrics::LifetimeMilestone::kExitedEarly); return EXIT_SUCCESS; } default: { ToolSupport::UsageHint(me, nullptr); - return EXIT_FAILURE; + return ExitFailure(); } } } @@ -451,34 +553,34 @@ int HandlerMain(int argc, char* argv[]) { #if defined(OS_MACOSX) if (options.handshake_fd < 0 && options.mach_service.empty()) { ToolSupport::UsageHint(me, "--handshake-fd or --mach-service is required"); - return EXIT_FAILURE; + return ExitFailure(); } if (options.handshake_fd >= 0 && !options.mach_service.empty()) { ToolSupport::UsageHint( me, "--handshake-fd and --mach-service are incompatible"); - return EXIT_FAILURE; + return ExitFailure(); } #elif defined(OS_WIN) if (!options.initial_client_data.IsValid() && options.pipe_name.empty()) { ToolSupport::UsageHint(me, "--initial-client-data or --pipe-name is required"); - return EXIT_FAILURE; + return ExitFailure(); } if (options.initial_client_data.IsValid() && !options.pipe_name.empty()) { ToolSupport::UsageHint( me, "--initial-client-data and --pipe-name are incompatible"); - return EXIT_FAILURE; + return ExitFailure(); } #endif // OS_MACOSX if (!options.database) { ToolSupport::UsageHint(me, "--database is required"); - return EXIT_FAILURE; + return ExitFailure(); } if (argc) { ToolSupport::UsageHint(me, nullptr); - return EXIT_FAILURE; + return ExitFailure(); } #if defined(OS_MACOSX) @@ -503,7 +605,7 @@ int HandlerMain(int argc, char* argv[]) { } if (!receive_right.is_valid()) { - return EXIT_FAILURE; + return ExitFailure(); } ExceptionHandlerServer exception_handler_server( @@ -520,6 +622,7 @@ int HandlerMain(int argc, char* argv[]) { // launchd.plist(5). // // Set up a SIGTERM handler that will call exception_handler_server.Stop(). + // This replaces the HandleTerminateSignal handler for SIGTERM. struct sigaction sa = {}; sigemptyset(&sa.sa_mask); sa.sa_flags = SA_SIGINFO; @@ -553,11 +656,13 @@ int HandlerMain(int argc, char* argv[]) { } } + Metrics::HandlerLifetimeMilestone(Metrics::LifetimeMilestone::kStarted); + std::unique_ptr database(CrashReportDatabase::Initialize( base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType( options.database)))); if (!database) { - return EXIT_FAILURE; + return ExitFailure(); } // TODO(scottmg): options.rate_limit should be removed when we have a diff --git a/util/misc/metrics.cc b/util/misc/metrics.cc index 48be7ecd..e2bd9f0b 100644 --- a/util/misc/metrics.cc +++ b/util/misc/metrics.cc @@ -98,6 +98,14 @@ void Metrics::ExceptionEncountered() { ExceptionProcessing(ExceptionProcessingState::kStarted); } +// static +void Metrics::HandlerLifetimeMilestone(LifetimeMilestone milestone) { + UMA_HISTOGRAM_ENUMERATION("Crashpad.HandlerLifetimeMilestone", + static_cast(milestone), + static_cast(LifetimeMilestone::kMaxValue)); +} + +// static void Metrics::HandlerCrashed(uint32_t exception_code) { UMA_HISTOGRAM_SPARSE_SLOWLY( "Crashpad.HandlerCrash.ExceptionCode." METRICS_OS_NAME, diff --git a/util/misc/metrics.h b/util/misc/metrics.h index fd2e1200..f1c5731a 100644 --- a/util/misc/metrics.h +++ b/util/misc/metrics.h @@ -130,6 +130,34 @@ class Metrics { //! \brief The exception handler server started capturing an exception. static void ExceptionEncountered(); + //! \brief An important event in a handler process’ lifetime. + enum class LifetimeMilestone : int32_t { + //! \brief The handler process started. + kStarted = 0, + + //! \brief The handler process exited normally and cleanly. + kExitedNormally, + + //! \brief The handler process exited early, but was successful in + //! performing some non-default action on user request. + kExitedEarly, + + //! \brief The handler process exited with a failure code. + kFailed, + + //! \brief The handler process was forcibly terminated. + kTerminated, + + //! \brief The handler process crashed. + kCrashed, + + //! \brief The number of values in this enumeration; not a valid value. + kMaxValue + }; + + //! \brief Records a handler start/exit/crash event. + static void HandlerLifetimeMilestone(LifetimeMilestone milestone); + //! \brief The handler process crashed with the given exception code. //! //! This is currently only reported on Windows. diff --git a/util/thread/thread_posix.cc b/util/thread/thread_posix.cc index 7142c786..58a98747 100644 --- a/util/thread/thread_posix.cc +++ b/util/thread/thread_posix.cc @@ -14,20 +14,22 @@ #include "util/thread/thread.h" +#include + #include "base/logging.h" namespace crashpad { void Thread::Start() { DCHECK(!platform_thread_); - int rv = pthread_create(&platform_thread_, nullptr, ThreadEntryThunk, this); - PCHECK(0 == rv); + errno = pthread_create(&platform_thread_, nullptr, ThreadEntryThunk, this); + PCHECK(errno == 0) << "pthread_create"; } void Thread::Join() { DCHECK(platform_thread_); - int rv = pthread_join(platform_thread_, nullptr); - PCHECK(0 == rv); + errno = pthread_join(platform_thread_, nullptr); + PCHECK(errno == 0) << "pthread_join"; platform_thread_ = 0; } diff --git a/util/thread/thread_win.cc b/util/thread/thread_win.cc index c4ac1eb7..466dfa9b 100644 --- a/util/thread/thread_win.cc +++ b/util/thread/thread_win.cc @@ -22,13 +22,13 @@ void Thread::Start() { DCHECK(!platform_thread_); platform_thread_ = CreateThread(nullptr, 0, ThreadEntryThunk, this, 0, nullptr); - PCHECK(platform_thread_); + PCHECK(platform_thread_) << "CreateThread"; } void Thread::Join() { DCHECK(platform_thread_); DWORD result = WaitForSingleObject(platform_thread_, INFINITE); - PCHECK(WAIT_OBJECT_0 == result); + PCHECK(result == WAIT_OBJECT_0) << "WaitForSingleObject"; platform_thread_ = 0; } diff --git a/util/util.gyp b/util/util.gyp index b2354d28..b27337c6 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -198,6 +198,8 @@ 'win/scoped_local_alloc.h', 'win/scoped_process_suspend.cc', 'win/scoped_process_suspend.h', + 'win/session_end_watcher.cc', + 'win/session_end_watcher.h', 'win/termination_codes.h', 'win/time.cc', 'win/time.h', @@ -268,6 +270,7 @@ ['OS=="win"', { 'link_settings': { 'libraries': [ + '-luser32.lib', '-lwinhttp.lib', ], }, diff --git a/util/util_test.gyp b/util/util_test.gyp index 7636941b..e16a292f 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -95,6 +95,7 @@ 'win/process_info_test.cc', 'win/registration_protocol_win_test.cc', 'win/scoped_process_suspend_test.cc', + 'win/session_end_watcher_test.cc', 'win/time_test.cc', ], 'conditions': [ @@ -114,6 +115,7 @@ '-ladvapi32.lib', '-limagehlp.lib', '-lrpcrt4.lib', + '-luser32.lib', ], }, }], diff --git a/util/win/scoped_handle.cc b/util/win/scoped_handle.cc index 5eb440e0..57d28217 100644 --- a/util/win/scoped_handle.cc +++ b/util/win/scoped_handle.cc @@ -25,7 +25,7 @@ void ScopedFileHANDLECloseTraits::Free(HANDLE handle) { } void ScopedKernelHANDLECloseTraits::Free(HANDLE handle) { - PCHECK(CloseHandle(handle)); + PCHECK(CloseHandle(handle)) << "CloseHandle"; } } // namespace internal diff --git a/util/win/session_end_watcher.cc b/util/win/session_end_watcher.cc new file mode 100644 index 00000000..e795f22a --- /dev/null +++ b/util/win/session_end_watcher.cc @@ -0,0 +1,243 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/win/session_end_watcher.h" + +#include "base/logging.h" +#include "base/scoped_generic.h" + +extern "C" { +extern IMAGE_DOS_HEADER __ImageBase; +} // extern "C" + +namespace crashpad { + +namespace { + +class ScopedSetEvent { + public: + explicit ScopedSetEvent(HANDLE event) : event_(event) {} + ~ScopedSetEvent() { + if (!SetEvent(event_)) { + PLOG(ERROR) << "SetEvent"; + } + } + + private: + HANDLE event_; + + DISALLOW_COPY_AND_ASSIGN(ScopedSetEvent); +}; + +// ScopedWindowClass and ScopedWindow operate on ATOM* and HWND*, respectively, +// instead of ATOM and HWND, so that the actual storage can exist as a local +// variable or a member variable, and the scoper can be responsible for +// releasing things only if the actual storage hasn’t been released and zeroed +// already by something else. +struct ScopedWindowClassTraits { + static ATOM* InvalidValue() { return nullptr; } + static void Free(ATOM* window_class) { + if (*window_class) { + if (!UnregisterClass(MAKEINTATOM(*window_class), 0)) { + PLOG(ERROR) << "UnregisterClass"; + } else { + *window_class = 0; + } + } + } +}; +using ScopedWindowClass = base::ScopedGeneric; + +struct ScopedWindowTraits { + static HWND* InvalidValue() { return nullptr; } + static void Free(HWND* window) { + if (*window) { + if (!DestroyWindow(*window)) { + PLOG(ERROR) << "DestroyWindow"; + } else { + *window = nullptr; + } + } + } +}; +using ScopedWindow = base::ScopedGeneric; + +// GetWindowLongPtr()’s return value doesn’t unambiguously indicate whether it +// was successful, because 0 could either represent successful retrieval of the +// value 0, or failure. This wrapper is more convenient to use. +bool GetWindowLongPtrAndSuccess(HWND window, int index, LONG_PTR* value) { + SetLastError(ERROR_SUCCESS); + *value = GetWindowLongPtr(window, index); + return *value || GetLastError() == ERROR_SUCCESS; +} + +// SetWindowLongPtr() has the same problem as GetWindowLongPtr(). Use this +// wrapper instead. +bool SetWindowLongPtrAndGetSuccess(HWND window, int index, LONG_PTR value) { + SetLastError(ERROR_SUCCESS); + LONG_PTR previous = SetWindowLongPtr(window, index, value); + return previous || GetLastError() == ERROR_SUCCESS; +} + +} // namespace + +SessionEndWatcher::SessionEndWatcher() + : Thread(), + window_(nullptr), + started_(nullptr), + stopped_(nullptr) { + // Set bManualReset for these events so that WaitForStart() and WaitForStop() + // can be called multiple times. + + started_.reset(CreateEvent(nullptr, true, false, nullptr)); + PLOG_IF(ERROR, !started_.get()) << "CreateEvent"; + + stopped_.reset(CreateEvent(nullptr, true, false, nullptr)); + PLOG_IF(ERROR, !stopped_.get()) << "CreateEvent"; + + Start(); +} + +SessionEndWatcher::~SessionEndWatcher() { + // Tear everything down by posting a WM_CLOSE to the window. This obviously + // can’t work until the window has been created, and that happens on a + // different thread, so wait for the start event to be signaled first. + WaitForStart(); + if (window_) { + if (!PostMessage(window_, WM_CLOSE, 0, 0)) { + PLOG(ERROR) << "PostMessage"; + } + } + + Join(); + DCHECK(!window_); +} + +void SessionEndWatcher::WaitForStart() { + if (WaitForSingleObject(started_.get(), INFINITE) != WAIT_OBJECT_0) { + PLOG(ERROR) << "WaitForSingleObject"; + } +} + +void SessionEndWatcher::WaitForStop() { + if (WaitForSingleObject(stopped_.get(), INFINITE) != WAIT_OBJECT_0) { + PLOG(ERROR) << "WaitForSingleObject"; + } +} + +void SessionEndWatcher::ThreadMain() { + ATOM atom = 0; + ScopedWindowClass window_class(&atom); + ScopedWindow window(&window_); + + ScopedSetEvent call_set_stop(stopped_.get()); + + { + ScopedSetEvent call_set_start(started_.get()); + + WNDCLASS wndclass = {}; + wndclass.lpfnWndProc = WindowProc; + wndclass.hInstance = reinterpret_cast(&__ImageBase); + wndclass.lpszClassName = L"crashpad_SessionEndWatcher"; + atom = RegisterClass(&wndclass); + if (!atom) { + PLOG(ERROR) << "RegisterClass"; + return; + } + + window_ = CreateWindow(MAKEINTATOM(atom), // lpClassName + nullptr, // lpWindowName + 0, // dwStyle + 0, // x + 0, // y + 0, // nWidth + 0, // nHeight + nullptr, // hWndParent + nullptr, // hMenu + nullptr, // hInstance + this); // lpParam + if (!window_) { + PLOG(ERROR) << "CreateWindow"; + return; + } + } + + MSG message; + BOOL rv = 0; + while (window_ && (rv = GetMessage(&message, window_, 0, 0)) > 0) { + TranslateMessage(&message); + DispatchMessage(&message); + } + if (window_ && rv == -1) { + PLOG(ERROR) << "GetMessage"; + return; + } +} + +// static +LRESULT CALLBACK SessionEndWatcher::WindowProc(HWND window, + UINT message, + WPARAM w_param, + LPARAM l_param) { + // Figure out which object this is. A pointer to it is stuffed into the last + // parameter of CreateWindow(), which shows up as CREATESTRUCT::lpCreateParams + // in a WM_CREATE message. That should be processed before any of the other + // messages of interest to this function. Once the object is known, save a + // pointer to it in the GWLP_USERDATA slot for later retrieval when processing + // other messages. + SessionEndWatcher* self; + if (!GetWindowLongPtrAndSuccess( + window, GWLP_USERDATA, reinterpret_cast(&self))) { + PLOG(ERROR) << "GetWindowLongPtr"; + } + if (!self && message == WM_CREATE) { + CREATESTRUCT* create = reinterpret_cast(l_param); + self = reinterpret_cast(create->lpCreateParams); + if (!SetWindowLongPtrAndGetSuccess( + window, GWLP_USERDATA, reinterpret_cast(self))) { + PLOG(ERROR) << "SetWindowLongPtr"; + } + } + + if (self) { + if (message == WM_ENDSESSION) { + // If w_param is false, this WM_ENDSESSION message cancels a previous + // WM_QUERYENDSESSION. + if (w_param) { + self->SessionEnding(); + + // If the session is ending, post a close message which will kick off + // window destruction and cause the message loop thread to terminate. + if (!PostMessage(self->window_, WM_CLOSE, 0, 0)) { + PLOG(ERROR) << "PostMessage"; + } + } + } else if (message == WM_DESTROY) { + // The window is being destroyed. Clear GWLP_USERDATA so that |self| won’t + // be found during a subsequent call into this function for this window. + // Clear self->window_ too, because it refers to an object that soon won’t + // exist. That signals the message loop to stop processing messages. + if (!SetWindowLongPtrAndGetSuccess(window, GWLP_USERDATA, 0)) { + PLOG(ERROR) << "SetWindowLongPtr"; + } + self->window_ = nullptr; + } + } + + // If the message is WM_CLOSE, DefWindowProc() will call DestroyWindow(), and + // this function will be called again with a WM_DESTROY message. + return DefWindowProc(window, message, w_param, l_param); +} + +} // namespace crashpad diff --git a/util/win/session_end_watcher.h b/util/win/session_end_watcher.h new file mode 100644 index 00000000..b23d391d --- /dev/null +++ b/util/win/session_end_watcher.h @@ -0,0 +1,79 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_UTIL_WIN_SESSION_END_WATCHER_H_ +#define CRASHPAD_UTIL_WIN_SESSION_END_WATCHER_H_ + +#include + +#include "base/macros.h" +#include "util/thread/thread.h" +#include "util/win/scoped_handle.h" + +namespace crashpad { + +//! \brief Creates a hidden window and waits for a `WM_ENDSESSION` message, +//! indicating that the session is ending and the application should +//! terminate. +//! +//! A dedicated thread will be created to run the `GetMessage()`-based message +//! loop required to monitor for this message. +//! +//! Users should subclass this class and receive notifications by implementing +//! the SessionEndWatcherEvent() method. +class SessionEndWatcher : public Thread { + public: + SessionEndWatcher(); + + //! \note The destructor waits for the thread that runs the message loop to + //! terminate. + ~SessionEndWatcher() override; + + protected: + // Exposed for testing. + HWND GetWindow() const { return window_; } + + // Exposed for testing. Blocks until window_ has been created. May be called + // multiple times if necessary. + void WaitForStart(); + + // Exposed for testing. Blocks until the message loop ends. May be called + // multiple times if necessary. + void WaitForStop(); + + private: + // Thread: + void ThreadMain() override; + + static LRESULT CALLBACK WindowProc(HWND window, + UINT message, + WPARAM w_param, + LPARAM l_param); + + //! \brief A `WM_ENDSESSION` message was received and it indicates that the + //! user session will be ending imminently. + //! + //! This method is called on the thread that runs the message loop. + virtual void SessionEnding() = 0; + + HWND window_; // Conceptually strong, but ownership managed in ThreadMain() + ScopedKernelHANDLE started_; + ScopedKernelHANDLE stopped_; + + DISALLOW_COPY_AND_ASSIGN(SessionEndWatcher); +}; + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_WIN_SESSION_END_WATCHER_H_ diff --git a/util/win/session_end_watcher_test.cc b/util/win/session_end_watcher_test.cc new file mode 100644 index 00000000..692d76e3 --- /dev/null +++ b/util/win/session_end_watcher_test.cc @@ -0,0 +1,62 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/win/session_end_watcher.h" + +#include "gtest/gtest.h" +#include "test/errors.h" + +namespace crashpad { +namespace test { +namespace { + +class SessionEndWatcherTest final : public SessionEndWatcher { + public: + SessionEndWatcherTest() : SessionEndWatcher(), called_(false) {} + + ~SessionEndWatcherTest() override {} + + void Run() { + WaitForStart(); + + HWND window = GetWindow(); + ASSERT_TRUE(window); + EXPECT_TRUE(PostMessage(window, WM_ENDSESSION, 1, 0)); + + WaitForStop(); + + EXPECT_TRUE(called_); + } + + private: + // SessionEndWatcher: + void SessionEnding() override { called_ = true; } + + bool called_; + + DISALLOW_COPY_AND_ASSIGN(SessionEndWatcherTest); +}; + +TEST(SessionEndWatcher, SessionEndWatcher) { + SessionEndWatcherTest test; + test.Run(); +} + +TEST(SessionEndWatcher, DoNothing) { + SessionEndWatcherTest test; +} + +} // namespace +} // namespace test +} // namespace crashpad From c1af20f1aa8a8f34b88c59c26ec8fece8504d3e9 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 22 Feb 2017 18:24:52 -0500 Subject: [PATCH 08/10] metrics: Consistently comment about enums used for metrics BUG=crashpad:100 Change-Id: I9ed0f260b4c92e7a706418f58c3db1ae027a04ab Reviewed-on: https://chromium-review.googlesource.com/446557 Reviewed-by: Mark Mentovai --- util/misc/metrics.h | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/util/misc/metrics.h b/util/misc/metrics.h index f1c5731a..b4bea910 100644 --- a/util/misc/metrics.h +++ b/util/misc/metrics.h @@ -30,8 +30,10 @@ namespace crashpad { //! Chromium's base, they allow integration with its metrics system. class Metrics { public: - //! \brief Values for CrashReportPending(). These are used as metrics - //! enumeration values, so new values should always be added at the end. + //! \brief Values for CrashReportPending(). + //! + //! \note These are used as metrics enumeration values, so new values should + //! always be added at the end, before PendingReportReason::kMaxValue. enum class PendingReportReason : int32_t { //! \brief A report was newly created and is ready for upload. kNewlyCreated = 0, @@ -53,8 +55,10 @@ class Metrics { //! \brief Reports on a crash upload attempt, and if it succeeded. static void CrashUploadAttempted(bool successful); - //! \brief Values for CrashUploadSkipped(). These are used as metrics - //! enumeration values, so new values should always be added at the end. + //! \brief Values for CrashUploadSkipped(). + //! + //! \note These are used as metrics enumeration values, so new values should + //! always be added at the end, before CrashSkippedReason::kMaxValue. enum class CrashSkippedReason : int32_t { //! \brief Crash uploading is disabled. kUploadsDisabled = 0, @@ -81,8 +85,10 @@ class Metrics { //! database, without the report being uploadad. static void CrashUploadSkipped(CrashSkippedReason reason); - //! \brief The result of capturing an exception. These are used as metrics - //! enumeration values, so new values should always be added at the end. + //! \brief The result of capturing an exception. + //! + //! \note These are used as metrics enumeration values, so new values should + //! always be added at the end, before CaptureResult::kMaxValue. enum class CaptureResult : int32_t { //! \brief The exception capture succeeded normally. kSuccess = 0, @@ -131,6 +137,9 @@ class Metrics { static void ExceptionEncountered(); //! \brief An important event in a handler process’ lifetime. + //! + //! \note These are used as metrics enumeration values, so new values should + //! always be added at the end, before LifetimeMilestone::kMaxValue. enum class LifetimeMilestone : int32_t { //! \brief The handler process started. kStarted = 0, From 9c8407123796e024c195aa6497228b2bcb1c0616 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 22 Feb 2017 20:08:03 -0500 Subject: [PATCH 09/10] Fix warning when building with clang on Windows Change-Id: If9928d8ca3b12a260b97d522abfa7e3b5ff47831 Reviewed-on: https://chromium-review.googlesource.com/446418 Reviewed-by: Scott Graham --- handler/handler_main.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 81d3de86..c7c2b297 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -362,6 +362,7 @@ void InstallCrashHandler() { // to exit, which isn’t necessary or desirable. SetConsoleCtrlHandler(ConsoleHandler, true); static TerminateHandler* terminate_handler = new TerminateHandler(); + ALLOW_UNUSED_LOCAL(terminate_handler); } #endif // OS_MACOSX From 6da9708e7cc93e2e1772439d51646e47583cb225 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 22 Feb 2017 20:43:28 -0500 Subject: [PATCH 10/10] doc: Fix Doxygen errors Change-Id: I5d5abf7b7eabe269a7c7b4d305a67fe910c887fd Reviewed-on: https://chromium-review.googlesource.com/446480 Reviewed-by: Scott Graham --- snapshot/cpu_context.h | 2 +- util/misc/zlib.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/snapshot/cpu_context.h b/snapshot/cpu_context.h index f5bc8bf1..ba3ac181 100644 --- a/snapshot/cpu_context.h +++ b/snapshot/cpu_context.h @@ -140,7 +140,7 @@ struct CPUContextX86 { static uint16_t FxsaveToFsaveTagWord( uint16_t fsw, uint8_t fxsave_tag, const X87OrMMXRegister st_mm[8]); - //! \breif Converts x87 floating-point tag words from `fsave` (full, 16-bit) + //! \brief Converts x87 floating-point tag words from `fsave` (full, 16-bit) //! to `fxsave` (abridged, 8-bit) form. //! //! This function performs the inverse operation of FxsaveToFsaveTagWord(). diff --git a/util/misc/zlib.h b/util/misc/zlib.h index e3da6438..99a9c91f 100644 --- a/util/misc/zlib.h +++ b/util/misc/zlib.h @@ -23,8 +23,8 @@ namespace crashpad { //! `inflateInit2()` that specifies a `gzip` wrapper instead of the default //! zlib wrapper. //! -//! \param[in] A \a window_bits value that only specifies the base-2 logarithm -//! of the deflate sliding window size. +//! \param[in] window_bits A \a window_bits value that only specifies the base-2 +//! logarithm of the deflate sliding window size. //! //! \return \a window_bits adjusted to specify a `gzip` wrapper, to be passed to //! `deflateInit2()` or `inflateInit2()`.