Merge branch 'google:main' into main

Makefile.in

@@ -2691,11 +2691,13 @@ src_tools_linux_dump_syms_dump_syms_SOURCES = \
 	src/tools/linux/dump_syms/dump_syms.cc
 
 src_tools_linux_dump_syms_dump_syms_CXXFLAGS = \
-	$(RUSTC_DEMANGLE_CFLAGS)
+	$(RUSTC_DEMANGLE_CFLAGS) \
+	$(ZSTD_CFLAGS)
 
 src_tools_linux_dump_syms_dump_syms_LDADD = \
 	$(RUSTC_DEMANGLE_LIBS) \
-	-lz -lzstd
+	$(ZSTD_CFLAGS) \
+	-lz
 
 src_tools_linux_md2core_minidump_2_core_SOURCES = \
 	src/common/linux/memory_mapped_file.cc \
@@ -2822,13 +2824,15 @@ src_common_dumper_unittest_SOURCES = \
 src_common_dumper_unittest_CPPFLAGS = \
 	$(AM_CPPFLAGS) $(TEST_CFLAGS) \
 	$(RUSTC_DEMANGLE_CFLAGS) \
-	$(PTHREAD_CFLAGS)
+	$(PTHREAD_CFLAGS) \
+	$(ZSTD_CFLAGS)
 
 src_common_dumper_unittest_LDADD = \
 	$(TEST_LIBS) \
 	$(RUSTC_DEMANGLE_LIBS) \
 	$(PTHREAD_CFLAGS) $(PTHREAD_LIBS) \
-	-lz -lzstd
+	$(ZSTD_LIBS) \
+	-lz
 
 src_common_mac_macho_reader_unittest_SOURCES = \
 	src/common/dwarf_cfi_to_module.cc \

src/client/linux/minidump_writer/linux_ptrace_dumper.cc

@@ -59,9 +59,24 @@
 
 #include "client/linux/minidump_writer/directory_reader.h"
 #include "client/linux/minidump_writer/line_reader.h"
+#include "common/linux/eintr_wrapper.h"
 #include "common/linux/linux_libc_support.h"
 #include "third_party/lss/linux_syscall_support.h"
 
