[07/11] s390: Implement backtrace on top of <unwind-link.h>

Message ID 00a31f83fdc74473ba1eaac8fa49d1f46a2d706c.1581613260.git.fweimer@redhat.com
State Committed
Headers

Commit Message

Florian Weimer Feb. 13, 2020, 5:08 p.m. UTC
  ---
 sysdeps/s390/s390-32/backtrace.c | 45 ++++++++++---------------------
 sysdeps/s390/s390-64/backtrace.c | 46 ++++++++++----------------------
 2 files changed, 28 insertions(+), 63 deletions(-)
  

Comments

Stefan Liebler Feb. 14, 2020, 11:54 a.m. UTC | #1
Hi Florian,

I've applied your patch series and also successfully run the testsuite 
on s390x/s390.

For s390, this is okay.



Just as info for future readers regarding
"The S/390 version preserves the backchain fallback for now.":
Here is the previous discussion:
https://www.sourceware.org/ml/libc-alpha/2020-02/msg00453.html
As soon as I have any news, I will reply there.

If we remove the backchain fallback. I assume the s390 specific 
backtrace.c files can be removed. Thus the debug/backtrace.c would be 
used. But I have to double check at this time.

If we still need the backchain fallback, I would either update the 
implementation in order to also get those "features":
/* Check whether we make any progress.  */
/* _Unwind_Backtrace seems to put NULL address above
    _start.  Fix it up here.  */

Or perhaps even better, add an architecture-fallback-hook in the default 
implementation.


Bye,
Stefan

