diff mbox

Fix maybe-uninitialized error on powerpc

Message ID 20200109182705.5309-1-msc@linux.ibm.com
State Superseded
Headers show

Commit Message

Matheus Castanho Jan. 9, 2020, 6:27 p.m. UTC
The build has been failing on powerpc64le-linux-gnu with GCC 10
due to a maybe-uninitialized error:

../sysdeps/ieee754/dbl-64/mpa.c:875:6: error: ‘w.e’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
  875 |   EY -= EX;
      |      ^~

This commits adds proper initialization to avoid it.

Tested on powerpc64le.
---
 sysdeps/ieee754/dbl-64/mpa.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Joseph Myers Jan. 9, 2020, 6:48 p.m. UTC | #1
On Thu, 9 Jan 2020, Matheus Castanho wrote:

> The build has been failing on powerpc64le-linux-gnu with GCC 10
> due to a maybe-uninitialized error:
> 
> ../sysdeps/ieee754/dbl-64/mpa.c:875:6: error: ‘w.e’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>   875 |   EY -= EX;
>       |      ^~
> 
> This commits adds proper initialization to avoid it.

glibc practice is not to add such initializations just to avoid warnings, 
if the warnings are false positives (rather, DIAG_* macros with 
appropriate comments, or __builtin_unreachable, are used instead).

I think what's going on here is that __dbl_mp may not set the exponent of 
a returned 0 value, but in fact a returned 0 value is impossible in the 
case where GCC is reporting uninitialized data.  If you agree with that 
analysis, does putting

  if (t == 0)
    __builtin_unreachable ();

(with a comment justifying *why* t cannot be 0), just before the __dbl_mp 
call in __inv, help with the warning?
Matheus Castanho Jan. 13, 2020, 1:47 p.m. UTC | #2
On 1/9/20 3:48 PM, Joseph Myers wrote:
> On Thu, 9 Jan 2020, Matheus Castanho wrote:
> 
>> The build has been failing on powerpc64le-linux-gnu with GCC 10
>> due to a maybe-uninitialized error:
>>
>> ../sysdeps/ieee754/dbl-64/mpa.c:875:6: error: ‘w.e’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>   875 |   EY -= EX;
>>       |      ^~
>>
>> This commits adds proper initialization to avoid it.
> 
> glibc practice is not to add such initializations just to avoid warnings, 
> if the warnings are false positives (rather, DIAG_* macros with 
> appropriate comments, or __builtin_unreachable, are used instead).
> 
> I think what's going on here is that __dbl_mp may not set the exponent of 
> a returned 0 value, but in fact a returned 0 value is impossible in the 
> case where GCC is reporting uninitialized data.  If you agree with that 
> analysis, does putting
> 
>   if (t == 0)
>     __builtin_unreachable ();
> 
> (with a comment justifying *why* t cannot be 0), just before the __dbl_mp 
> call in __inv, help with the warning?

Thanks for the help, that seems the problem indeed, and this check does
solve it.

I'll do this instead (with the justification comment) and send a new
patch later.
diff mbox

Patch

diff --git a/sysdeps/ieee754/dbl-64/mpa.c b/sysdeps/ieee754/dbl-64/mpa.c
index 3bb8bff90d..8ad0fc7535 100644
--- a/sysdeps/ieee754/dbl-64/mpa.c
+++ b/sysdeps/ieee754/dbl-64/mpa.c
@@ -895,6 +895,7 @@  SECTION
 __dvd (const mp_no *x, const mp_no *y, mp_no *z, int p)
 {
   mp_no w;
+  w.e = 0;
 
   if (X[0] == 0)
     Z[0] = 0;