+#if defined(__arm__)
+/*
+ * https://elixir.bootlin.com/linux/v6.8-rc2/source/arch/arm/include/asm/user.h#L81
+ * User specific VFP registers. If only VFPv2 is present, registers 16 to 31
+ * are ignored by the ptrace system call and the signal handler.
+ */
+typedef struct {
+  unsigned long long fpregs[32];
+  unsigned long fpscr;
+// Kernel just appends fpscr to the copy of fpregs, so we need to force
+// compiler to build the same layout.
+} __attribute__((packed, aligned(4))) user_vfp_t;
+#endif  // defined(__arm__)
+
 // Suspends a thread by attaching to it.
 static bool SuspendThread(pid_t pid) {
   // This may fail if the thread has just died or debugged.
@@ -70,11 +85,29 @@ static bool SuspendThread(pid_t pid) {
       errno != 0) {
     return false;
   }
-  while (sys_waitpid(pid, NULL, __WALL) < 0) {
-    if (errno != EINTR) {
+  while (true) {
+    int status;
+    int r = HANDLE_EINTR(sys_waitpid(pid, &status, __WALL));
+    if (r < 0) {
       sys_ptrace(PTRACE_DETACH, pid, NULL, NULL);
       return false;
     }
+
+    if (!WIFSTOPPED(status))
+      return false;
+
+    // Any signal will stop the thread, make sure it is SIGSTOP. Otherwise, this
+    // signal will be delivered after PTRACE_DETACH, and the thread will enter
+    // the "T (stopped)" state.
+    if (WSTOPSIG(status) == SIGSTOP)
+      break;
+
+    // Signals other than SIGSTOP that are received need to be reinjected,
+    // or they will otherwise get lost.
+    r = sys_ptrace(PTRACE_CONT, pid, NULL,
+                   reinterpret_cast<void*>(WSTOPSIG(status)));
+    if (r < 0)
+      return false;
   }
 #if defined(__i386) || defined(__x86_64)
   // On x86, the stack pointer is NULL or -1, when executing trusted code in
@@ -152,6 +185,24 @@ bool LinuxPtraceDumper::CopyFromProcess(void* dest, pid_t child,
   return true;
 }
 
+// This read VFP registers via either PTRACE_GETREGSET or PTRACE_GETREGS
+#if defined(__arm__)
+static bool ReadVFPRegistersArm32(pid_t tid, struct iovec* io) {
+#ifdef PTRACE_GETREGSET
+  if (sys_ptrace(PTRACE_GETREGSET, tid, reinterpret_cast<void*>(NT_ARM_VFP),
+                 io) == 0 && io->iov_len == sizeof(user_vfp_t)) {
+    return true;
+  }
+#endif  // PTRACE_GETREGSET
+#ifdef PTRACE_GETVFPREGS
+  if (sys_ptrace(PTRACE_GETVFPREGS, tid, nullptr, io->iov_base) == 0) {
+    return true;
+  }
+#endif  // PTRACE_GETVFPREGS
+  return false;
+}
+#endif  // defined(__arm__)
+
 bool LinuxPtraceDumper::ReadRegisterSet(ThreadInfo* info, pid_t tid)
 {
 #ifdef PTRACE_GETREGSET
@@ -163,7 +214,24 @@ bool LinuxPtraceDumper::ReadRegisterSet(ThreadInfo* info, pid_t tid)
 
   info->GetFloatingPointRegisters(&io.iov_base, &io.iov_len);
   if (sys_ptrace(PTRACE_GETREGSET, tid, (void*)NT_FPREGSET, (void*)&io) == -1) {
-    return false;
+  // We are going to check if we can read VFP registers on ARM32.
+  // Currently breakpad does not support VFP registers to be a part of minidump,
+  // so this is only to confirm that we can actually read FP registers.
+  // That is needed to prevent a false-positive minidumps failures with ARM32
+  // binaries running on top of ARM64 Linux kernels.
+#if defined(__arm__)
+    switch (errno) {
+      case EIO:
+      case EINVAL:
+        user_vfp_t vfp;
+        struct iovec io;
+        io.iov_base = &vfp;
+        io.iov_len = sizeof(vfp);
+        return ReadVFPRegistersArm32(tid, &io);
+      default:
+        return false;
+    }
+#endif  // defined(__arm__)
   }
   return true;
 #else
@@ -194,7 +262,24 @@ bool LinuxPtraceDumper::ReadRegisters(ThreadInfo* info, pid_t tid) {
   void* fp_addr;
   info->GetFloatingPointRegisters(&fp_addr, NULL);
   if (sys_ptrace(PTRACE_GETFPREGS, tid, NULL, fp_addr) == -1) {
-    return false;
+  // We are going to check if we can read VFP registers on ARM32.
+  // Currently breakpad does not support VFP registers to be a part of minidump,
+  // so this is only to confirm that we can actually read FP registers.
+  // That is needed to prevent a false-positive minidumps failures with ARM32
+  // binaries running on top of ARM64 Linux kernels.
+#if defined(__arm__)
+    switch (errno) {
+      case EIO:
+      case EINVAL:
+        user_vfp_t vfp;
+        struct iovec io;
+        io.iov_base = &vfp;
+        io.iov_len = sizeof(vfp);
+        return ReadVFPRegistersArm32(tid, &io);
+      default:
+        return false;
+    }
+#endif  // defined(__arm__)
   }
 #endif  // !(defined(__ANDROID__) && defined(__ARM_EABI__))
 #endif  // !defined(__SOFTFP__)

src/common/dwarf/dwarf2reader.cc

@@ -285,8 +285,8 @@ const uint8_t* CompilationUnit::SkipAttribute(const uint8_t* start,
     case DW_FORM_sec_offset:
       return start + reader_->OffsetSize();
   }
-  fprintf(stderr,"Unhandled form type");
-  return NULL;
+  fprintf(stderr,"Unhandled form type 0x%x\n", form);
+  return nullptr;
 }
 
 // Read the abbreviation offset from a compilation unit header.
@@ -369,7 +369,7 @@ void CompilationUnit::ReadHeader() {
         break;
       case DW_UT_type:
       case DW_UT_split_type:
-	is_type_unit_ = true;
+        is_type_unit_ = true;
         headerptr += ReadTypeSignature(headerptr);
         headerptr += ReadTypeOffset(headerptr);
         break;
@@ -463,7 +463,11 @@ uint64_t CompilationUnit::Start() {
   }
 
   // Now that we have our abbreviations, start processing DIE's.
-  ProcessDIEs();
+  if (!ProcessDIEs()) {
+    // If ProcessDIEs fails return 0, ourlength must be non-zero
+    // as it is equal to header_.length + (12 or 4)
+    return 0;
+  }
 
   // If this is a skeleton compilation unit generated with split DWARF,
   // and the client needs the full debug info, we need to find the full
@@ -512,7 +516,7 @@ const uint8_t* CompilationUnit::ProcessOffsetBaseAttribute(
                                                                      &len));
       start += len;
       return ProcessOffsetBaseAttribute(dieoffset, start, attr, form,
-					   implicit_const);
+                                        implicit_const);
 
     case DW_FORM_flag_present:
       return start;
@@ -568,10 +572,10 @@ const uint8_t* CompilationUnit::ProcessOffsetBaseAttribute(
       // offset size.
       assert(header_.version >= 2);
       if (header_.version == 2) {
-	reader_->ReadAddress(start);
+        reader_->ReadAddress(start);
         return start + reader_->AddressSize();
       } else if (header_.version >= 3) {
-	reader_->ReadOffset(start);
+        reader_->ReadOffset(start);
         return start + reader_->OffsetSize();
       }
       break;
@@ -647,8 +651,8 @@ const uint8_t* CompilationUnit::ProcessOffsetBaseAttribute(
       reader_->ReadUnsignedLEB128(start, &len);
       return start + len;
   }
-  fprintf(stderr, "Unhandled form type\n");
-  return NULL;
+  fprintf(stderr,"Unhandled form type 0x%x\n", form);
+  return nullptr;
 }
 
 // If one really wanted, you could merge SkipAttribute and
@@ -896,11 +900,11 @@ const uint8_t* CompilationUnit::ProcessDIE(uint64_t dieoffset,
     uint64_t dieoffset_copy = dieoffset;
     const uint8_t* start_copy = start;
     for (AttributeList::const_iterator i = abbrev.attributes.begin();
-	 i != abbrev.attributes.end();
-	 i++) {
+         i != abbrev.attributes.end();
+         i++) {
       start_copy = ProcessOffsetBaseAttribute(dieoffset_copy, start_copy,
-						 i->attr_, i->form_,
-						 i->value_);
+                                              i->attr_, i->form_,
+                                              i->value_);
     }
   }
 
@@ -922,7 +926,7 @@ const uint8_t* CompilationUnit::ProcessDIE(uint64_t dieoffset,
   return start;
 }
 
-void CompilationUnit::ProcessDIEs() {
+bool CompilationUnit::ProcessDIEs() {
   const uint8_t* dieptr = after_header_;
   size_t len;
 
@@ -953,13 +957,21 @@ void CompilationUnit::ProcessDIEs() {
     if (abbrev_num == 0) {
       if (die_stack.size() == 0)
         // If it is padding, then we are done with the compilation unit's DIEs.
-        return;
+        return true;
       const uint64_t offset = die_stack.top();
       die_stack.pop();
       handler_->EndDIE(offset);
       continue;
     }
 
+    // Abbrev > abbrev_.size() indicates a corruption in the dwarf file.
+    if (abbrev_num > abbrevs_->size()) {
+      fprintf(stderr, "An invalid abbrev was referenced %" PRIu64 " / %zu. "
+              "Stopped procesing following DIEs in this CU.", abbrev_num,
+              abbrevs_->size());
+      return false;
+    }
+
     const Abbrev& abbrev = abbrevs_->at(static_cast<size_t>(abbrev_num));
     const enum DwarfTag tag = abbrev.tag;
     if (!handler_->StartDIE(absolute_offset, tag)) {
@@ -989,6 +1001,7 @@ void CompilationUnit::ProcessDIEs() {
       handler_->EndDIE(absolute_offset);
     }
   }
+  return true;
 }
 
 // Check for a valid ELF file and return the Address size.