On 2/13/20 6:08 PM, Florian Weimer wrote:
> ---
>   sysdeps/s390/s390-32/backtrace.c | 45 ++++++++++---------------------
>   sysdeps/s390/s390-64/backtrace.c | 46 ++++++++++----------------------
>   2 files changed, 28 insertions(+), 63 deletions(-)
> 
> diff --git a/sysdeps/s390/s390-32/backtrace.c b/sysdeps/s390/s390-32/backtrace.c
> index 497e4f8875..21da6761f2 100644
> --- a/sysdeps/s390/s390-32/backtrace.c
> +++ b/sysdeps/s390/s390-32/backtrace.c
> @@ -17,12 +17,10 @@
>      License along with the GNU C Library; if not, see
>      <https://www.gnu.org/licenses/>.  */
>   
> -#include <libc-lock.h>
> -#include <dlfcn.h>
>   #include <execinfo.h>
>   #include <stddef.h>
>   #include <stdlib.h>
> -#include <unwind.h>
> +#include <unwind-link.h>
>   
>   /* This is a global variable set at program start time.  It marks the
>      highest used stack address.  */
> @@ -57,27 +55,11 @@ struct layout
>   struct trace_arg
>   {
>     void **array;
> +  struct unwind_link *unwind_link;
>     int cnt, size;
>   };
>   
>   #ifdef SHARED
> -static _Unwind_Reason_Code (*unwind_backtrace) (_Unwind_Trace_Fn, void *);
> -static _Unwind_Ptr (*unwind_getip) (struct _Unwind_Context *);
> -
> -static void
> -init (void)
> -{
> -  void *handle = __libc_dlopen ("libgcc_s.so.1");
> -
> -  if (handle == NULL)
> -    return;
> -
> -  unwind_backtrace = __libc_dlsym (handle, "_Unwind_Backtrace");
> -  unwind_getip = __libc_dlsym (handle, "_Unwind_GetIP");
> -  if (unwind_getip == NULL)
> -    unwind_backtrace = NULL;
> -}
> -
>   static int
>   __backchain_backtrace (void **array, int size)
>   {
> @@ -103,9 +85,6 @@ __backchain_backtrace (void **array, int size)
>   
>     return cnt;
>   }
> -#else
> -# define unwind_backtrace _Unwind_Backtrace
> -# define unwind_getip _Unwind_GetIP
>   #endif
>   
>   static _Unwind_Reason_Code
> @@ -116,7 +95,8 @@ backtrace_helper (struct _Unwind_Context *ctx, void *a)
>     /* We are first called with address in the __backtrace function.
>        Skip it.  */
>     if (arg->cnt != -1)
> -    arg->array[arg->cnt] = (void *) unwind_getip (ctx);
> +    arg->array[arg->cnt]
> +      = (void *) UNWIND_LINK_PTR (arg->unwind_link, _Unwind_GetIP) (ctx);
>     if (++arg->cnt == arg->size)
>       return _URC_END_OF_STACK;
>     return _URC_NO_REASON;
> @@ -125,21 +105,24 @@ backtrace_helper (struct _Unwind_Context *ctx, void *a)
>   int
>   __backtrace (void **array, int size)
>   {
> -  struct trace_arg arg = { .array = array, .size = size, .cnt = -1 };
> +  struct trace_arg arg =
> +    {
> +     .array = array,
> +     .unwind_link = __libc_unwind_link_get (),
> +     .size = size,
> +     .cnt = -1
> +    };
>   
>     if (size <= 0)
>       return 0;
>   
>   #ifdef SHARED
> -  __libc_once_define (static, once);
> -
> -  __libc_once (once, init);
> -
> -  if (unwind_backtrace == NULL)
> +  if (arg.unwind_link == NULL)
>       return __backchain_backtrace (array, size);
>   #endif
>   
> -  unwind_backtrace (backtrace_helper, &arg);
> +  UNWIND_LINK_PTR (arg.unwind_link, _Unwind_Backtrace)
> +    (backtrace_helper, &arg);
>   
>     return arg.cnt != -1 ? arg.cnt : 0;
>   }
> diff --git a/sysdeps/s390/s390-64/backtrace.c b/sysdeps/s390/s390-64/backtrace.c
> index 5d14e01280..1b8818f219 100644
> --- a/sysdeps/s390/s390-64/backtrace.c
> +++ b/sysdeps/s390/s390-64/backtrace.c
> @@ -17,13 +17,10 @@
>      License along with the GNU C Library; if not, see
>      <https://www.gnu.org/licenses/>.  */
>   
> -#include <libc-lock.h>
> -#include <dlfcn.h>
>   #include <execinfo.h>
>   #include <stddef.h>
>   #include <stdlib.h>
> -#include <unwind.h>
> -
> +#include <unwind-link.h>
>   
>   /* This is a global variable set at program start time.  It marks the
>      highest used stack address.  */
> @@ -56,27 +53,11 @@ struct layout
>   struct trace_arg
>   {
>     void **array;
> +  struct unwind_link *unwind_link;
>     int cnt, size;
>   };
>   
>   #ifdef SHARED
> -static _Unwind_Reason_Code (*unwind_backtrace) (_Unwind_Trace_Fn, void *);
> -static _Unwind_Ptr (*unwind_getip) (struct _Unwind_Context *);
> -
> -static void
> -init (void)
> -{
> -  void *handle = __libc_dlopen ("libgcc_s.so.1");
> -
> -  if (handle == NULL)
> -    return;
> -
> -  unwind_backtrace = __libc_dlsym (handle, "_Unwind_Backtrace");
> -  unwind_getip = __libc_dlsym (handle, "_Unwind_GetIP");
> -  if (unwind_getip == NULL)
> -    unwind_backtrace = NULL;
> -}
> -
>   static int
>   __backchain_backtrace (void **array, int size)
>   {
> @@ -102,9 +83,6 @@ __backchain_backtrace (void **array, int size)
>   
>     return cnt;
>   }
> -#else
> -# define unwind_backtrace _Unwind_Backtrace
> -# define unwind_getip _Unwind_GetIP
>   #endif
>   
>   static _Unwind_Reason_Code
> @@ -115,7 +93,8 @@ backtrace_helper (struct _Unwind_Context *ctx, void *a)
>     /* We are first called with address in the __backtrace function.
>        Skip it.  */
>     if (arg->cnt != -1)
> -    arg->array[arg->cnt] = (void *) unwind_getip (ctx);
> +    arg->array[arg->cnt]
> +      = (void *) UNWIND_LINK_PTR (arg->unwind_link, _Unwind_GetIP) (ctx);
>     if (++arg->cnt == arg->size)
>       return _URC_END_OF_STACK;
>     return _URC_NO_REASON;
> @@ -124,21 +103,24 @@ backtrace_helper (struct _Unwind_Context *ctx, void *a)
>   int
>   __backtrace (void **array, int size)
>   {
> -  struct trace_arg arg = { .array = array, .size = size, .cnt = -1 };
> +  struct trace_arg arg =
> +    {
> +     .array = array,
> +     .unwind_link = __libc_unwind_link_get (),
> +     .size = size,
> +     .cnt = -1
> +    };
>   
>     if (size <= 0)
>       return 0;
>   
>   #ifdef SHARED
> -  __libc_once_define (static, once);
> -
> -  __libc_once (once, init);
> -
> -  if (unwind_backtrace == NULL)
> +  if (arg.unwind_link == NULL)
>       return __backchain_backtrace (array, size);
>   #endif
>   
> -  unwind_backtrace (backtrace_helper, &arg);
> +  UNWIND_LINK_PTR (arg.unwind_link, _Unwind_Backtrace)
> +    (backtrace_helper, &arg);
>   
>     return arg.cnt != -1 ? arg.cnt : 0;
>   }
>
  

