* [PATCH 2/2] soft-fp: ignore maybe-uninitialized
2018-09-19 10:08 [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Martin Jansa
@ 2018-09-19 10:08 ` Martin Jansa
2018-09-19 13:28 ` Joseph Myers
2018-09-19 13:23 ` [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Joseph Myers
1 sibling, 1 reply; 23+ messages in thread
From: Martin Jansa @ 2018-09-19 10:08 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
* with -O it fails with:
In file included from ../soft-fp/soft-fp.h:318,
from ../sysdeps/ieee754/soft-fp/s_fdiv.c:28:
../sysdeps/ieee754/soft-fp/s_fdiv.c: In function '__fdiv':
../soft-fp/op-2.h:98:25: error: 'R_f1' may be used uninitialized in this function [-Werror=maybe-uninitialized]
X##_f0 = (X##_f1 << (_FP_W_TYPE_SIZE - (N)) | X##_f0 >> (N) \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f1' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:36: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
../soft-fp/op-2.h:101:17: error: 'R_f0' may be used uninitialized in this function [-Werror=maybe-uninitialized]
: (X##_f0 << (_FP_W_TYPE_SIZE - (N))) != 0)); \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f0' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:14: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
soft-fp/op-2.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/soft-fp/op-2.h b/soft-fp/op-2.h
index 6020d663d4..6672337949 100644
--- a/soft-fp/op-2.h
+++ b/soft-fp/op-2.h
@@ -92,6 +92,8 @@
X##_f1 = 0; \
}))
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
#define _FP_FRAC_SRS_2(X, N, sz) \
(void) (((N) < _FP_W_TYPE_SIZE) \
? ({ \
@@ -109,6 +111,7 @@
| X##_f0) != 0)); \
X##_f1 = 0; \
}))
+#pragma GCC diagnostic pop
#define _FP_FRAC_ADDI_2(X, I) \
__FP_FRAC_ADDI_2 (X##_f1, X##_f0, I)
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors
@ 2018-09-19 10:08 Martin Jansa
2018-09-19 10:08 ` [PATCH 2/2] soft-fp: ignore maybe-uninitialized Martin Jansa
2018-09-19 13:23 ` [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Joseph Myers
0 siblings, 2 replies; 23+ messages in thread
From: Martin Jansa @ 2018-09-19 10:08 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
* with -O included in CFLAGS it fails to build with:
../sysdeps/ieee754/ldbl-96/e_jnl.c: In function '__ieee754_jnl':
../sysdeps/ieee754/ldbl-96/e_jnl.c:146:20: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrtl (x);
~~~~~~~~~~^~~~~~
../sysdeps/ieee754/ldbl-96/e_jnl.c: In function '__ieee754_ynl':
../sysdeps/ieee754/ldbl-96/e_jnl.c:375:16: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrtl (x);
~~~~~~~~~~^~~~~~
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
sysdeps/ieee754/dbl-64/e_jn.c | 2 ++
sysdeps/ieee754/ldbl-128/e_jnl.c | 4 ++++
sysdeps/ieee754/ldbl-96/e_jnl.c | 4 ++++
3 files changed, 10 insertions(+)
diff --git a/sysdeps/ieee754/dbl-64/e_jn.c b/sysdeps/ieee754/dbl-64/e_jn.c
index cb7c6cf266..e9964311ca 100644
--- a/sysdeps/ieee754/dbl-64/e_jn.c
+++ b/sysdeps/ieee754/dbl-64/e_jn.c
@@ -109,6 +109,7 @@ __ieee754_jn (int n, double x)
case 1: temp = -c + s; break;
case 2: temp = -c - s; break;
case 3: temp = c - s; break;
+ default: temp = 0; // just to prevent error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
}
b = invsqrtpi * temp / sqrt (x);
}
@@ -316,6 +317,7 @@ __ieee754_yn (int n, double x)
case 1: temp = -s - c; break;
case 2: temp = -s + c; break;
case 3: temp = s + c; break;
+ default: temp = 0; // just to prevent error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
}
b = invsqrtpi * temp / sqrt (x);
}
diff --git a/sysdeps/ieee754/ldbl-128/e_jnl.c b/sysdeps/ieee754/ldbl-128/e_jnl.c
index 540b95ca58..2270522e24 100644
--- a/sysdeps/ieee754/ldbl-128/e_jnl.c
+++ b/sysdeps/ieee754/ldbl-128/e_jnl.c
@@ -150,6 +150,8 @@ __ieee754_jnl (int n, _Float128 x)
case 3:
temp = c - s;
break;
+ default:
+ temp = 0; // just to prevent error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
}
b = invsqrtpi * temp / sqrtl (x);
}
@@ -386,6 +388,8 @@ __ieee754_ynl (int n, _Float128 x)
case 3:
temp = s + c;
break;
+ default:
+ temp = 0; // just to prevent error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
}
b = invsqrtpi * temp / sqrtl (x);
}
diff --git a/sysdeps/ieee754/ldbl-96/e_jnl.c b/sysdeps/ieee754/ldbl-96/e_jnl.c
index fd28f6ae91..ee158cf39a 100644
--- a/sysdeps/ieee754/ldbl-96/e_jnl.c
+++ b/sysdeps/ieee754/ldbl-96/e_jnl.c
@@ -143,6 +143,8 @@ __ieee754_jnl (int n, long double x)
case 3:
temp = c - s;
break;
+ default:
+ temp = 0; // just to prevent error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
}
b = invsqrtpi * temp / sqrtl (x);
}
@@ -372,6 +374,8 @@ __ieee754_ynl (int n, long double x)
case 3:
temp = s + c;
break;
+ default:
+ temp = 0; // just to prevent error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
}
b = invsqrtpi * temp / sqrtl (x);
}
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors
2018-09-19 10:08 [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Martin Jansa
2018-09-19 10:08 ` [PATCH 2/2] soft-fp: ignore maybe-uninitialized Martin Jansa
@ 2018-09-19 13:23 ` Joseph Myers
2018-09-29 10:46 ` [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444] Martin Jansa
2018-09-29 19:39 ` [PATCHv3] " Martin Jansa
1 sibling, 2 replies; 23+ messages in thread
From: Joseph Myers @ 2018-09-19 13:23 UTC (permalink / raw)
To: Martin Jansa; +Cc: libc-alpha
On Wed, 19 Sep 2018, Martin Jansa wrote:
> * with -O included in CFLAGS it fails to build with:
This is bug 19444, so [BZ #19444] in ChangeLog entries for it. (If your
patch only fixes some of the issues building with -O, your commit message
needs to make it clear, so that it's obvious whether the bug should be
resolved as FIXED with target milestone set to the first mainline release
with the fix immediately after commit. But the bug number should be given
even for changes only fixing part of a bug - they just need to make clear
whether they are complete or partial fixes.)
> + default: temp = 0; // just to prevent error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
We don't use // comments in glibc code. We also don't add initializers /
assignments that might generate code just to silence warnings; if the code
cannot be written in a way that makes it clear to the compiler what is /
is not uninitialized, without resulting in extra generated code, we use
the DIAG_* macros from libc-diag.h, with appropriate comments in every
case explaining why in fact the warning is a false positive, to silence
the warnings locally.
Unfortunately <https://sourceware.org/glibc/wiki/Style_and_Conventions>
doesn't mention either of these conventions, so there is clearly scope for
us to improve our documentation on contributing to glibc.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] soft-fp: ignore maybe-uninitialized
2018-09-19 10:08 ` [PATCH 2/2] soft-fp: ignore maybe-uninitialized Martin Jansa
@ 2018-09-19 13:28 ` Joseph Myers
2018-09-30 15:53 ` [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444] Martin Jansa
2018-09-30 15:54 ` [PATCH 3/3] locale: prevent maybe-uninitialized errors with -Os " Martin Jansa
0 siblings, 2 replies; 23+ messages in thread
From: Joseph Myers @ 2018-09-19 13:28 UTC (permalink / raw)
To: Martin Jansa; +Cc: libc-alpha
On Wed, 19 Sep 2018, Martin Jansa wrote:
> diff --git a/soft-fp/op-2.h b/soft-fp/op-2.h
> index 6020d663d4..6672337949 100644
> --- a/soft-fp/op-2.h
> +++ b/soft-fp/op-2.h
> @@ -92,6 +92,8 @@
> X##_f1 = 0; \
> }))
>
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
As soft-fp is shared with libgcc, it can't use the DIAG_* macros from
libc-diag.h. *But* the principle of needing detailed comments to justify
any warning suppression remains. Those comments would need to identify
the warning in question, and why it is a false positive, and the most
recent compiler version known to produce that false positive, and the
suppression should probably be conditional on building with -O if it isn't
needed with -O2 / -Os.
Rather than putting the suppression around a macro definition, I wonder
about putting it at the place where the macros are used (s_fdiv.c, it
appears from the warnings you quote). That's what we do for the soft-fp
implementations of fma, for example, and it would allow you to use the
DIAG_* macros. You'd need to add DIAG_IGNORE_O1_NEEDS_COMMENT like
DIAG_IGNORE_Os_NEEDS_COMMENT to libc-diag.h, and then use it with an
appropriate comment in s_fdiv.c (explaining the warning and why it is a
false positive).
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444]
2018-09-19 13:23 ` [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Joseph Myers
@ 2018-09-29 10:46 ` Martin Jansa
2018-09-30 13:25 ` Joseph Myers
2018-09-29 19:39 ` [PATCHv3] " Martin Jansa
1 sibling, 1 reply; 23+ messages in thread
From: Martin Jansa @ 2018-09-29 10:46 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
With -O included in CFLAGS it fails to build with:
../sysdeps/ieee754/ldbl-96/e_jnl.c: In function '__ieee754_jnl':
../sysdeps/ieee754/ldbl-96/e_jnl.c:146:20: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrtl (x);
~~~~~~~~~~^~~~~~
../sysdeps/ieee754/ldbl-96/e_jnl.c: In function '__ieee754_ynl':
../sysdeps/ieee754/ldbl-96/e_jnl.c:375:16: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrtl (x);
~~~~~~~~~~^~~~~~
[BZ #23716]
* sysdeps/ieee754/ldbl-96/e_jnl.c: Fix build with -O
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
ChangeLog | 4 ++++
sysdeps/ieee754/ldbl-96/e_jnl.c | 15 +++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 07760299e6..5066a4840d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-29 Martin Jansa <Martin.Jansa@gmail.com>
+ Partial fix for [BZ #23716]
+ * sysdeps/ieee754/ldbl-96/e_jnl.c: Fix build with -O
+
2018-09-28 Joseph Myers <joseph@codesourcery.com>
* math/fromfp.h: Do not include <math_private.h>.
diff --git a/sysdeps/ieee754/ldbl-96/e_jnl.c b/sysdeps/ieee754/ldbl-96/e_jnl.c
index 855190841b..c5b27e7fcf 100644
--- a/sysdeps/ieee754/ldbl-96/e_jnl.c
+++ b/sysdeps/ieee754/ldbl-96/e_jnl.c
@@ -62,6 +62,7 @@
#include <math_private.h>
#include <fenv_private.h>
#include <math-underflow.h>
+#include <libc-diag.h>
static const long double
invsqrtpi = 5.64189583547756286948079e-1L, two = 2.0e0L, one = 1.0e0L;
@@ -144,7 +145,14 @@ __ieee754_jnl (int n, long double x)
temp = c - s;
break;
}
+/* With GCC 8 when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above should cover all possible values of n & 3
+ so I belive it's false possitive. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
@@ -373,7 +381,14 @@ __ieee754_ynl (int n, long double x)
temp = s + c;
break;
}
+/* With GCC 8 when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above should cover all possible values of n & 3
+ so I belive it's false possitive. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv3] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444]
2018-09-19 13:23 ` [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Joseph Myers
2018-09-29 10:46 ` [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444] Martin Jansa
@ 2018-09-29 19:39 ` Martin Jansa
1 sibling, 0 replies; 23+ messages in thread
From: Martin Jansa @ 2018-09-29 19:39 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
With -O included in CFLAGS it fails to build with:
../sysdeps/ieee754/ldbl-96/e_jnl.c: In function '__ieee754_jnl':
../sysdeps/ieee754/ldbl-96/e_jnl.c:146:20: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrtl (x);
~~~~~~~~~~^~~~~~
../sysdeps/ieee754/ldbl-96/e_jnl.c: In function '__ieee754_ynl':
../sysdeps/ieee754/ldbl-96/e_jnl.c:375:16: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrtl (x);
~~~~~~~~~~^~~~~~
../sysdeps/ieee754/dbl-64/e_jn.c: In function '__ieee754_jn':
../sysdeps/ieee754/dbl-64/e_jn.c:113:20: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrt (x);
~~~~~~~~~~^~~~~~
../sysdeps/ieee754/dbl-64/e_jn.c: In function '__ieee754_yn':
../sysdeps/ieee754/dbl-64/e_jn.c:320:16: error: 'temp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
b = invsqrtpi * temp / sqrt (x);
~~~~~~~~~~^~~~~~
Build tested with Yocto for ARM, AARCH64, X86, X86_64, PPC with
-O, -O1, -Os.
For soft-fp ARM it needs one more fix for -O1:
https://sourceware.org/ml/libc-alpha/2018-09/msg00300.html
For AARCH64 it needs one more fix for -Os.
[BZ #23716]
* sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
* sysdeps/ieee754/ldbl-96/e_jnl.c: Likewise.
* sysdeps/ieee754/ldbl-128/e_jnl.c: Likewise.
* sysdeps/ieee754/ldbl-128ibm/e_jnl.c: Likewise.
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
ChangeLog | 7 +++++++
sysdeps/ieee754/dbl-64/e_jn.c | 21 +++++++++++++++++++++
sysdeps/ieee754/ldbl-128/e_jnl.c | 21 +++++++++++++++++++++
sysdeps/ieee754/ldbl-128ibm/e_jnl.c | 21 +++++++++++++++++++++
sysdeps/ieee754/ldbl-96/e_jnl.c | 21 +++++++++++++++++++++
5 files changed, 91 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 07760299e6..4bafeefda5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2018-09-29 Martin Jansa <Martin.Jansa@gmail.com>
+ Partial fix for [BZ #23716]
+ * sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
+ * sysdeps/ieee754/ldbl-96/e_jnl.c: Likewise.
+ * sysdeps/ieee754/ldbl-128/e_jnl.c: Likewise.
+ * sysdeps/ieee754/ldbl-128ibm/e_jnl.c: Likewise.
+
2018-09-28 Joseph Myers <joseph@codesourcery.com>
* math/fromfp.h: Do not include <math_private.h>.
diff --git a/sysdeps/ieee754/dbl-64/e_jn.c b/sysdeps/ieee754/dbl-64/e_jn.c
index aff06ead16..20c8ff4f8f 100644
--- a/sysdeps/ieee754/dbl-64/e_jn.c
+++ b/sysdeps/ieee754/dbl-64/e_jn.c
@@ -43,6 +43,7 @@
#include <math_private.h>
#include <fenv_private.h>
#include <math-underflow.h>
+#include <libc-diag.h>
static const double
invsqrtpi = 5.64189583547756279280e-01, /* 0x3FE20DD7, 0x50429B6D */
@@ -110,7 +111,17 @@ __ieee754_jn (int n, double x)
case 2: temp = -c - s; break;
case 3: temp = c - s; break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrt (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
@@ -317,7 +328,17 @@ __ieee754_yn (int n, double x)
case 2: temp = -s + c; break;
case 3: temp = s + c; break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrt (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
diff --git a/sysdeps/ieee754/ldbl-128/e_jnl.c b/sysdeps/ieee754/ldbl-128/e_jnl.c
index 7610d18c67..fa49f72844 100644
--- a/sysdeps/ieee754/ldbl-128/e_jnl.c
+++ b/sysdeps/ieee754/ldbl-128/e_jnl.c
@@ -62,6 +62,7 @@
#include <math_private.h>
#include <fenv_private.h>
#include <math-underflow.h>
+#include <libc-diag.h>
static const _Float128
invsqrtpi = L(5.6418958354775628694807945156077258584405E-1),
@@ -151,7 +152,17 @@ __ieee754_jnl (int n, _Float128 x)
temp = c - s;
break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
@@ -387,7 +398,17 @@ __ieee754_ynl (int n, _Float128 x)
temp = s + c;
break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
diff --git a/sysdeps/ieee754/ldbl-128ibm/e_jnl.c b/sysdeps/ieee754/ldbl-128ibm/e_jnl.c
index 50b4558e74..1f8fd36cb8 100644
--- a/sysdeps/ieee754/ldbl-128ibm/e_jnl.c
+++ b/sysdeps/ieee754/ldbl-128ibm/e_jnl.c
@@ -62,6 +62,7 @@
#include <math_private.h>
#include <fenv_private.h>
#include <math-underflow.h>
+#include <libc-diag.h>
static const long double
invsqrtpi = 5.6418958354775628694807945156077258584405E-1L,
@@ -151,7 +152,17 @@ __ieee754_jnl (int n, long double x)
temp = c - s;
break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
@@ -387,7 +398,17 @@ __ieee754_ynl (int n, long double x)
temp = s + c;
break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
diff --git a/sysdeps/ieee754/ldbl-96/e_jnl.c b/sysdeps/ieee754/ldbl-96/e_jnl.c
index 855190841b..60964fa887 100644
--- a/sysdeps/ieee754/ldbl-96/e_jnl.c
+++ b/sysdeps/ieee754/ldbl-96/e_jnl.c
@@ -62,6 +62,7 @@
#include <math_private.h>
#include <fenv_private.h>
#include <math-underflow.h>
+#include <libc-diag.h>
static const long double
invsqrtpi = 5.64189583547756286948079e-1L, two = 2.0e0L, one = 1.0e0L;
@@ -144,7 +145,17 @@ __ieee754_jnl (int n, long double x)
temp = c - s;
break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
@@ -373,7 +384,17 @@ __ieee754_ynl (int n, long double x)
temp = s + c;
break;
}
+/* With GCC 8 (and older) when compiling with -O the compiler
+ warns that the variable 'temp', may be used uninitialized.
+ The switch above covers all possible values of n & 3
+ but GCC without VRP enabled isn't able to figure out the
+ range of possible values is [0,3] as explained in:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69230
+ so it's false possitive with -O1 and lower. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
b = invsqrtpi * temp / sqrtl (x);
+DIAG_POP_NEEDS_COMMENT;
}
else
{
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444]
2018-09-29 10:46 ` [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444] Martin Jansa
@ 2018-09-30 13:25 ` Joseph Myers
2018-09-30 13:50 ` Martin Jansa
0 siblings, 1 reply; 23+ messages in thread
From: Joseph Myers @ 2018-09-30 13:25 UTC (permalink / raw)
To: Martin Jansa; +Cc: libc-alpha
On Sat, 29 Sep 2018, Martin Jansa wrote:
> +/* With GCC 8 when compiling with -O the compiler
> + warns that the variable 'temp', may be used uninitialized.
> + The switch above should cover all possible values of n & 3
> + so I belive it's false possitive. */
> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
> b = invsqrtpi * temp / sqrtl (x);
> +DIAG_POP_NEEDS_COMMENT;
DIAG_* and associated comments should be indented to match the associated
code rather than at the left margin. Any alphabetic characters at the
left margin in the middle of a function are a bad idea because they break
things trying to determine the function name for diff context etc.
As per <https://sourceware.org/ml/libc-alpha/2018-09/msg00311.html> I
think it would be best to add DIAG_IGNORE_O1_NEEDS_COMMENT to libc-diag.h,
and use it for cases such as here that only need the diagnostics disabled
for -O1.
glibc code is written and maintained by many people over time, so there
shouldn't be an "I" in comments; they should reflect a current collective
understanding rather than one person's view, and read as such. Also, the
point isn't that the switch *should* cover all possible value of n & 3,
it's that it *does* cover all possible values (and sets temp in every
case) and so the warning *is* a false positive.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444]
2018-09-30 13:25 ` Joseph Myers
@ 2018-09-30 13:50 ` Martin Jansa
2018-09-30 16:49 ` Joseph Myers
0 siblings, 1 reply; 23+ messages in thread
From: Martin Jansa @ 2018-09-30 13:50 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]
On Sun, Sep 30, 2018 at 01:25:08PM +0000, Joseph Myers wrote:
> On Sat, 29 Sep 2018, Martin Jansa wrote:
>
> > +/* With GCC 8 when compiling with -O the compiler
> > + warns that the variable 'temp', may be used uninitialized.
> > + The switch above should cover all possible values of n & 3
> > + so I belive it's false possitive. */
> > +DIAG_PUSH_NEEDS_COMMENT;
> > +DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
> > b = invsqrtpi * temp / sqrtl (x);
> > +DIAG_POP_NEEDS_COMMENT;
>
> DIAG_* and associated comments should be indented to match the associated
> code rather than at the left margin. Any alphabetic characters at the
> left margin in the middle of a function are a bad idea because they break
> things trying to determine the function name for diff context etc.
OK, will update in v3.
> As per <https://sourceware.org/ml/libc-alpha/2018-09/msg00311.html> I
> think it would be best to add DIAG_IGNORE_O1_NEEDS_COMMENT to libc-diag.h,
> and use it for cases such as here that only need the diagnostics disabled
> for -O1.
How should I distinguish between -O1 and -O2? For -Os it's simple,
because there is __OPTIMIZE_SIZE__ defined only when -Os, but for -O1 as
well as -O2 and -O3 there is __OPTIMIZE__, but I don't know how to find
out which level is being used. And it's still not completely accurate
-O1 + -ftree-vrp should be working fine and -O2 + -fno-tree-vrp will
probably trigger the maybe-uninitialized warning. And I don't see any
macro defined only when VRP is enabled in gcc.
> glibc code is written and maintained by many people over time, so there
> shouldn't be an "I" in comments; they should reflect a current collective
> understanding rather than one person's view, and read as such. Also, the
> point isn't that the switch *should* cover all possible value of n & 3,
> it's that it *does* cover all possible values (and sets temp in every
> case) and so the warning *is* a false positive.
OK, I've already improved it a bit in local v3 (still waiting for build tests
across 7 architectures x 3 optimization levels), I've also one more fix
for aarch64 when -Os is being used.
Thanks
--
Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-09-19 13:28 ` Joseph Myers
@ 2018-09-30 15:53 ` Martin Jansa
2018-09-30 16:53 ` Joseph Myers
2018-09-30 15:54 ` [PATCH 3/3] locale: prevent maybe-uninitialized errors with -Os " Martin Jansa
1 sibling, 1 reply; 23+ messages in thread
From: Martin Jansa @ 2018-09-30 15:53 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
* with -O, -O1, -Os it fails with:
In file included from ../soft-fp/soft-fp.h:318,
from ../sysdeps/ieee754/soft-fp/s_fdiv.c:28:
../sysdeps/ieee754/soft-fp/s_fdiv.c: In function '__fdiv':
../soft-fp/op-2.h:98:25: error: 'R_f1' may be used uninitialized in this function [-Werror=maybe-uninitialized]
X##_f0 = (X##_f1 << (_FP_W_TYPE_SIZE - (N)) | X##_f0 >> (N) \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f1' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:36: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
../soft-fp/op-2.h:101:17: error: 'R_f0' may be used uninitialized in this function [-Werror=maybe-uninitialized]
: (X##_f0 << (_FP_W_TYPE_SIZE - (N))) != 0)); \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f0' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:14: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
Build tested with Yocto for ARM, AARCH64, X86, X86_64, PPC, MIPS, MIPS64
with -O, -O1, -Os.
For AARCH64 it needs one more fix in locale for -Os.
Partial fix for [BZ #23716]
* sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
ChangeLog | 4 ++++
sysdeps/ieee754/soft-fp/s_fdiv.c | 12 ++++++++++++
2 files changed, 16 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 4bafeefda5..9cdf59c9ab 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
+ Partial fix for [BZ #23716]
+ * sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O.
+
2018-09-29 Martin Jansa <Martin.Jansa@gmail.com>
Partial fix for [BZ #23716]
* sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
diff --git a/sysdeps/ieee754/soft-fp/s_fdiv.c b/sysdeps/ieee754/soft-fp/s_fdiv.c
index 341339f5ed..14655b77da 100644
--- a/sysdeps/ieee754/soft-fp/s_fdiv.c
+++ b/sysdeps/ieee754/soft-fp/s_fdiv.c
@@ -25,6 +25,16 @@
#undef fdivl
#include <math-narrow.h>
+
+#include <libc-diag.h>
+/* R_f[01] are not set in cases where it is not used in packing, but the
+ compiler does not see that it is set in all cases where it is
+ used, resulting in warnings that it may be used uninitialized.
+ The location of the warning differs in different versions of GCC,
+ it may be where R is defined using a macro or it may be where the
+ macro is defined. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
#include <soft-fp.h>
#include <single.h>
#include <double.h>
@@ -53,4 +63,6 @@ __fdiv (double x, double y)
CHECK_NARROW_DIV (ret, x, y);
return ret;
}
+DIAG_POP_NEEDS_COMMENT;
+
libm_alias_float_double (div)
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/3] locale: prevent maybe-uninitialized errors with -Os [BZ #19444]
2018-09-19 13:28 ` Joseph Myers
2018-09-30 15:53 ` [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444] Martin Jansa
@ 2018-09-30 15:54 ` Martin Jansa
2018-12-17 18:22 ` Martin Jansa
2018-12-17 21:40 ` [PATCHv2 " Martin Jansa
1 sibling, 2 replies; 23+ messages in thread
From: Martin Jansa @ 2018-09-30 15:54 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
Fixes following error when building for aarch64 with -Os:
| In file included from strcoll_l.c:43:
| strcoll_l.c: In function '__strcoll_l':
| ../locale/weight.h:31:26: error: 'seq2.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
| int_fast32_t i = table[*(*cpp)++];
| ^~~~~~~~~
| strcoll_l.c:304:18: note: 'seq2.back_us' was declared here
| coll_seq seq1, seq2;
| ^~~~
| In file included from strcoll_l.c:43:
| ../locale/weight.h:31:26: error: 'seq1.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
| int_fast32_t i = table[*(*cpp)++];
| ^~~~~~~~~
| strcoll_l.c:304:12: note: 'seq1.back_us' was declared here
| coll_seq seq1, seq2;
| ^~~~
Partial fix for [BZ #23716]
* locale/weight.h: Fix build with -Os.
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
ChangeLog | 4 ++++
locale/weight.h | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 9cdf59c9ab..ef506309aa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
+ Partial fix for [BZ #23716]
+ * locale/weight.h: Fix build with -Os.
+
2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
Partial fix for [BZ #23716]
* sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O.
diff --git a/locale/weight.h b/locale/weight.h
index 6028d3595e..10bcea25e5 100644
--- a/locale/weight.h
+++ b/locale/weight.h
@@ -28,7 +28,14 @@ findidx (const int32_t *table,
const unsigned char *extra,
const unsigned char **cpp, size_t len)
{
+ /* With GCC 8 when compiling with -Os the compiler warns that
+ seq1.back_us and seq2.back_us might be used uninitialized.
+ This uninitialized use is impossible for the same reason
+ as described in comments in locale/weightwc.h. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
int_fast32_t i = table[*(*cpp)++];
+ DIAG_POP_NEEDS_COMMENT;
const unsigned char *cp;
const unsigned char *usrc;
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444]
2018-09-30 13:50 ` Martin Jansa
@ 2018-09-30 16:49 ` Joseph Myers
0 siblings, 0 replies; 23+ messages in thread
From: Joseph Myers @ 2018-09-30 16:49 UTC (permalink / raw)
To: Martin Jansa; +Cc: libc-alpha
On Sun, 30 Sep 2018, Martin Jansa wrote:
> > As per <https://sourceware.org/ml/libc-alpha/2018-09/msg00311.html> I
> > think it would be best to add DIAG_IGNORE_O1_NEEDS_COMMENT to libc-diag.h,
> > and use it for cases such as here that only need the diagnostics disabled
> > for -O1.
>
> How should I distinguish between -O1 and -O2? For -Os it's simple,
> because there is __OPTIMIZE_SIZE__ defined only when -Os, but for -O1 as
> well as -O2 and -O3 there is __OPTIMIZE__, but I don't know how to find
> out which level is being used. And it's still not completely accurate
> -O1 + -ftree-vrp should be working fine and -O2 + -fno-tree-vrp will
> probably trigger the maybe-uninitialized warning. And I don't see any
> macro defined only when VRP is enabled in gcc.
It looks like that would require a configure test, so maybe a separate
macro there is overkill. Does anyone else have thoughts on this question
of how narrowly to disable warnings for cases that only warn for -O1?
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-09-30 15:53 ` [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444] Martin Jansa
@ 2018-09-30 16:53 ` Joseph Myers
2018-09-30 17:49 ` Martin Jansa
0 siblings, 1 reply; 23+ messages in thread
From: Joseph Myers @ 2018-09-30 16:53 UTC (permalink / raw)
To: Martin Jansa; +Cc: libc-alpha
On Sun, 30 Sep 2018, Martin Jansa wrote:
> +#include <libc-diag.h>
> +/* R_f[01] are not set in cases where it is not used in packing, but the
> + compiler does not see that it is set in all cases where it is
> + used, resulting in warnings that it may be used uninitialized.
> + The location of the warning differs in different versions of GCC,
> + it may be where R is defined using a macro or it may be where the
> + macro is defined. */
This comment is missing the key information that it's specifically a
warning seen with -O1.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-09-30 16:53 ` Joseph Myers
@ 2018-09-30 17:49 ` Martin Jansa
2018-10-01 15:03 ` Joseph Myers
0 siblings, 1 reply; 23+ messages in thread
From: Martin Jansa @ 2018-09-30 17:49 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
On Sun, Sep 30, 2018 at 04:53:36PM +0000, Joseph Myers wrote:
> On Sun, 30 Sep 2018, Martin Jansa wrote:
>
> > +#include <libc-diag.h>
> > +/* R_f[01] are not set in cases where it is not used in packing, but the
> > + compiler does not see that it is set in all cases where it is
> > + used, resulting in warnings that it may be used uninitialized.
> > + The location of the warning differs in different versions of GCC,
> > + it may be where R is defined using a macro or it may be where the
> > + macro is defined. */
>
> This comment is missing the key information that it's specifically a
> warning seen with -O1.
The other 3 cases of the same:
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/soft-fp/s_fma.c;h=41cf81a74bfc6a8c21c565fb04c90ced4bfd19e8;hb=refs/heads/master#l32
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/soft-fp/s_fmaf.c;h=afe64356b3a9c5f3467c35b80f33ba946fbeee46;hb=refs/heads/master#l32
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/soft-fp/s_fmal.c;h=4c2d4c5d9be09f0c8464fb20b81af3aff131e65d;hb=refs/heads/master#l32
are probably also triggered only with -O1, do you want me to update all
4 or just the new case?
--
Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-09-30 17:49 ` Martin Jansa
@ 2018-10-01 15:03 ` Joseph Myers
2018-10-01 16:26 ` [PATCHv3] " Martin Jansa
2018-10-01 16:46 ` [PATCHv4] " Martin Jansa
0 siblings, 2 replies; 23+ messages in thread
From: Joseph Myers @ 2018-10-01 15:03 UTC (permalink / raw)
To: Martin Jansa; +Cc: libc-alpha
On Sun, 30 Sep 2018, Martin Jansa wrote:
> On Sun, Sep 30, 2018 at 04:53:36PM +0000, Joseph Myers wrote:
> > On Sun, 30 Sep 2018, Martin Jansa wrote:
> >
> > > +#include <libc-diag.h>
> > > +/* R_f[01] are not set in cases where it is not used in packing, but the
> > > + compiler does not see that it is set in all cases where it is
> > > + used, resulting in warnings that it may be used uninitialized.
> > > + The location of the warning differs in different versions of GCC,
> > > + it may be where R is defined using a macro or it may be where the
> > > + macro is defined. */
> >
> > This comment is missing the key information that it's specifically a
> > warning seen with -O1.
>
> The other 3 cases of the same:
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/soft-fp/s_fma.c;h=41cf81a74bfc6a8c21c565fb04c90ced4bfd19e8;hb=refs/heads/master#l32
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/soft-fp/s_fmaf.c;h=afe64356b3a9c5f3467c35b80f33ba946fbeee46;hb=refs/heads/master#l32
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/soft-fp/s_fmal.c;h=4c2d4c5d9be09f0c8464fb20b81af3aff131e65d;hb=refs/heads/master#l32
>
> are probably also triggered only with -O1, do you want me to update all
> 4 or just the new case?
They're definitely not limited to -O1 (or weren't when added in 2015,
commit dc6b5aed1b406a53c4512d355376b4e12c7da971). So it's only the new
case for which a comment about being limited to -O1 is appropriate.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv3] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-10-01 15:03 ` Joseph Myers
@ 2018-10-01 16:26 ` Martin Jansa
2018-10-01 16:46 ` [PATCHv4] " Martin Jansa
1 sibling, 0 replies; 23+ messages in thread
From: Martin Jansa @ 2018-10-01 16:26 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
* with -O, -O1, -Os it fails with:
In file included from ../soft-fp/soft-fp.h:318,
from ../sysdeps/ieee754/soft-fp/s_fdiv.c:28:
../sysdeps/ieee754/soft-fp/s_fdiv.c: In function '__fdiv':
../soft-fp/op-2.h:98:25: error: 'R_f1' may be used uninitialized in this function [-Werror=maybe-uninitialized]
X##_f0 = (X##_f1 << (_FP_W_TYPE_SIZE - (N)) | X##_f0 >> (N) \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f1' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:36: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
../soft-fp/op-2.h:101:17: error: 'R_f0' may be used uninitialized in this function [-Werror=maybe-uninitialized]
: (X##_f0 << (_FP_W_TYPE_SIZE - (N))) != 0)); \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f0' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:14: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
Build tested with Yocto for ARM, AARCH64, X86, X86_64, PPC, MIPS, MIPS64
with -O, -O1, -Os.
For AARCH64 it needs one more fix in locale for -Os.
Partial fix for [BZ #23716]
* sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
ChangeLog | 4 ++++
sysdeps/ieee754/soft-fp/s_fdiv.c | 12 ++++++++++++
2 files changed, 16 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 4bafeefda5..9cdf59c9ab 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
+ Partial fix for [BZ #23716]
+ * sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O.
+
2018-09-29 Martin Jansa <Martin.Jansa@gmail.com>
Partial fix for [BZ #23716]
* sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
diff --git a/sysdeps/ieee754/soft-fp/s_fdiv.c b/sysdeps/ieee754/soft-fp/s_fdiv.c
index 341339f5ed..a8ed916278 100644
--- a/sysdeps/ieee754/soft-fp/s_fdiv.c
+++ b/sysdeps/ieee754/soft-fp/s_fdiv.c
@@ -25,6 +25,16 @@
#undef fdivl
#include <math-narrow.h>
+
+#include <libc-diag.h>
+/* R_f[01] are not set in cases where it is not used in packing, but the
+ compiler does not see that it is set in all cases where it is
+ used, resulting in warnings that it may be used uninitialized.
+ The location of the warning differs in different versions of GCC,
+ it may be where R is defined using a macro or it may be where the
+ macro is defined. This happends only with -O1. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
#include <soft-fp.h>
#include <single.h>
#include <double.h>
@@ -53,4 +63,6 @@ __fdiv (double x, double y)
CHECK_NARROW_DIV (ret, x, y);
return ret;
}
+DIAG_POP_NEEDS_COMMENT;
+
libm_alias_float_double (div)
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv4] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-10-01 15:03 ` Joseph Myers
2018-10-01 16:26 ` [PATCHv3] " Martin Jansa
@ 2018-10-01 16:46 ` Martin Jansa
2018-10-02 15:42 ` Joseph Myers
2019-01-03 14:44 ` Mark Corbin
1 sibling, 2 replies; 23+ messages in thread
From: Martin Jansa @ 2018-10-01 16:46 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
* with -O, -O1, -Os it fails with:
In file included from ../soft-fp/soft-fp.h:318,
from ../sysdeps/ieee754/soft-fp/s_fdiv.c:28:
../sysdeps/ieee754/soft-fp/s_fdiv.c: In function '__fdiv':
../soft-fp/op-2.h:98:25: error: 'R_f1' may be used uninitialized in this function [-Werror=maybe-uninitialized]
X##_f0 = (X##_f1 << (_FP_W_TYPE_SIZE - (N)) | X##_f0 >> (N) \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f1' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:36: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
../soft-fp/op-2.h:101:17: error: 'R_f0' may be used uninitialized in this function [-Werror=maybe-uninitialized]
: (X##_f0 << (_FP_W_TYPE_SIZE - (N))) != 0)); \
^~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:14: note: 'R_f0' was declared here
FP_DECL_D (R);
^
../soft-fp/op-2.h:37:14: note: in definition of macro '_FP_FRAC_DECL_2'
_FP_W_TYPE X##_f0 _FP_ZERO_INIT, X##_f1 _FP_ZERO_INIT
^
../soft-fp/double.h:95:24: note: in expansion of macro '_FP_DECL'
# define FP_DECL_D(X) _FP_DECL (2, X)
^~~~~~~~
../sysdeps/ieee754/soft-fp/s_fdiv.c:38:3: note: in expansion of macro 'FP_DECL_D'
FP_DECL_D (R);
^~~~~~~~~
Build tested with Yocto for ARM, AARCH64, X86, X86_64, PPC, MIPS, MIPS64
with -O, -O1, -Os.
For AARCH64 it needs one more fix in locale for -Os.
Partial fix for [BZ #23716]
* sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
ChangeLog | 4 ++++
sysdeps/ieee754/soft-fp/s_fdiv.c | 12 ++++++++++++
2 files changed, 16 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 4bafeefda5..9cdf59c9ab 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
+ Partial fix for [BZ #23716]
+ * sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O.
+
2018-09-29 Martin Jansa <Martin.Jansa@gmail.com>
Partial fix for [BZ #23716]
* sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
diff --git a/sysdeps/ieee754/soft-fp/s_fdiv.c b/sysdeps/ieee754/soft-fp/s_fdiv.c
index 341339f5ed..f124edd760 100644
--- a/sysdeps/ieee754/soft-fp/s_fdiv.c
+++ b/sysdeps/ieee754/soft-fp/s_fdiv.c
@@ -25,6 +25,16 @@
#undef fdivl
#include <math-narrow.h>
+
+#include <libc-diag.h>
+/* R_f[01] are not set in cases where it is not used in packing, but the
+ compiler does not see that it is set in all cases where it is
+ used, resulting in warnings that it may be used uninitialized.
+ The location of the warning differs in different versions of GCC,
+ it may be where R is defined using a macro or it may be where the
+ macro is defined. This happens only with -O1. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
#include <soft-fp.h>
#include <single.h>
#include <double.h>
@@ -53,4 +63,6 @@ __fdiv (double x, double y)
CHECK_NARROW_DIV (ret, x, y);
return ret;
}
+DIAG_POP_NEEDS_COMMENT;
+
libm_alias_float_double (div)
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv4] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-10-01 16:46 ` [PATCHv4] " Martin Jansa
@ 2018-10-02 15:42 ` Joseph Myers
2019-01-03 14:44 ` Mark Corbin
1 sibling, 0 replies; 23+ messages in thread
From: Joseph Myers @ 2018-10-02 15:42 UTC (permalink / raw)
To: Martin Jansa; +Cc: libc-alpha
Thanks, committed.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/3] locale: prevent maybe-uninitialized errors with -Os [BZ #19444]
2018-09-30 15:54 ` [PATCH 3/3] locale: prevent maybe-uninitialized errors with -Os " Martin Jansa
@ 2018-12-17 18:22 ` Martin Jansa
2018-12-17 21:40 ` [PATCHv2 " Martin Jansa
1 sibling, 0 replies; 23+ messages in thread
From: Martin Jansa @ 2018-12-17 18:22 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2790 bytes --]
On Sun, Sep 30, 2018 at 03:53:55PM +0000, Martin Jansa wrote:
> Fixes following error when building for aarch64 with -Os:
> | In file included from strcoll_l.c:43:
> | strcoll_l.c: In function '__strcoll_l':
> | ../locale/weight.h:31:26: error: 'seq2.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | int_fast32_t i = table[*(*cpp)++];
> | ^~~~~~~~~
> | strcoll_l.c:304:18: note: 'seq2.back_us' was declared here
> | coll_seq seq1, seq2;
> | ^~~~
> | In file included from strcoll_l.c:43:
> | ../locale/weight.h:31:26: error: 'seq1.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | int_fast32_t i = table[*(*cpp)++];
> | ^~~~~~~~~
> | strcoll_l.c:304:12: note: 'seq1.back_us' was declared here
> | coll_seq seq1, seq2;
> | ^~~~
>
> Partial fix for [BZ #23716]
> * locale/weight.h: Fix build with -Os.
>
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
I'm sorry for delay, I've just sent v6 for the first patch in this
series:
[PATCH 1/3] sysdeps/ieee754: prevent maybe-uninitialized errors
the 2nd one:
[PATCH 2/3] soft-fp: ignore maybe-uninitialized
was already merged, but I haven't noticed any feedback on this 3rd one
and it's still applicable in latest master.
Regards,
> ---
> ChangeLog | 4 ++++
> locale/weight.h | 7 +++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/ChangeLog b/ChangeLog
> index 9cdf59c9ab..ef506309aa 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
> + Partial fix for [BZ #23716]
> + * locale/weight.h: Fix build with -Os.
> +
> 2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
> Partial fix for [BZ #23716]
> * sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O.
> diff --git a/locale/weight.h b/locale/weight.h
> index 6028d3595e..10bcea25e5 100644
> --- a/locale/weight.h
> +++ b/locale/weight.h
> @@ -28,7 +28,14 @@ findidx (const int32_t *table,
> const unsigned char *extra,
> const unsigned char **cpp, size_t len)
> {
> + /* With GCC 8 when compiling with -Os the compiler warns that
> + seq1.back_us and seq2.back_us might be used uninitialized.
> + This uninitialized use is impossible for the same reason
> + as described in comments in locale/weightwc.h. */
> + DIAG_PUSH_NEEDS_COMMENT;
> + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
> int_fast32_t i = table[*(*cpp)++];
> + DIAG_POP_NEEDS_COMMENT;
> const unsigned char *cp;
> const unsigned char *usrc;
>
> --
> 2.17.1
>
--
Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv2 3/3] locale: prevent maybe-uninitialized errors with -Os [BZ #19444]
2018-09-30 15:54 ` [PATCH 3/3] locale: prevent maybe-uninitialized errors with -Os " Martin Jansa
2018-12-17 18:22 ` Martin Jansa
@ 2018-12-17 21:40 ` Martin Jansa
2019-01-04 23:08 ` Martin Jansa
2019-02-05 10:00 ` Martin Jansa
1 sibling, 2 replies; 23+ messages in thread
From: Martin Jansa @ 2018-12-17 21:40 UTC (permalink / raw)
To: libc-alpha; +Cc: Martin Jansa
Fixes following error when building for aarch64 with -Os:
| In file included from strcoll_l.c:43:
| strcoll_l.c: In function '__strcoll_l':
| ../locale/weight.h:31:26: error: 'seq2.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
| int_fast32_t i = table[*(*cpp)++];
| ^~~~~~~~~
| strcoll_l.c:304:18: note: 'seq2.back_us' was declared here
| coll_seq seq1, seq2;
| ^~~~
| In file included from strcoll_l.c:43:
| ../locale/weight.h:31:26: error: 'seq1.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
| int_fast32_t i = table[*(*cpp)++];
| ^~~~~~~~~
| strcoll_l.c:304:12: note: 'seq1.back_us' was declared here
| coll_seq seq1, seq2;
| ^~~~
Partial fix for [BZ #19444]
* locale/weight.h: Fix build with -Os.
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
ChangeLog | 4 ++++
locale/weight.h | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 2e12129846..2d5679c112 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2018-12-17 Martin Jansa <Martin.Jansa@gmail.com>
+ Partial fix for [BZ #19444]
+ * locale/weight.h: Fix build with -Os.
+
+2018-12-17 Martin Jansa <Martin.Jansa@gmail.com>
Partial fix for [BZ #19444]
* sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
* sysdeps/ieee754/ldbl-96/e_jnl.c: Likewise.
diff --git a/locale/weight.h b/locale/weight.h
index 6028d3595e..10bcea25e5 100644
--- a/locale/weight.h
+++ b/locale/weight.h
@@ -28,7 +28,14 @@ findidx (const int32_t *table,
const unsigned char *extra,
const unsigned char **cpp, size_t len)
{
+ /* With GCC 8 when compiling with -Os the compiler warns that
+ seq1.back_us and seq2.back_us might be used uninitialized.
+ This uninitialized use is impossible for the same reason
+ as described in comments in locale/weightwc.h. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
int_fast32_t i = table[*(*cpp)++];
+ DIAG_POP_NEEDS_COMMENT;
const unsigned char *cp;
const unsigned char *usrc;
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv4] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2018-10-01 16:46 ` [PATCHv4] " Martin Jansa
2018-10-02 15:42 ` Joseph Myers
@ 2019-01-03 14:44 ` Mark Corbin
2019-01-03 17:24 ` Adhemerval Zanella
1 sibling, 1 reply; 23+ messages in thread
From: Mark Corbin @ 2019-01-03 14:44 UTC (permalink / raw)
To: Martin Jansa, libc-alpha
On 01/10/2018 17:26, Martin Jansa wrote:
> ---
> ChangeLog | 4 ++++
> sysdeps/ieee754/soft-fp/s_fdiv.c | 12 ++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/ChangeLog b/ChangeLog
> index 4bafeefda5..9cdf59c9ab 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
> + Partial fix for [BZ #23716]
> + * sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O.
> +
> 2018-09-29 Martin Jansa <Martin.Jansa@gmail.com>
> Partial fix for [BZ #23716]
> * sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
> diff --git a/sysdeps/ieee754/soft-fp/s_fdiv.c b/sysdeps/ieee754/soft-fp/s_fdiv.c
> index 341339f5ed..f124edd760 100644
> --- a/sysdeps/ieee754/soft-fp/s_fdiv.c
> +++ b/sysdeps/ieee754/soft-fp/s_fdiv.c
> @@ -25,6 +25,16 @@
> #undef fdivl
>
> #include <math-narrow.h>
> +
> +#include <libc-diag.h>
> +/* R_f[01] are not set in cases where it is not used in packing, but the
> + compiler does not see that it is set in all cases where it is
> + used, resulting in warnings that it may be used uninitialized.
> + The location of the warning differs in different versions of GCC,
> + it may be where R is defined using a macro or it may be where the
> + macro is defined. This happens only with -O1. */
> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
> #include <soft-fp.h>
> #include <single.h>
> #include <double.h>
> @@ -53,4 +63,6 @@ __fdiv (double x, double y)
> CHECK_NARROW_DIV (ret, x, y);
> return ret;
> }
> +DIAG_POP_NEEDS_COMMENT;
> +
> libm_alias_float_double (div)
Hello
It would be really helpful if this fix (commit id
4a06ceea33ecc220bbfe264d8f1e74de2f04e90d) could be backported from
master to the release/2.28/master branch.
This would allow all glibc 2.28 patches to be dropped from Buildroot.
Thanks
Mark
--
Mark Corbin
Embecosm Ltd.
https://www.embecosm.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv4] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444]
2019-01-03 14:44 ` Mark Corbin
@ 2019-01-03 17:24 ` Adhemerval Zanella
0 siblings, 0 replies; 23+ messages in thread
From: Adhemerval Zanella @ 2019-01-03 17:24 UTC (permalink / raw)
To: libc-alpha
On 03/01/2019 12:44, Mark Corbin wrote:
>
>
> On 01/10/2018 17:26, Martin Jansa wrote:
>> ---
>> ChangeLog | 4 ++++
>> sysdeps/ieee754/soft-fp/s_fdiv.c | 12 ++++++++++++
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/ChangeLog b/ChangeLog
>> index 4bafeefda5..9cdf59c9ab 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2018-09-30 Martin Jansa <Martin.Jansa@gmail.com>
>> + Partial fix for [BZ #23716]
>> + * sysdeps/ieee754/soft-fp/s_fdiv.c: Fix build with -O.
>> +
>> 2018-09-29 Martin Jansa <Martin.Jansa@gmail.com>
>> Partial fix for [BZ #23716]
>> * sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
>> diff --git a/sysdeps/ieee754/soft-fp/s_fdiv.c b/sysdeps/ieee754/soft-fp/s_fdiv.c
>> index 341339f5ed..f124edd760 100644
>> --- a/sysdeps/ieee754/soft-fp/s_fdiv.c
>> +++ b/sysdeps/ieee754/soft-fp/s_fdiv.c
>> @@ -25,6 +25,16 @@
>> #undef fdivl
>>
>> #include <math-narrow.h>
>> +
>> +#include <libc-diag.h>
>> +/* R_f[01] are not set in cases where it is not used in packing, but the
>> + compiler does not see that it is set in all cases where it is
>> + used, resulting in warnings that it may be used uninitialized.
>> + The location of the warning differs in different versions of GCC,
>> + it may be where R is defined using a macro or it may be where the
>> + macro is defined. This happens only with -O1. */
>> +DIAG_PUSH_NEEDS_COMMENT;
>> +DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
>> #include <soft-fp.h>
>> #include <single.h>
>> #include <double.h>
>> @@ -53,4 +63,6 @@ __fdiv (double x, double y)
>> CHECK_NARROW_DIV (ret, x, y);
>> return ret;
>> }
>> +DIAG_POP_NEEDS_COMMENT;
>> +
>> libm_alias_float_double (div)
>
> Hello
>
> It would be really helpful if this fix (commit id
> 4a06ceea33ecc220bbfe264d8f1e74de2f04e90d) could be backported from
> master to the release/2.28/master branch.
>
> This would allow all glibc 2.28 patches to be dropped from Buildroot.
>
> Thanks
>
> Mark
>
Done.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] locale: prevent maybe-uninitialized errors with -Os [BZ #19444]
2018-12-17 21:40 ` [PATCHv2 " Martin Jansa
@ 2019-01-04 23:08 ` Martin Jansa
2019-02-05 10:00 ` Martin Jansa
1 sibling, 0 replies; 23+ messages in thread
From: Martin Jansa @ 2019-01-04 23:08 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2605 bytes --]
On Mon, Dec 17, 2018 at 09:36:18PM +0000, Martin Jansa wrote:
> Fixes following error when building for aarch64 with -Os:
> | In file included from strcoll_l.c:43:
> | strcoll_l.c: In function '__strcoll_l':
> | ../locale/weight.h:31:26: error: 'seq2.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | int_fast32_t i = table[*(*cpp)++];
> | ^~~~~~~~~
> | strcoll_l.c:304:18: note: 'seq2.back_us' was declared here
> | coll_seq seq1, seq2;
> | ^~~~
> | In file included from strcoll_l.c:43:
> | ../locale/weight.h:31:26: error: 'seq1.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | int_fast32_t i = table[*(*cpp)++];
> | ^~~~~~~~~
> | strcoll_l.c:304:12: note: 'seq1.back_us' was declared here
> | coll_seq seq1, seq2;
> | ^~~~
>
> Partial fix for [BZ #19444]
> * locale/weight.h: Fix build with -Os.
Hi,
any feedback on this one?
Other 2 patches from this series were already merged.
Thanks
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
> ChangeLog | 4 ++++
> locale/weight.h | 7 +++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/ChangeLog b/ChangeLog
> index 2e12129846..2d5679c112 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,9 @@
> 2018-12-17 Martin Jansa <Martin.Jansa@gmail.com>
>
> + Partial fix for [BZ #19444]
> + * locale/weight.h: Fix build with -Os.
> +
> +2018-12-17 Martin Jansa <Martin.Jansa@gmail.com>
> Partial fix for [BZ #19444]
> * sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
> * sysdeps/ieee754/ldbl-96/e_jnl.c: Likewise.
> diff --git a/locale/weight.h b/locale/weight.h
> index 6028d3595e..10bcea25e5 100644
> --- a/locale/weight.h
> +++ b/locale/weight.h
> @@ -28,7 +28,14 @@ findidx (const int32_t *table,
> const unsigned char *extra,
> const unsigned char **cpp, size_t len)
> {
> + /* With GCC 8 when compiling with -Os the compiler warns that
> + seq1.back_us and seq2.back_us might be used uninitialized.
> + This uninitialized use is impossible for the same reason
> + as described in comments in locale/weightwc.h. */
> + DIAG_PUSH_NEEDS_COMMENT;
> + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
> int_fast32_t i = table[*(*cpp)++];
> + DIAG_POP_NEEDS_COMMENT;
> const unsigned char *cp;
> const unsigned char *usrc;
>
> --
> 2.17.1
>
--
Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] locale: prevent maybe-uninitialized errors with -Os [BZ #19444]
2018-12-17 21:40 ` [PATCHv2 " Martin Jansa
2019-01-04 23:08 ` Martin Jansa
@ 2019-02-05 10:00 ` Martin Jansa
1 sibling, 0 replies; 23+ messages in thread
From: Martin Jansa @ 2019-02-05 10:00 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2601 bytes --]
On Mon, Dec 17, 2018 at 09:36:18PM +0000, Martin Jansa wrote:
> Fixes following error when building for aarch64 with -Os:
> | In file included from strcoll_l.c:43:
> | strcoll_l.c: In function '__strcoll_l':
> | ../locale/weight.h:31:26: error: 'seq2.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | int_fast32_t i = table[*(*cpp)++];
> | ^~~~~~~~~
> | strcoll_l.c:304:18: note: 'seq2.back_us' was declared here
> | coll_seq seq1, seq2;
> | ^~~~
> | In file included from strcoll_l.c:43:
> | ../locale/weight.h:31:26: error: 'seq1.back_us' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> | int_fast32_t i = table[*(*cpp)++];
> | ^~~~~~~~~
> | strcoll_l.c:304:12: note: 'seq1.back_us' was declared here
> | coll_seq seq1, seq2;
> | ^~~~
>
> Partial fix for [BZ #19444]
> * locale/weight.h: Fix build with -Os.
ping?
This is waiting for review since 30 Sep 2018, please provide some
feedback.
Thanks
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
> ChangeLog | 4 ++++
> locale/weight.h | 7 +++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/ChangeLog b/ChangeLog
> index 2e12129846..2d5679c112 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,9 @@
> 2018-12-17 Martin Jansa <Martin.Jansa@gmail.com>
>
> + Partial fix for [BZ #19444]
> + * locale/weight.h: Fix build with -Os.
> +
> +2018-12-17 Martin Jansa <Martin.Jansa@gmail.com>
> Partial fix for [BZ #19444]
> * sysdeps/ieee754/dbl-96/e_jnl.c: Fix build with -O
> * sysdeps/ieee754/ldbl-96/e_jnl.c: Likewise.
> diff --git a/locale/weight.h b/locale/weight.h
> index 6028d3595e..10bcea25e5 100644
> --- a/locale/weight.h
> +++ b/locale/weight.h
> @@ -28,7 +28,14 @@ findidx (const int32_t *table,
> const unsigned char *extra,
> const unsigned char **cpp, size_t len)
> {
> + /* With GCC 8 when compiling with -Os the compiler warns that
> + seq1.back_us and seq2.back_us might be used uninitialized.
> + This uninitialized use is impossible for the same reason
> + as described in comments in locale/weightwc.h. */
> + DIAG_PUSH_NEEDS_COMMENT;
> + DIAG_IGNORE_Os_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
> int_fast32_t i = table[*(*cpp)++];
> + DIAG_POP_NEEDS_COMMENT;
> const unsigned char *cp;
> const unsigned char *usrc;
>
> --
> 2.17.1
>
--
Martin 'JaMa' Jansa jabber: Martin.Jansa@gmail.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2019-02-05 10:00 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 10:08 [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Martin Jansa
2018-09-19 10:08 ` [PATCH 2/2] soft-fp: ignore maybe-uninitialized Martin Jansa
2018-09-19 13:28 ` Joseph Myers
2018-09-30 15:53 ` [PATCHv2] sysdeps/ieee754/soft-fp: ignore maybe-uninitialized with -O [BZ #19444] Martin Jansa
2018-09-30 16:53 ` Joseph Myers
2018-09-30 17:49 ` Martin Jansa
2018-10-01 15:03 ` Joseph Myers
2018-10-01 16:26 ` [PATCHv3] " Martin Jansa
2018-10-01 16:46 ` [PATCHv4] " Martin Jansa
2018-10-02 15:42 ` Joseph Myers
2019-01-03 14:44 ` Mark Corbin
2019-01-03 17:24 ` Adhemerval Zanella
2018-09-30 15:54 ` [PATCH 3/3] locale: prevent maybe-uninitialized errors with -Os " Martin Jansa
2018-12-17 18:22 ` Martin Jansa
2018-12-17 21:40 ` [PATCHv2 " Martin Jansa
2019-01-04 23:08 ` Martin Jansa
2019-02-05 10:00 ` Martin Jansa
2018-09-19 13:23 ` [PATCH 1/2] sysdeps/ieee754: prevent maybe-uninitialized errors Joseph Myers
2018-09-29 10:46 ` [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444] Martin Jansa
2018-09-30 13:25 ` Joseph Myers
2018-09-30 13:50 ` Martin Jansa
2018-09-30 16:49 ` Joseph Myers
2018-09-29 19:39 ` [PATCHv3] " Martin Jansa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).