diff mbox

S/390: Get rid of warning: the comparision will always evaluate as false

Message ID m1107c$q5c$1@ger.gmane.org
State Committed
Headers show

Commit Message

Stefan Liebler Oct. 7, 2014, 3:18 p.m. UTC
Hi,

On s390-32|64 i get the following build warning
"sysdeps/s390/s390-64/backtrace.c:133:24: warning: the comparison will 
always evaluate as ‘false’ for the address of ‘_Unwind_Backtrace’ will 
never be NULL [-Waddress]" for the non SHARED case.
In SHARED case, the _Unwind_Backtrace function is loaded via dlsym() and 
thus needs to be NULL-checked (if (unwind_backtrace == NULL)) in order 
to fallback to __backchain_backtrace() function.

In non SHARED case, the #define unwind_backtrace _Unwind_Backtrace was 
introduced with "* sysdeps/s390/s390-32/backtrace.c (init): Guard with 
#ifdef SHARED." (Roland McGrath <roland@gnu.org>  2004-06-11 22:12:55) 
and the NULL-check will never be false for _Unwind_Backtrace() function. 
Thus this NULL-check is now only evaluated in SHARED-case.

The __backchain_backtrace function is only used in the SHARED-case,
if the unwind-function is not found. Thus it is now only compiled in the 
SHARED-case, to avoid the warning: warning: ‘__backchain_backtrace’ 
defined but not used [-Wunused-function].

On s390-32, this function is declared as static, but not on s390-64.
These functions were introduced in the same patch
"Update." (Ulrich Drepper <drepper@redhat.com>  2003-12-06 01:20:16)
and the Changelog from Martin Schwidefsky says:
* sysdeps/s390/s390-32/backtrace.c (trace_arg): New structure.
(unwind_backtrace, unwind_getip): New variables.
(init, __backchain_backtrace, backtrace_helper): New functions.
(__backtrace): Use unwind info for backtrace instead of backchain
walking if the unwind functions can be found.
* sysdeps/s390/s390-64/backtrace.c: Likewise.

On s390-64 it isn´t exported in the shared library, thus on s390-64 it 
is now declared as static too!

Bye Stefan

---
2014-10-07  Stefan Liebler  <stli@linux.vnet.ibm.com>

	* sysdeps/s390/s390-32/backtrace.c (__backtrace):
	Check for unwind_backtrace ==  NULL only in SHARED case.
	(__backchain_backtrace): Compile only in SHARED case.
	* sysdeps/s390/s390-64/backtrace.c (__backtrace):
	Likewise.
	(__backchain_backtrace): Declare as static.

Comments

Stefan Liebler Oct. 15, 2014, 6:33 a.m. UTC | #1
ping

On 10/07/2014 05:18 PM, Stefan Liebler wrote:
> Hi,
>
> On s390-32|64 i get the following build warning
> "sysdeps/s390/s390-64/backtrace.c:133:24: warning: the comparison will
> always evaluate as ‘false’ for the address of ‘_Unwind_Backtrace’ will
> never be NULL [-Waddress]" for the non SHARED case.
> In SHARED case, the _Unwind_Backtrace function is loaded via dlsym() and
> thus needs to be NULL-checked (if (unwind_backtrace == NULL)) in order
> to fallback to __backchain_backtrace() function.
>
> In non SHARED case, the #define unwind_backtrace _Unwind_Backtrace was
> introduced with "* sysdeps/s390/s390-32/backtrace.c (init): Guard with
> #ifdef SHARED." (Roland McGrath <roland@gnu.org>  2004-06-11 22:12:55)
> and the NULL-check will never be false for _Unwind_Backtrace() function.
> Thus this NULL-check is now only evaluated in SHARED-case.
>
> The __backchain_backtrace function is only used in the SHARED-case,
> if the unwind-function is not found. Thus it is now only compiled in the
> SHARED-case, to avoid the warning: warning: ‘__backchain_backtrace’
> defined but not used [-Wunused-function].
>
> On s390-32, this function is declared as static, but not on s390-64.
> These functions were introduced in the same patch
> "Update." (Ulrich Drepper <drepper@redhat.com>  2003-12-06 01:20:16)
> and the Changelog from Martin Schwidefsky says:
> * sysdeps/s390/s390-32/backtrace.c (trace_arg): New structure.
> (unwind_backtrace, unwind_getip): New variables.
> (init, __backchain_backtrace, backtrace_helper): New functions.
> (__backtrace): Use unwind info for backtrace instead of backchain
> walking if the unwind functions can be found.
> * sysdeps/s390/s390-64/backtrace.c: Likewise.
>
> On s390-64 it isn´t exported in the shared library, thus on s390-64 it
> is now declared as static too!
>
> Bye Stefan
>
> ---
> 2014-10-07  Stefan Liebler  <stli@linux.vnet.ibm.com>
>
>      * sysdeps/s390/s390-32/backtrace.c (__backtrace):
>      Check for unwind_backtrace ==  NULL only in SHARED case.
>      (__backchain_backtrace): Compile only in SHARED case.
>      * sysdeps/s390/s390-64/backtrace.c (__backtrace):
>      Likewise.
>      (__backchain_backtrace): Declare as static.
Andreas Krebbel Nov. 13, 2014, 9:48 a.m. UTC | #2
On 10/07/2014 05:18 PM, Stefan Liebler wrote:
> 2014-10-07  Stefan Liebler  <stli@linux.vnet.ibm.com>
> 
> 	* sysdeps/s390/s390-32/backtrace.c (__backtrace):
> 	Check for unwind_backtrace ==  NULL only in SHARED case.
> 	(__backchain_backtrace): Compile only in SHARED case.
> 	* sysdeps/s390/s390-64/backtrace.c (__backtrace):
> 	Likewise.
> 	(__backchain_backtrace): Declare as static.
> 

