From patchwork Mon Jan 27 20:35:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 37580 Received: (qmail 48947 invoked by alias); 27 Jan 2020 20:36:00 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 48826 invoked by uid 89); 27 Jan 2020 20:36:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-qt1-f193.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:references:from:autocrypt:subject:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=irwF50pTKRbC6qXFrvlaYX1M8dBH0w5Vj+PfvI5+tqg=; b=LBKYuWKF9Hsm99uaiMSiRI0yLanm9dKTgGmHhY4mE5NDQq3zA1IkbCvoG73JMMvhvo ZOy7A817Ze3JfI/glcp5EaAGGn8Vv7jdm/T7Cb0/cXAaNMSXu87MLa3Z2S6lRjqwEnQU LKFZvdYhvQsVWR11+Nt5PN8dqAq12t87Mg1oIgjbPYyznfgthvNK0SMlkAE6Qa9qhtR0 P1v9nfVr1kcTPCgo3hHlGErh8BKAtkTC4TQkEFwDr2KOaorxpJWAKYRL0h0v/ZOAUeYx eReTWMLcHkR7DhDrG/oq8bml+hKjbyAoqd9Vb/IoSC72hF2LeIMODwhtnfBHX60rJrhE vgHw== Return-Path: To: Joseph Myers , GNU C Library References: From: Adhemerval Zanella Subject: Re: Build raise with -fasynchronous-unwind-tables Message-ID: Date: Mon, 27 Jan 2020 17:35:52 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: On 24/01/2020 23:18, Joseph Myers wrote: > On Fri, 24 Jan 2020, Joseph Myers wrote: > >> causes the test for pass for Arm. For MIPS, the test still does not >> pass (the backtrace has an address that is 2 bytes after the "address >> in signal handler", for unknown reasons), although the patch allows a >> longer backtrace to be produced. > > I now understand the MIPS issue: libgcc/config/mips/linux-unwind.h adds 2 > to the return address for a signal frame. > > The question then is how best to fix that MIPS issue. Adding 2 in the > MIPS sigcontext_get_pc would fix things for the testcase, and be safe for > the use in debug/segfault.c. I'm less sure, however, what's safe for the > use of sigcontext_get_pc in sysdeps/unix/sysv/linux/profil-counter.h - > what exactly that use requires regarding the return value of > sigcontext_get_pc. > My plan to refactor this code and add sigcontext_get_pc is that the current proposal to fix BZ#12683 requires the SIGCANCEL handler to obtain where exactly the syscall was interrupted to decide whether or not to act on cancellation request. The tst-sigcontext-get_pc idea is to check if kernel SA_SIGINFO returns the expected interrupted instruction using a different mechanism (libgcc). And my understanding is what really need to be fixed is the backtrace result on MIPS, since the expected correct value of the interrupted instruction by a signal frame is the one set by the kernel in the create signal frame. Changing the value of sigcontext_get_pc to take in consideration the libgcc added value might be ok for profil-counter.h but it definitely wrong for BZ#12683 fix (it will mostly likely create cancellation request with false-positive side-effects). Below a proposed fix: --- [PATCH] mips: Fix bracktrace result for signal frames The MIPS fallback code to handle a frame where its FDE can not be obtained (for instance a signal frame) is to read the kernel allocated signal frame and add 2 to the value of 'sc_pc' [1]. The added value is used to recognize an end of an EH region on mips16 [2]. For default MIPS encoding we can mask off the 2 least significant bit to get the faulting instruction. However, on mips16/micromips there is no easy way to recognize when libgcc adds this extra offset. Checked with backtrace and tst-sigcontext-get_pc tests on mips-linux-gnu and mips64-linux-gnu. [1] libgcc/config/mips/linux-unwind.h from gcc code. [2] gcc/config/mips/mips.h from gcc code. */ --- debug/backtrace.c | 3 +- sysdeps/generic/unwind-arch.h | 30 +++++++++++++++ sysdeps/unix/sysv/linux/mips/unwind-arch.h | 45 ++++++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 sysdeps/generic/unwind-arch.h create mode 100644 sysdeps/unix/sysv/linux/mips/unwind-arch.h diff --git a/debug/backtrace.c b/debug/backtrace.c index cc4b9a5c90..957558fd88 100644 --- a/debug/backtrace.c +++ b/debug/backtrace.c @@ -23,6 +23,7 @@ #include #include #include +#include struct trace_arg { @@ -77,7 +78,7 @@ backtrace_helper (struct _Unwind_Context *ctx, void *a) Skip it. */ if (arg->cnt != -1) { - arg->array[arg->cnt] = (void *) unwind_getip (ctx); + arg->array[arg->cnt] = unwind_getip_masked (unwind_getip (ctx)); /* Check whether we make any progress. */ _Unwind_Word cfa = unwind_getcfa (ctx); diff --git a/sysdeps/generic/unwind-arch.h b/sysdeps/generic/unwind-arch.h new file mode 100644 index 0000000000..d1030c368a --- /dev/null +++ b/sysdeps/generic/unwind-arch.h @@ -0,0 +1,30 @@ +/* Return backtrace of current program state. Arch-specific bits. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library 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 + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#ifndef _UNWIND_ARCH_H +#define _UNWIND_ARCH_H + +#include + +static inline void * +unwind_getip_masked (_Unwind_Ptr ip) +{ + return (void*) ip; +} + +#endif diff --git a/sysdeps/unix/sysv/linux/mips/unwind-arch.h b/sysdeps/unix/sysv/linux/mips/unwind-arch.h new file mode 100644 index 0000000000..0f5a1a83a4 --- /dev/null +++ b/sysdeps/unix/sysv/linux/mips/unwind-arch.h @@ -0,0 +1,45 @@ +/* Return backtrace of current program state. Arch-specific bits. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library 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 + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#ifndef _UNWIND_ARCH_H +#define _UNWIND_ARCH_H + +#include + +/* The MIPS fallback code to handle a frame where its FDE can not be obtained + (for instance a signal frame) is to read the kernel allocated signal frame + and add 2 to the value of 'sc_pc' [1]. The added value is used to + recognize an end of an EH region on mips16 [2]. + + For default MIPS encoding we can mask off the 2 least significant bit to + get the faulting instruction. However, on mips16/micromips there is no + easy way to recognize when libgcc adds this extra offset. + + [1] libgcc/config/mips/linux-unwind.h from gcc code. + [2] gcc/config/mips/mips.h from gcc code. */ + +static inline void * +unwind_getip_masked (_Unwind_Ptr ip) +{ +#ifndef __mips16 + ip &= -4; +#endif + return (void*) ip; +} +