Patch

diff --git a/sysdeps/s390/s390-32/backtrace.c b/sysdeps/s390/s390-32/backtrace.c
index 497e4f8875..21da6761f2 100644
--- a/sysdeps/s390/s390-32/backtrace.c
+++ b/sysdeps/s390/s390-32/backtrace.c
@@ -17,12 +17,10 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <libc-lock.h>
-#include <dlfcn.h>
 #include <execinfo.h>
 #include <stddef.h>
 #include <stdlib.h>
-#include <unwind.h>
+#include <unwind-link.h>
 
 /* This is a global variable set at program start time.  It marks the
    highest used stack address.  */
@@ -57,27 +55,11 @@  struct layout
 struct trace_arg
 {
   void **array;
+  struct unwind_link *unwind_link;
   int cnt, size;
 };
 
 #ifdef SHARED
-static _Unwind_Reason_Code (*unwind_backtrace) (_Unwind_Trace_Fn, void *);
-static _Unwind_Ptr (*unwind_getip) (struct _Unwind_Context *);
-
-static void
-init (void)
-{
-  void *handle = __libc_dlopen ("libgcc_s.so.1");
-
-  if (handle == NULL)
-    return;
-
-  unwind_backtrace = __libc_dlsym (handle, "_Unwind_Backtrace");
-  unwind_getip = __libc_dlsym (handle, "_Unwind_GetIP");
-  if (unwind_getip == NULL)
-    unwind_backtrace = NULL;
-}
-
 static int
 __backchain_backtrace (void **array, int size)
 {
@@ -103,9 +85,6 @@  __backchain_backtrace (void **array, int size)
 
   return cnt;
 }
-#else
-# define unwind_backtrace _Unwind_Backtrace
-# define unwind_getip _Unwind_GetIP
 #endif
 
 static _Unwind_Reason_Code
@@ -116,7 +95,8 @@  backtrace_helper (struct _Unwind_Context *ctx, void *a)
   /* We are first called with address in the __backtrace function.
      Skip it.  */
   if (arg->cnt != -1)
-    arg->array[arg->cnt] = (void *) unwind_getip (ctx);
+    arg->array[arg->cnt]
+      = (void *) UNWIND_LINK_PTR (arg->unwind_link, _Unwind_GetIP) (ctx);
   if (++arg->cnt == arg->size)
     return _URC_END_OF_STACK;
   return _URC_NO_REASON;