Applied. Thanks!

-Andreas-
diff mbox

Patch

diff --git a/sysdeps/s390/s390-32/backtrace.c b/sysdeps/s390/s390-32/backtrace.c
index 3ade10c..e3122cf 100644
--- a/sysdeps/s390/s390-32/backtrace.c
+++ b/sysdeps/s390/s390-32/backtrace.c
@@ -77,10 +77,6 @@  init (void)
   if (unwind_getip == NULL)
     unwind_backtrace = NULL;
 }
-#else
-# define unwind_backtrace _Unwind_Backtrace
-# define unwind_getip _Unwind_GetIP
-#endif
 
 static int
 __backchain_backtrace (void **array, int size)
@@ -107,6 +103,10 @@  __backchain_backtrace (void **array, int size)
 
   return cnt;
 }
+#else
+# define unwind_backtrace _Unwind_Backtrace
+# define unwind_getip _Unwind_GetIP
+#endif
 
 static _Unwind_Reason_Code
 backtrace_helper (struct _Unwind_Context *ctx, void *a)
@@ -130,9 +130,10 @@  __backtrace (void **array, int size)
   __libc_once_define (static, once);
 
   __libc_once (once, init);
-#endif
+
   if (unwind_backtrace == NULL)
     return __backchain_backtrace (array, size);
+#endif
 
   if (size >= 1)
     unwind_backtrace (backtrace_helper, &arg);
diff --git a/sysdeps/s390/s390-64/backtrace.c b/sysdeps/s390/s390-64/backtrace.c
index 39a15e0..74b5581 100644
--- a/sysdeps/s390/s390-64/backtrace.c
+++ b/sysdeps/s390/s390-64/backtrace.c
@@ -76,12 +76,8 @@  init (void)
   if (unwind_getip == NULL)
     unwind_backtrace = NULL;
 }
-#else
-# define unwind_backtrace _Unwind_Backtrace
-# define unwind_getip _Unwind_GetIP
-#endif
 
-int
+static int
 __backchain_backtrace (void **array, int size)
 {
   /* We assume that all the code is generated with frame pointers set.  */
@@ -106,6 +102,10 @@  __backchain_backtrace (void **array, int size)
 
   return cnt;
 }
+#else
+# define unwind_backtrace _Unwind_Backtrace
+# define unwind_getip _Unwind_GetIP
+#endif
 
 static _Unwind_Reason_Code
 backtrace_helper (struct _Unwind_Context *ctx, void *a)
@@ -129,9 +129,10 @@  __backtrace (void **array, int size)
   __libc_once_define (static, once);
 
   __libc_once (once, init);
-#endif
+
   if (unwind_backtrace == NULL)
     return __backchain_backtrace (array, size);
+#endif
 
   if (size >= 1)
     unwind_backtrace (backtrace_helper, &arg);