Code review - Issue 10734002: add aarch64 support to minidump-2-corehttps://breakpad.appspot.com/2015-05-04T16:31:03+00:00rietveld
Message from unknown
2015-04-18T05:00:27+00:00vapierurn:md5:193d96cd3b15f8122aa79b60d62f1587
Message from vapier@chromium.org
2015-04-18T05:07:15+00:00vapierurn:md5:82660ce734bc7671be6b75fe46d7d90f
Message from unknown
2015-04-18T05:07:44+00:00vapierurn:md5:6301411b146640ce665cb9cb8d850a75
Message from benchan@chromium.org
2015-04-20T05:57:59+00:00Ben Chanurn:md5:7861da6233a18272af4b1a3cdb858e34
+1
On Fri, Apr 17, 2015 at 10:07 PM, <vapier@chromium.org> wrote:
> Reviewers: Ben Chan, Ted Mielczarek,
>
> Description:
> add aarch64 support to minidump-2-core
>
> The thread info expects the struct names as they expect in asm/ptrace.h,
> but the header doesn't include that, it includes sys/user.h. Rename the
> reg structs to match that header.
>
> Rename the elf_siginfo to _elf_siginfo to avoid conflicting with the one
> in the sys/procfs.h. It is only used locally in one place, so we don't
> need to update any callers.
>
> Otherwise, drop in aarch64 support into the minidump-2-core file.
>
> BUG=chromium:334368
>
> Please review this at https://breakpad.appspot.com/10734002/
>
> Affected files:
> M src/client/linux/dump_writer_common/thread_info.h
> M src/tools/linux/md2core/minidump-2-core.cc
>
>
> Index: src/client/linux/dump_writer_common/thread_info.h
> ===================================================================
> --- src/client/linux/dump_writer_common/thread_info.h (revision 1451)
> +++ src/client/linux/dump_writer_common/thread_info.h (working copy)
> @@ -64,10 +64,11 @@
> // Mimicking how strace does this(see syscall.c, search for GETREGS)
> struct user_regs regs;
> struct user_fpregs fpregs;
> +#error
> #elif defined(__aarch64__)
> - // Use the structures defined in <asm/ptrace.h>
> - struct user_pt_regs regs;
> - struct user_fpsimd_state fpregs;
> + // Use the structures defined in <sys/user.h>
> + struct user_regs_struct regs;
> + struct user_fpsimd_struct fpregs;
> #elif defined(__mips__)
> user_regs_struct regs;
> user_fpregs_struct fpregs;
> Index: src/tools/linux/md2core/minidump-2-core.cc
> ===================================================================
> --- src/tools/linux/md2core/minidump-2-core.cc (revision 1451)
> +++ src/tools/linux/md2core/minidump-2-core.cc (working copy)
> @@ -74,6 +74,8 @@
> #define ELF_ARCH EM_ARM
> #elif defined(__mips__)
> #define ELF_ARCH EM_MIPS
> +#elif defined(__aarch64__)
> + #define ELF_ARCH EM_AARCH64
> #endif
>
> #if defined(__arm__)
> @@ -133,14 +135,14 @@
> long tv_usec; /* Microseconds
> */
> } elf_timeval;
>
> -typedef struct elf_siginfo { /* Information about signal (unused)
> */
> +typedef struct _elf_siginfo { /* Information about signal (unused)
> */
> int32_t si_signo; /* Signal number
> */
> int32_t si_code; /* Extra code
> */
> int32_t si_errno; /* Errno
> */
> -} elf_siginfo;
> +} _elf_siginfo;
>
> typedef struct prstatus { /* Information about thread; includes CPU
> reg*/
> - elf_siginfo pr_info; /* Info associated with signal
> */
> + _elf_siginfo pr_info; /* Info associated with signal
> */
> uint16_t pr_cursig; /* Current signal
> */
> unsigned long pr_sigpend; /* Set of pending signals
> */
> unsigned long pr_sighold; /* Set of held signals
> */
> @@ -215,6 +217,9 @@
> #if defined(__i386__)
> user_fpxregs_struct fpxregs;
> #endif
> +#if defined(__aarch64__)
> + user_fpsimd_struct fpregs;
> +#endif
> uintptr_t stack_addr;
> const uint8_t* stack;
> size_t stack_length;
> @@ -364,6 +369,22 @@
> thread->regs.uregs[16] = rawregs->cpsr;
> thread->regs.uregs[17] = 0; // what is ORIG_r0 exactly?
> }
> +#elif defined(__aarch64__)
> +static void
> +ParseThreadRegisters(CrashedProcess::Thread* thread,
> + const MinidumpMemoryRange& range) {
> + const MDRawContextARM64* rawregs = range.GetData<MDRawContextARM64>(0);
> +
> + for (int i = 0; i < 31; ++i)
> + thread->regs.regs[i] = rawregs->iregs[i];
> + thread->regs.sp = rawregs->iregs[MD_CONTEXT_ARM64_REG_SP];
> + thread->regs.pc = rawregs->iregs[MD_CONTEXT_ARM64_REG_PC];
> + thread->regs.pstate = rawregs->cpsr;
> +
> + memcpy(thread->fpregs.vregs, rawregs->float_save.regs, 8 * 32);
> + thread->fpregs.fpsr = rawregs->float_save.fpsr;
> + thread->fpregs.fpcr = rawregs->float_save.fpcr;
> +}
> #elif defined(__mips__)
> static void
> ParseThreadRegisters(CrashedProcess::Thread* thread,
> @@ -452,6 +473,12 @@
> "This version of minidump-2-core only supports ARM
> (32bit).\n");
> _exit(1);
> }
> +#elif defined(__aarch64__)
> + if (sysinfo->processor_architecture != MD_CPU_ARCHITECTURE_ARM64) {
> + fprintf(stderr,
> + "This version of minidump-2-core only supports ARM
> (64bit).\n");
> + _exit(1);
> + }
> #elif defined(__mips__)
> if (sysinfo->processor_architecture != MD_CPU_ARCHITECTURE_MIPS) {
> fprintf(stderr,
>
>
>
Message from ted.mielczarek@gmail.com
2015-05-04T11:17:33+00:00Ted Mielczarekurn:md5:b764c44624869974e8482de77298d73c
One of these days someone is going to break down and fix minidump2core to work cross-cpu-architecture, right?
Message from vapier@chromium.org
2015-05-04T16:31:03+00:00vapierurn:md5:9d5d46e85473c311a70969facb4188cd
i don't have any more plans for big work on breakpad what with crashpad coming down the pike