diff mbox

Build raise with -fasynchronous-unwind-tables

Message ID f1130de1-fe40-d9b8-720c-dc906e1c7f9e@linaro.org
State Dropped
Headers show

Commit Message

Adhemerval Zanella Jan. 27, 2020, 8:35 p.m. UTC
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 mbox

Patch

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 <gnu/lib-names.h>
 #include <stdlib.h>
 #include <unwind.h>
+#include <unwind-arch.h>
 
 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
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _UNWIND_ARCH_H
+#define _UNWIND_ARCH_H
+
+#include <unwind.h>
+
+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
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _UNWIND_ARCH_H
+#define _UNWIND_ARCH_H
+
+#include <unwind.h>
+
+/* 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;
+}
+