From 34875869667ec97018671a0f8307aa0f9376f03e Mon Sep 17 00:00:00 2001 From: Gabriele Svelto Date: Thu, 3 Mar 2022 13:14:55 +0000 Subject: [PATCH] Bug 1756771 - Pack all Breakpad CPU context structures consistently r=glandium Also add out-of-tree patches for bugs we fixed in the meantime. Differential Revision: https://phabricator.services.mozilla.com/D139466 --- .../27-fix-unified-builds-bug-1733547.patch | 22 ++++ .../28-no-garbage-in-code-ids.patch | 34 +++++ .../29-cpu-context-packing.patch | 118 ++++++++++++++++++ .../common/minidump_cpu_arm64.h | 8 +- .../google_breakpad/common/minidump_cpu_ppc.h | 17 +-- 5 files changed, 182 insertions(+), 17 deletions(-) create mode 100644 toolkit/crashreporter/breakpad-patches/27-fix-unified-builds-bug-1733547.patch create mode 100644 toolkit/crashreporter/breakpad-patches/28-no-garbage-in-code-ids.patch create mode 100644 toolkit/crashreporter/breakpad-patches/29-cpu-context-packing.patch diff --git a/toolkit/crashreporter/breakpad-patches/27-fix-unified-builds-bug-1733547.patch b/toolkit/crashreporter/breakpad-patches/27-fix-unified-builds-bug-1733547.patch new file mode 100644 index 000000000000..5afc50316ec3 --- /dev/null +++ b/toolkit/crashreporter/breakpad-patches/27-fix-unified-builds-bug-1733547.patch @@ -0,0 +1,22 @@ +diff --git a/src/processor/stack_frame_symbolizer.cc b/src/processor/stack_frame_symbolizer.cc +--- a/src/processor/stack_frame_symbolizer.cc ++++ b/src/processor/stack_frame_symbolizer.cc +@@ -124,17 +124,16 @@ StackFrameSymbolizer::SymbolizerResult S + + case SymbolSupplier::INTERRUPT: + return kInterrupt; + + default: + BPLOG(ERROR) << "Unknown SymbolResult enum: " << symbol_result; + return kError; + } +- return kError; + } + + WindowsFrameInfo* StackFrameSymbolizer::FindWindowsFrameInfo( + const StackFrame* frame) { + return resolver_ ? resolver_->FindWindowsFrameInfo(frame) : NULL; + } + + CFIFrameInfo* StackFrameSymbolizer::FindCFIFrameInfo( + diff --git a/toolkit/crashreporter/breakpad-patches/28-no-garbage-in-code-ids.patch b/toolkit/crashreporter/breakpad-patches/28-no-garbage-in-code-ids.patch new file mode 100644 index 000000000000..64f791d01e69 --- /dev/null +++ b/toolkit/crashreporter/breakpad-patches/28-no-garbage-in-code-ids.patch @@ -0,0 +1,34 @@ +diff --git a/src/common/linux/file_id.cc b/src/common/linux/file_id.cc +--- a/src/common/linux/file_id.cc ++++ b/src/common/linux/file_id.cc +@@ -115,28 +115,27 @@ static bool FindElfBuildIDNote(const voi + + return false; + } + + // Attempt to locate the .text section of an ELF binary and generate + // a simple hash by XORing the first page worth of bytes into |identifier|. + static bool HashElfTextSection(const void* elf_mapped_base, + wasteful_vector& identifier) { +- identifier.resize(kMDGUIDSize); +- + void* text_section; + size_t text_size; + if (!FindElfSection(elf_mapped_base, ".text", SHT_PROGBITS, + (const void**)&text_section, &text_size) || + text_size == 0) { + return false; + } + + // Only provide |kMDGUIDSize| bytes to keep identifiers produced by this + // function backwards-compatible. ++ identifier.resize(kMDGUIDSize); + my_memset(&identifier[0], 0, kMDGUIDSize); + const uint8_t* ptr = reinterpret_cast(text_section); + const uint8_t* ptr_end = ptr + std::min(text_size, static_cast(4096)); + while (ptr < ptr_end) { + for (unsigned i = 0; i < kMDGUIDSize; i++) + identifier[i] ^= ptr[i]; + ptr += kMDGUIDSize; + } + diff --git a/toolkit/crashreporter/breakpad-patches/29-cpu-context-packing.patch b/toolkit/crashreporter/breakpad-patches/29-cpu-context-packing.patch new file mode 100644 index 000000000000..f6ef0c4d29ac --- /dev/null +++ b/toolkit/crashreporter/breakpad-patches/29-cpu-context-packing.patch @@ -0,0 +1,118 @@ +diff --git a/src/google_breakpad/common/minidump_cpu_arm64.h b/src/google_breakpad/common/minidump_cpu_arm64.h +--- a/src/google_breakpad/common/minidump_cpu_arm64.h ++++ b/src/google_breakpad/common/minidump_cpu_arm64.h +@@ -86,16 +86,20 @@ typedef struct { + #define MD_CONTEXT_ARM64_INTEGER (MD_CONTEXT_ARM64 | 0x00000002) + #define MD_CONTEXT_ARM64_FLOATING_POINT (MD_CONTEXT_ARM64 | 0x00000004) + #define MD_CONTEXT_ARM64_DEBUG (MD_CONTEXT_ARM64 | 0x00000008) + #define MD_CONTEXT_ARM64_FULL (MD_CONTEXT_ARM64_CONTROL | \ + MD_CONTEXT_ARM64_INTEGER | \ + MD_CONTEXT_ARM64_FLOATING_POINT) + #define MD_CONTEXT_ARM64_ALL (MD_CONTEXT_ARM64_FULL | MD_CONTEXT_ARM64_DEBUG) + ++/* Use the same 32-bit alignment when accessing these structures from 64-bit ++ * code as is used natively in 32-bit code. */ ++#pragma pack(push, 4) ++ + typedef struct { + /* Determines which fields of this struct are populated */ + uint32_t context_flags; + + /* CPSR (flags, basically): 32 bits: + bit 31 - N (negative) + bit 30 - Z (zero) + bit 29 - C (carry) +@@ -125,20 +129,16 @@ typedef struct { + typedef struct { + uint32_t fpsr; /* FPU status register */ + uint32_t fpcr; /* FPU control register */ + + /* 32 128-bit floating point registers, d0 .. d31. */ + uint128_struct regs[MD_FLOATINGSAVEAREA_ARM64_FPR_COUNT]; + } MDFloatingSaveAreaARM64_Old; + +-/* Use the same 32-bit alignment when accessing this structure from 64-bit code +- * as is used natively in 32-bit code. */ +-#pragma pack(push, 4) +- + typedef struct { + /* The next field determines the layout of the structure, and which parts + * of it are populated + */ + uint64_t context_flags; + + /* 33 64-bit integer registers, x0 .. x31 + the PC + * Note the following fixed uses: +diff --git a/src/google_breakpad/common/minidump_cpu_ppc.h b/src/google_breakpad/common/minidump_cpu_ppc.h +--- a/src/google_breakpad/common/minidump_cpu_ppc.h ++++ b/src/google_breakpad/common/minidump_cpu_ppc.h +@@ -73,16 +73,20 @@ + /* + * Breakpad minidump extension for PowerPC support. Based on Darwin/Mac OS X' + * mach/ppc/_types.h + */ + + #ifndef GOOGLE_BREAKPAD_COMMON_MINIDUMP_CPU_PPC_H__ + #define GOOGLE_BREAKPAD_COMMON_MINIDUMP_CPU_PPC_H__ + ++/* Use the same 32-bit alignment when accessing these structures from 64-bit ++ * code as is used natively in 32-bit code. */ ++#pragma pack(push, 4) ++ + #define MD_FLOATINGSAVEAREA_PPC_FPR_COUNT 32 + + typedef struct { + /* fpregs is a double[32] in mach/ppc/_types.h, but a uint64_t is used + * here for precise sizing. */ + uint64_t fpregs[MD_FLOATINGSAVEAREA_PPC_FPR_COUNT]; + uint32_t fpscr_pad; + uint32_t fpscr; /* Status/control */ +@@ -99,25 +103,16 @@ typedef struct { + uint32_t save_pad5[4]; + uint32_t save_vrvalid; /* Indicates which vector registers are saved */ + uint32_t save_pad6[7]; + } MDVectorSaveAreaPPC; /* ppc_vector_state */ + + + #define MD_CONTEXT_PPC_GPR_COUNT 32 + +-/* Use the same 32-bit alignment when accessing this structure from 64-bit code +- * as is used natively in 32-bit code. #pragma pack is a MSVC extension +- * supported by gcc. */ +-#if defined(__SUNPRO_C) || defined(__SUNPRO_CC) +-#pragma pack(4) +-#else +-#pragma pack(push, 4) +-#endif +- + typedef struct { + /* context_flags is not present in ppc_thread_state, but it aids + * identification of MDRawContextPPC among other raw context types, + * and it guarantees alignment when we get to float_save. */ + uint32_t context_flags; + + uint32_t srr0; /* Machine status save/restore: stores pc + * (instruction) */ +@@ -140,21 +135,17 @@ typedef struct { + MDVectorSaveAreaPPC vector_save; + } MDRawContextPPC; /* Based on ppc_thread_state */ + + /* Indices into gpr for registers with a dedicated or conventional purpose. */ + enum MDPPCRegisterNumbers { + MD_CONTEXT_PPC_REG_SP = 1 + }; + +-#if defined(__SUNPRO_C) || defined(__SUNPRO_CC) +-#pragma pack(0) +-#else + #pragma pack(pop) +-#endif + + /* For (MDRawContextPPC).context_flags. These values indicate the type of + * context stored in the structure. MD_CONTEXT_PPC is Breakpad-defined. Its + * value was chosen to avoid likely conflicts with MD_CONTEXT_* for other + * CPUs. */ + #define MD_CONTEXT_PPC 0x20000000 + #define MD_CONTEXT_PPC_BASE (MD_CONTEXT_PPC | 0x00000001) + #define MD_CONTEXT_PPC_FLOATING_POINT (MD_CONTEXT_PPC | 0x00000008) + diff --git a/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_arm64.h b/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_arm64.h index 0411bebb45cc..a94962009e3f 100644 --- a/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_arm64.h +++ b/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_arm64.h @@ -91,6 +91,10 @@ typedef struct { MD_CONTEXT_ARM64_FLOATING_POINT) #define MD_CONTEXT_ARM64_ALL (MD_CONTEXT_ARM64_FULL | MD_CONTEXT_ARM64_DEBUG) +/* Use the same 32-bit alignment when accessing these structures from 64-bit + * code as is used natively in 32-bit code. */ +#pragma pack(push, 4) + typedef struct { /* Determines which fields of this struct are populated */ uint32_t context_flags; @@ -130,10 +134,6 @@ typedef struct { uint128_struct regs[MD_FLOATINGSAVEAREA_ARM64_FPR_COUNT]; } MDFloatingSaveAreaARM64_Old; -/* Use the same 32-bit alignment when accessing this structure from 64-bit code - * as is used natively in 32-bit code. */ -#pragma pack(push, 4) - typedef struct { /* The next field determines the layout of the structure, and which parts * of it are populated diff --git a/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_ppc.h b/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_ppc.h index b24cc42438e4..fcc85cd9b0dd 100644 --- a/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_ppc.h +++ b/toolkit/crashreporter/google-breakpad/src/google_breakpad/common/minidump_cpu_ppc.h @@ -78,6 +78,10 @@ #ifndef GOOGLE_BREAKPAD_COMMON_MINIDUMP_CPU_PPC_H__ #define GOOGLE_BREAKPAD_COMMON_MINIDUMP_CPU_PPC_H__ +/* Use the same 32-bit alignment when accessing these structures from 64-bit + * code as is used natively in 32-bit code. */ +#pragma pack(push, 4) + #define MD_FLOATINGSAVEAREA_PPC_FPR_COUNT 32 typedef struct { @@ -104,15 +108,6 @@ typedef struct { #define MD_CONTEXT_PPC_GPR_COUNT 32 -/* Use the same 32-bit alignment when accessing this structure from 64-bit code - * as is used natively in 32-bit code. #pragma pack is a MSVC extension - * supported by gcc. */ -#if defined(__SUNPRO_C) || defined(__SUNPRO_CC) -#pragma pack(4) -#else -#pragma pack(push, 4) -#endif - typedef struct { /* context_flags is not present in ppc_thread_state, but it aids * identification of MDRawContextPPC among other raw context types, @@ -145,11 +140,7 @@ enum MDPPCRegisterNumbers { MD_CONTEXT_PPC_REG_SP = 1 }; -#if defined(__SUNPRO_C) || defined(__SUNPRO_CC) -#pragma pack(0) -#else #pragma pack(pop) -#endif /* For (MDRawContextPPC).context_flags. These values indicate the type of * context stored in the structure. MD_CONTEXT_PPC is Breakpad-defined. Its