@@ -125,21 +105,24 @@  backtrace_helper (struct _Unwind_Context *ctx, void *a)
 int
 __backtrace (void **array, int size)
 {
-  struct trace_arg arg = { .array = array, .size = size, .cnt = -1 };
+  struct trace_arg arg =
+    {
+     .array = array,
+     .unwind_link = __libc_unwind_link_get (),
+     .size = size,
+     .cnt = -1
+    };
 
   if (size <= 0)
     return 0;
 
 #ifdef SHARED
-  __libc_once_define (static, once);
-
-  __libc_once (once, init);
-
-  if (unwind_backtrace == NULL)
+  if (arg.unwind_link == NULL)
     return __backchain_backtrace (array, size);
 #endif
 
-  unwind_backtrace (backtrace_helper, &arg);
+  UNWIND_LINK_PTR (arg.unwind_link, _Unwind_Backtrace)
+    (backtrace_helper, &arg);
 
   return arg.cnt != -1 ? arg.cnt : 0;
 }
diff --git a/sysdeps/s390/s390-64/backtrace.c b/sysdeps/s390/s390-64/backtrace.c
index 5d14e01280..1b8818f219 100644
--- a/sysdeps/s390/s390-64/backtrace.c
+++ b/sysdeps/s390/s390-64/backtrace.c
@@ -17,13 +17,10 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <libc-lock.h>
-#include <dlfcn.h>
 #include <execinfo.h>
 #include <stddef.h>
 #include <stdlib.h>
-#include <unwind.h>
-
+#include <unwind-link.h>
 
 /* This is a global variable set at program start time.  It marks the
    highest used stack address.  */
@@ -56,27 +53,11 @@  struct layout
 struct trace_arg
 {
   void **array;
+  struct unwind_link *unwind_link;
   int cnt, size;
 };
 
 #ifdef SHARED
-static _Unwind_Reason_Code (*unwind_backtrace) (_Unwind_Trace_Fn, void *);
-static _Unwind_Ptr (*unwind_getip) (struct _Unwind_Context *);
-
-static void
-init (void)
-{
-  void *handle = __libc_dlopen ("libgcc_s.so.1");
-
-  if (handle == NULL)
-    return;
-
-  unwind_backtrace = __libc_dlsym (handle, "_Unwind_Backtrace");
-  unwind_getip = __libc_dlsym (handle, "_Unwind_GetIP");
-  if (unwind_getip == NULL)
-    unwind_backtrace = NULL;
-}
-
 static int
 __backchain_backtrace (void **array, int size)
 {
@@ -102,9 +83,6 @@  __backchain_backtrace (void **array, int size)
 
   return cnt;
 }
-#else
-# define unwind_backtrace _Unwind_Backtrace
-# define unwind_getip _Unwind_GetIP
 #endif
 
 static _Unwind_Reason_Code
@@ -115,7 +93,8 @@  backtrace_helper (struct _Unwind_Context *ctx, void *a)
   /* We are first called with address in the __backtrace function.
      Skip it.  */
   if (arg->cnt != -1)
-    arg->array[arg->cnt] = (void *) unwind_getip (ctx);
+    arg->array[arg->cnt]
+      = (void *) UNWIND_LINK_PTR (arg->unwind_link, _Unwind_GetIP) (ctx);
   if (++arg->cnt == arg->size)
     return _URC_END_OF_STACK;
   return _URC_NO_REASON;
@@ -124,21 +103,24 @@  backtrace_helper (struct _Unwind_Context *ctx, void *a)
 int
 __backtrace (void **array, int size)
 {
-  struct trace_arg arg = { .array = array, .size = size, .cnt = -1 };
+  struct trace_arg arg =
+    {
+     .array = array,
+     .unwind_link = __libc_unwind_link_get (),
+     .size = size,
+     .cnt = -1
+    };
 
   if (size <= 0)
     return 0;
 
 #ifdef SHARED
-  __libc_once_define (static, once);
-
-  __libc_once (once, init);
-
-  if (unwind_backtrace == NULL)
+  if (arg.unwind_link == NULL)
     return __backchain_backtrace (array, size);
 #endif
 
-  unwind_backtrace (backtrace_helper, &arg);
+  UNWIND_LINK_PTR (arg.unwind_link, _Unwind_Backtrace)
+    (backtrace_helper, &arg);
 
   return arg.cnt != -1 ? arg.cnt : 0;
 }