From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 32EB53851C19 for ; Thu, 11 May 2023 16:07:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 32EB53851C19 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from r6.localdomain (82-217-174-174.cable.dynamic.v4.ziggo.nl [82.217.174.174]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id B9BD0313ACB3; Thu, 11 May 2023 18:07:06 +0200 (CEST) Received: by r6.localdomain (Postfix, from userid 1000) id 7EF88340154; Thu, 11 May 2023 18:07:06 +0200 (CEST) Message-ID: Subject: Re: [PATCH 4/5] stack: Fix stack unwind failure on mips From: Mark Wielaard To: ying.huang@oss.cipunited.com, elfutils-devel@sourceware.org Date: Thu, 11 May 2023 18:07:06 +0200 In-Reply-To: <20230411081141.1762395-5-ying.huang@oss.cipunited.com> References: <20230411081141.1762395-1-ying.huang@oss.cipunited.com> <20230411081141.1762395-5-ying.huang@oss.cipunited.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.1 (3.48.1-1.fc38) MIME-Version: 1.0 X-Spam-Status: No, score=-3035.0 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_BARRACUDACENTRAL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, On Tue, 2023-04-11 at 16:12 +0800, Ying Huang wrote: > From: Ying Huang >=20 > add abi_cfi, set_initial_registers_tid, unwind on mips. > "./src/stack -p PID" can show stack information > --- > backends/Makefile.am | 3 +- > backends/mips_cfi.c | 68 +++++++++++++++++++++++++++++++++ > backends/mips_init.c | 4 ++ > backends/mips_initreg.c | 70 ++++++++++++++++++++++++++++++++++ > backends/mips_unwind.c | 84 +++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 228 insertions(+), 1 deletion(-) > create mode 100644 backends/mips_cfi.c > create mode 100644 backends/mips_initreg.c > create mode 100644 backends/mips_unwind.c This looks good. Just two questions below. > diff --git a/backends/Makefile.am b/backends/Makefile.am > index 428a1a03..ddc31c9d 100644 > --- a/backends/Makefile.am > +++ b/backends/Makefile.am > @@ -100,7 +100,8 @@ loongarch_SRCS =3D loongarch_init.c loongarch_symbol.= c > =20 > arc_SRCS =3D arc_init.c arc_symbol.c > =20 > -mips_SRCS =3D mips_init.c mips_symbol.c mips_attrs.c > +mips_SRCS =3D mips_init.c mips_symbol.c mips_attrs.c mips_initreg.c \ > + mips_cfi.c mips_unwind.c OK =20 > libebl_backends_a_SOURCES =3D $(i386_SRCS) $(sh_SRCS) $(x86_64_SRCS) \ > $(ia64_SRCS) $(alpha_SRCS) $(arm_SRCS) \ > diff --git a/backends/mips_cfi.c b/backends/mips_cfi.c > new file mode 100644 > index 00000000..77132cc1 > --- /dev/null > +++ b/backends/mips_cfi.c > @@ -0,0 +1,68 @@ > +/* MIPS ABI-specified defaults for DWARF CFI. > + Copyright (C) 2009 Red Hat, Inc. > + Copyright (C) 2023 CIP United Inc. > + This file is part of elfutils. > + > + This file is free software; you can redistribute it and/or modify > + it under the terms of either > + > + * the GNU Lesser General Public License as published by the Free > + Software Foundation; either version 3 of the License, or (at > + your option) any later version > + > + or > + > + * the GNU General Public License as published by the Free > + Software Foundation; either version 2 of the License, or (at > + your option) any later version > + > + or both in parallel, as here. > + > + elfutils is distributed in the hope that it will be useful, but > + WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + General Public License for more details. > + > + You should have received copies of the GNU General Public License and > + the GNU Lesser General Public License along with this program. If > + not, see . */ > + > +#ifdef HAVE_CONFIG_H > +# include > +#endif > + > +#include > + > +#define BACKEND mips_ > +#include "libebl_CPU.h" > + > +int > +mips_abi_cfi (Ebl *ebl __attribute__ ((unused)), Dwarf_CIE *abi_info) > +{ > + static const uint8_t abi_cfi[] =3D > + { > + DW_CFA_def_cfa, ULEB128_7 (31), ULEB128_7 (0), > + /* Callee-saved regs. */ > + DW_CFA_same_value, ULEB128_7 (16), /* s0 */ > + DW_CFA_same_value, ULEB128_7 (17), /* s1 */ > + DW_CFA_same_value, ULEB128_7 (18), /* s2 */ > + DW_CFA_same_value, ULEB128_7 (19), /* s3 */ > + DW_CFA_same_value, ULEB128_7 (20), /* s4 */ > + DW_CFA_same_value, ULEB128_7 (21), /* s5 */ > + DW_CFA_same_value, ULEB128_7 (22), /* s6 */ > + DW_CFA_same_value, ULEB128_7 (23), /* s7 */ > + DW_CFA_same_value, ULEB128_7 (28), /* gp */ > + DW_CFA_same_value, ULEB128_7 (29), /* sp */ > + DW_CFA_same_value, ULEB128_7 (30), /* fp */ > + > + DW_CFA_val_offset, ULEB128_7 (29), ULEB128_7 (0), > + }; > + > + abi_info->initial_instructions =3D abi_cfi; > + abi_info->initial_instructions_end =3D &abi_cfi[sizeof abi_cfi]; > + abi_info->data_alignment_factor =3D -4; > + > + abi_info->return_address_register =3D 31; /* %ra */ > + > + return 0; > +} Looks good, but do you have a reference to the ABI docs would be nice to add an URL as comment for people to double check. > diff --git a/backends/mips_init.c b/backends/mips_init.c > index 4c2f21b9..3caa9fee 100644 > --- a/backends/mips_init.c > +++ b/backends/mips_init.c > @@ -55,5 +55,9 @@ mips_init (Elf *elf __attribute__ ((unused)), > HOOK (eh, check_object_attribute); > HOOK (eh, check_special_symbol); > HOOK (eh, check_reloc_target_type); > + HOOK (eh, set_initial_registers_tid); > + HOOK (eh, abi_cfi); > + HOOK (eh, unwind); > + eh->frame_nregs =3D 32; > return eh; > } OK > diff --git a/backends/mips_initreg.c b/backends/mips_initreg.c > new file mode 100644 > index 00000000..31b8de13 > --- /dev/null > +++ b/backends/mips_initreg.c > @@ -0,0 +1,70 @@ > +/* Fetch live process registers from TID. > + Copyright (C) 2023 CIP United Inc. > + This file is part of elfutils. > + > + This file is free software; you can redistribute it and/or modify > + it under the terms of either > + > + * the GNU Lesser General Public License as published by the Free > + Software Foundation; either version 3 of the License, or (at > + your option) any later version > + > + or > + > + * the GNU General Public License as published by the Free > + Software Foundation; either version 2 of the License, or (at > + your option) any later version > + > + or both in parallel, as here. > + > + elfutils is distributed in the hope that it will be useful, but > + WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + General Public License for more details. > + > + You should have received copies of the GNU General Public License and > + the GNU Lesser General Public License along with this program. If > + not, see . */ > + > +#ifdef HAVE_CONFIG_H > +# include > +#endif > + > +#include > +#if (defined(mips) || defined(__mips) || defined(__mips__) || defined(MI= PS) || defined(__MIPS__)) && defined(__linux__) > +# include > +# include > +#endif > + > +#define BACKEND mips_ > +#include "libebl_CPU.h" > +#include > + > + > +bool > +mips_set_initial_registers_tid (pid_t tid __attribute__ ((unused)), > + ebl_tid_registers_t *setfunc __attribute__ ((unused)), > + void *arg __attribute__ ((unused))) > +{ > +#if (!defined(mips) && !defined(__mips) && !defined(__mips__) && !define= d(MIPS) && !defined(__MIPS__)) || !defined(__linux__) > + return false; > +#else /* __mips__ */ > +/* For PTRACE_GETREGS */ > +struct pt_regs { > + uint64_t regs[32]; > + uint64_t lo; > + uint64_t hi; > + uint64_t pc; > + uint64_t badvaddr; > + uint64_t cause; > + uint64_t status; > +}; Isn't this defined in some standard (or glibc/linux specific) header? > + struct pt_regs gregs; > + if (ptrace (PTRACE_GETREGS, tid, 0, &gregs) !=3D 0) > + return false; > + if (! setfunc (-1, 1, (Dwarf_Word *) &gregs.pc, arg)) > + return false; > + return setfunc (0, 32, (Dwarf_Word *) &gregs.regs[0], arg); > +#endif /* __mips__ */ > +} OK > diff --git a/backends/mips_unwind.c b/backends/mips_unwind.c > new file mode 100644 > index 00000000..d09db3a9 > --- /dev/null > +++ b/backends/mips_unwind.c > @@ -0,0 +1,84 @@ > +/* Get previous frame state for an existing frame state. > + Copyright (C) 2016 The Qt Company Ltd. > + Copyright (C) 2023 CIP United Inc. > + This file is part of elfutils. > + > + This file is free software; you can redistribute it and/or modify > + it under the terms of either > + > + * the GNU Lesser General Public License as published by the Free > + Software Foundation; either version 3 of the License, or (at > + your option) any later version > + > + or > + > + * the GNU General Public License as published by the Free > + Software Foundation; either version 2 of the License, or (at > + your option) any later version > + > + or both in parallel, as here. > + > + elfutils is distributed in the hope that it will be useful, but > + WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + General Public License for more details. > + > + You should have received copies of the GNU General Public License and > + the GNU Lesser General Public License along with this program. If > + not, see . */ > + > +#ifdef HAVE_CONFIG_H > +# include > +#endif > + > +#define BACKEND mips_ > +#define SP_REG 29 > +#define FP_REG 30 > +#define LR_REG 31 > +#define FP_OFFSET 0 > +#define LR_OFFSET 8 > +#define SP_OFFSET 16 > + > +#include "libebl_CPU.h" > + > +/* There was no CFI. Maybe we happen to have a frame pointer and can unw= ind from that? */ > + > +bool > +EBLHOOK(unwind) (Ebl *ebl __attribute__ ((unused)), Dwarf_Addr pc __attr= ibute__ ((unused)), > + ebl_tid_registers_t *setfunc, ebl_tid_registers_get_t *= getfunc, > + ebl_pid_memory_read_t *readfunc, void *arg, > + bool *signal_framep __attribute__ ((unused))) > +{ > + Dwarf_Word fp, lr, sp; > + > + if (!getfunc(LR_REG, 1, &lr, arg)) > + return false; > + > + if (lr =3D=3D 0 || !setfunc(-1, 1, &lr, arg)) > + return false; > + > + if (!getfunc(FP_REG, 1, &fp, arg)) > + fp =3D 0; > + > + if (!getfunc(SP_REG, 1, &sp, arg)) > + sp =3D 0; > + > + Dwarf_Word newLr, newFp, newSp; > + > + if (!readfunc(fp + LR_OFFSET, &newLr, arg)) > + newLr =3D 0; > + > + if (!readfunc(fp + FP_OFFSET, &newFp, arg)) > + newFp =3D 0; > + > + newSp =3D fp + SP_OFFSET; > + > + // These are not fatal if they don't work. They will just prevent unwi= nding at the next frame. > + setfunc(LR_REG, 1, &newLr, arg); > + setfunc(FP_REG, 1, &newFp, arg); > + setfunc(SP_REG, 1, &newSp, arg); > + > + // If the fp is invalid, we might still have a valid lr. > + // But if the fp is valid, then the stack should be moving in the righ= t direction. > + return fp =3D=3D 0 || newSp > sp; > +} Looks good. Thanks, Mark