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

Message ID 00a31f83fdc74473ba1eaac8fa49d1f46a2d706c.1581613260.git.fweimer@redhat.com
State New
Headers show
Series
  • Unify dynamic loading of the libgcc_s unwinder
Related show

Commit Message

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

-- 
2.24.1

Comments

Stefan Liebler Feb. 14, 2020, 11:54 a.m. | #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;
 }