| Message ID | alpine.DEB.2.10.1508201800380.30940@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 20, 2015 at 1:00 PM, Joseph Myers <joseph@codesourcery.com> wrote: > The uninitialized variable warnings in math/ having been fixed for all > the supported floating-point formats, this patch removes the use of > -Wno-uninitialized there, continuing with the goal of avoiding -Wno- > options in makefiles as far as possible.. Not sure how you didn't see this, but my fresh build fails with this (GCC 4.8.4, Ubuntu 14.04, vanilla configure): gcc ../sysdeps/ieee754/ldbl-96/k_tanl.c -c -std=gnu99 -fgnu89-inline -fno-stack-protector -O2 -Wall -Werror -Wno-error=undef -Wundef -Wwrite-strings -fmerge-all-constants -frounding-math -g -Wstrict-prototypes -U_FORTIFY_SOURCE -D__NO_MATH_INLINES -D__LIBC_INTERNAL_MATH_INLINES -I../include -I/usr/local/google/home/stanshebs/base/linux/math -I/usr/local/google/home/stanshebs/base/linux -I../sysdeps/unix/sysv/linux/x86_64/64 -I../sysdeps/unix/sysv/linux/x86_64 -I../sysdeps/unix/sysv/linux/x86 -I../sysdeps/unix/sysv/linux/wordsize-64 -I../sysdeps/x86_64/nptl -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix/x86_64 -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/x86_64/64 -I../sysdeps/x86_64/fpu/multiarch -I../sysdeps/x86_64/fpu -I../sysdeps/x86/fpu/include -I../sysdeps/x86/fpu -I../sysdeps/x86_64/multiarch -I../sysdeps/x86_64 -I../sysdeps/x86 -I../sysdeps/ieee754/ldbl-96 -I../sysdeps/ieee754/dbl-64/wordsize-64 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/wordsize-64 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include /usr/local/google/home/stanshebs/base/linux/libc-modules.h -DMODULE_NAME=libm -include ../include/libc-symbols.h -o /usr/local/google/home/stanshebs/base/linux/math/k_tanl.o -MD -MP -MF /usr/local/google/home/stanshebs/base/linux/math/k_tanl.o.dt -MT /usr/local/google/home/stanshebs/base/linux/math/k_tanl.o ../sysdeps/ieee754/ldbl-96/k_tanl.c: In function ‘__kernel_tanl’: ../sysdeps/ieee754/ldbl-96/k_tanl.c:139:10: error: ‘sign’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if (sign < 0) ^ cc1: all warnings being treated as errors Stan
On Thu, 20 Aug 2015, Stan Shebs wrote: > On Thu, Aug 20, 2015 at 1:00 PM, Joseph Myers <joseph@codesourcery.com> wrote: > > The uninitialized variable warnings in math/ having been fixed for all > > the supported floating-point formats, this patch removes the use of > > -Wno-uninitialized there, continuing with the goal of avoiding -Wno- > > options in makefiles as far as possible.. > > Not sure how you didn't see this, but my fresh build fails with this > (GCC 4.8.4, Ubuntu 14.04, vanilla configure): I'm testing with GCC 5. Maybe newer GCC detects that sign is set under one "absx >= 0.6743316650390625L" conditional and used under the next such conditional, so is never actually used uninitialized, but older GCC doesn't detect that the conditionals are the same? Anyway, since you're testing with the compiler with which this fails, I advise submitting a patch that uses the DIAG_* macros from libc-internal.h to suppress the warning around the specific code that gets the warning, with a comment explaining why it's a false positive and with 4.8 named as the version in the call to DIAG_IGNORE_NEEDS_COMMENT. (Remember that when disabling -Wmaybe-uninitialized, you need to disable -Wuninitialized instead if !__GNUC_PREREQ (4, 7), as 4.6 doesn't have -Wmaybe-uninitialized.)
On Thu, Aug 20, 2015 at 3:47 PM, Joseph Myers <joseph@codesourcery.com> wrote: > On Thu, 20 Aug 2015, Stan Shebs wrote: > >> On Thu, Aug 20, 2015 at 1:00 PM, Joseph Myers <joseph@codesourcery.com> wrote: >> > The uninitialized variable warnings in math/ having been fixed for all >> > the supported floating-point formats, this patch removes the use of >> > -Wno-uninitialized there, continuing with the goal of avoiding -Wno- >> > options in makefiles as far as possible.. >> >> Not sure how you didn't see this, but my fresh build fails with this >> (GCC 4.8.4, Ubuntu 14.04, vanilla configure): > > I'm testing with GCC 5. Maybe newer GCC detects that sign is set under > one "absx >= 0.6743316650390625L" conditional and used under the next such > conditional, so is never actually used uninitialized, but older GCC > doesn't detect that the conditionals are the same? > > Anyway, since you're testing with the compiler with which this fails, I > advise submitting a patch that uses the DIAG_* macros from libc-internal.h > to suppress the warning around the specific code that gets the warning, > with a comment explaining why it's a false positive and with 4.8 named as > the version in the call to DIAG_IGNORE_NEEDS_COMMENT. (Remember that when > disabling -Wmaybe-uninitialized, you need to disable -Wuninitialized > instead if !__GNUC_PREREQ (4, 7), as 4.6 doesn't have > -Wmaybe-uninitialized.) Yes, that works, thanks! I notice that there are several pre-existing instances of this pattern, but some list 4.9 and others list 5 as the version - does anybody know if there is a canonical minimal GCC version for the better uninitialized warning, or is it just whatever was getting tested at the time? Stan
On Thu, 20 Aug 2015, Stan Shebs wrote: > Yes, that works, thanks! I notice that there are several pre-existing > instances of this pattern, but some list 4.9 and others list 5 as the > version - does anybody know if there is a canonical minimal GCC > version for the better uninitialized warning, or is it just whatever > was getting tested at the time? The version listed with the DIAG_* macros is always the version that is observed to have the bad warning - the point of the version number is to indicate to recheck if the bad warning is still seen after support for that version is obsoleted. It's whatever someone was testing with, not necessarily the most recent version with the bad warning (so if listed as 4.8, it may well still be the case that the warning is present with current GCC trunk).
On Thu, 2015-08-20 at 18:00 +0000, Joseph Myers wrote: > The uninitialized variable warnings in math/ having been fixed for all > the supported floating-point formats, this patch removes the use of > -Wno-uninitialized there, continuing with the goal of avoiding -Wno- > options in makefiles as far as possible.. > > Tested for x86_64 and x86 (full build and testsuite runs), and for > powerpc and mips64 (verified that glibc builds without errors). > Committed. I am seeing a mips32 build failure (using Top-of-tree GCC) with this change: In file included from ../soft-fp/soft-fp.h:321:0, from ../soft-fp/fmatf4.c:30, from ../sysdeps/mips/ieee754/s_fmal.c:7: ../soft-fp/fmatf4.c: In function �~@~X__fmal�~@~Y: ../soft-fp/op-common.h:281:10: error: �~@~XR_e�~@~Y may be used uninitialized in this function [-Werror=maybe-uninitialized] X##_e++; \ ^ ../soft-fp/fmatf4.c:40:14: note: �~@~XR_e�~@~Y was declared here FP_DECL_Q (R); ^ ../soft-fp/op-common.h:38:14: note: in definition of macro �~@~X_FP_DECL�~@~Y _FP_I_TYPE X##_e __attribute__ ((unused)) _FP_ZERO_INIT; \ ^ ../soft-fp/fmatf4.c:40:3: note: in expansion of macro �~@~XFP_DECL_Q�~@~Y FP_DECL_Q (R); I am not sure if a DIAG is needed or if something actually needs to be initialized. Steve Ellcey sellcey@imgtec.com
On Fri, 21 Aug 2015, Steve Ellcey wrote: > On Thu, 2015-08-20 at 18:00 +0000, Joseph Myers wrote: > > The uninitialized variable warnings in math/ having been fixed for all > > the supported floating-point formats, this patch removes the use of > > -Wno-uninitialized there, continuing with the goal of avoiding -Wno- > > options in makefiles as far as possible.. > > > > Tested for x86_64 and x86 (full build and testsuite runs), and for > > powerpc and mips64 (verified that glibc builds without errors). > > Committed. > > I am seeing a mips32 build failure (using Top-of-tree GCC) with this > change: > > In file included from ../soft-fp/soft-fp.h:321:0, > from ../soft-fp/fmatf4.c:30, mips32 shouldn't be using fmatf4; sysdeps/mips/ieee754/s_fmal.c even has a #error to make sure of this. Do you mean mips64? There are also diagnostic control macros in fmatf4.c specifically to disable this warning, so you'll need to look at why those aren't working for you.
On Fri, 2015-08-21 at 16:10 +0000, Joseph Myers wrote: > > > > I am seeing a mips32 build failure (using Top-of-tree GCC) with this > > change: > > > > In file included from ../soft-fp/soft-fp.h:321:0, > > from ../soft-fp/fmatf4.c:30, > > mips32 shouldn't be using fmatf4; sysdeps/mips/ieee754/s_fmal.c even has a > #error to make sure of this. Do you mean mips64? > > There are also diagnostic control macros in fmatf4.c specifically to > disable this warning, so you'll need to look at why those aren't working > for you. Ah, it is mips64 but with the N32 ABI. Steve Ellcey sellcey@imgtec.com
On Fri, 21 Aug 2015, Steve Ellcey wrote: > On Fri, 2015-08-21 at 16:10 +0000, Joseph Myers wrote: > > > > > > > I am seeing a mips32 build failure (using Top-of-tree GCC) with this > > > change: > > > > > > In file included from ../soft-fp/soft-fp.h:321:0, > > > from ../soft-fp/fmatf4.c:30, > > > > mips32 shouldn't be using fmatf4; sysdeps/mips/ieee754/s_fmal.c even has a > > #error to make sure of this. Do you mean mips64? > > > > There are also diagnostic control macros in fmatf4.c specifically to > > disable this warning, so you'll need to look at why those aren't working > > for you. > > Ah, it is mips64 but with the N32 ABI. OK, that explains why the file is being compiled. As it's GCC trunk, maybe the problem can be reduced to a GCC bug report and the bug fixed well before GCC 6 is out (given that I don't see it with GCC 5, so it's likely a regression)? If not, maybe moving the PUSH / IGNORE macros up to be right after the libc-internal.h inclusion helps? Suppressing warnings for a whole file isn't ideal, but it's better than suppressing them for the whole of libm.
On Fri, 2015-08-21 at 16:39 +0000, Joseph Myers wrote: > > Ah, it is mips64 but with the N32 ABI. > > OK, that explains why the file is being compiled. > > As it's GCC trunk, maybe the problem can be reduced to a GCC bug report > and the bug fixed well before GCC 6 is out (given that I don't see it with > GCC 5, so it's likely a regression)? If not, maybe moving the PUSH / > IGNORE macros up to be right after the libc-internal.h inclusion helps? > Suppressing warnings for a whole file isn't ideal, but it's better than > suppressing them for the whole of libm. There is something weird going on. If I cut out the line that compiles s_fmal.c I can run that command and get the error. If I change the '-c' to '-E' to get a preprocessed source file and then compile that with '-c' (and all the other options normally used in the compilation) I don't get the error. Something about expanding the macros seems to make the error go away. I will see if I can create a standalone test case with the unexpanded macros still in it. Steve Ellcey sellcey@imgtec.com
Steve Ellcey <sellcey@imgtec.com> writes: > There is something weird going on. If I cut out the line that compiles > s_fmal.c I can run that command and get the error. If I change the '-c' > to '-E' to get a preprocessed source file and then compile that with > '-c' (and all the other options normally used in the compilation) I > don't get the error. Probably something is inappropriately setting the system-header flag in the line directives. Does it still happen if you preprocess with -P? Andreas.
On Fri, 2015-08-21 at 19:50 +0200, Andreas Schwab wrote: > Steve Ellcey <sellcey@imgtec.com> writes: > > > There is something weird going on. If I cut out the line that compiles > > s_fmal.c I can run that command and get the error. If I change the '-c' > > to '-E' to get a preprocessed source file and then compile that with > > '-c' (and all the other options normally used in the compilation) I > > don't get the error. > > Probably something is inappropriately setting the system-header flag in > the line directives. Does it still happen if you preprocess with -P? > > Andreas. I still do not get the warning when I use -P (i.e. I preprocessed with '-E -P' and then did a '-c' on that file. I did not get the warning/error that I see with -c on the original source file. Steve Ellcey sellcey@imgtec.com
Joseph Myers <joseph@codesourcery.com> writes:
> * math/Makefile (CFLAGS): Don't add -Wno-uninitialized.
That breaks aarch64.
In file included from ../include/fpu_control.h:1:0,
from ../sysdeps/aarch64/fpu/math_private.h:23,
from ../sysdeps/ieee754/dbl-64/e_exp.c:41:
../sysdeps/ieee754/dbl-64/e_exp.c: In function '__ieee754_exp':
../sysdeps/aarch64/fpu/fpu_control.h:28:3: error: 'ctx.env.__fpcr' may be used uninitialized in this function [-Werror=maybe-uninitialized]
__asm__ __volatile__ ("msr fpcr, %0" : : "r" (fpcr))
^
In file included from ../sysdeps/ieee754/flt-32/math_private.h:3:0,
from ../sysdeps/ieee754/dbl-64/wordsize-64/math_private.h:3,
from ../sysdeps/aarch64/fpu/math_private.h:326,
from ../sysdeps/ieee754/dbl-64/e_exp.c:41:
../sysdeps/generic/math_private.h:652:17: note: 'ctx.env.__fpcr' was declared here
struct rm_ctx ctx __attribute__((cleanup (CLEANUPFUNC ## _ctx))); \
^
../sysdeps/generic/math_private.h:661:3: note: in expansion of macro 'SET_RESTORE_ROUND_GENERIC'
SET_RESTORE_ROUND_GENERIC (RM, libc_feholdsetround, libc_feresetround)
^
../sysdeps/ieee754/dbl-64/e_exp.c:63:5: note: in expansion of macro 'SET_RESTORE_ROUND'
SET_RESTORE_ROUND (FE_TONEAREST);
^
Andreas.
On Mon, 24 Aug 2015, Andreas Schwab wrote: > That breaks aarch64. > > In file included from ../include/fpu_control.h:1:0, > from ../sysdeps/aarch64/fpu/math_private.h:23, > from ../sysdeps/ieee754/dbl-64/e_exp.c:41: > ../sysdeps/ieee754/dbl-64/e_exp.c: In function '__ieee754_exp': > ../sysdeps/aarch64/fpu/fpu_control.h:28:3: error: 'ctx.env.__fpcr' may be used uninitialized in this function [-Werror=maybe-uninitialized] > __asm__ __volatile__ ("msr fpcr, %0" : : "r" (fpcr)) > ^ > In file included from ../sysdeps/ieee754/flt-32/math_private.h:3:0, > from ../sysdeps/ieee754/dbl-64/wordsize-64/math_private.h:3, > from ../sysdeps/aarch64/fpu/math_private.h:326, > from ../sysdeps/ieee754/dbl-64/e_exp.c:41: > ../sysdeps/generic/math_private.h:652:17: note: 'ctx.env.__fpcr' was declared here > struct rm_ctx ctx __attribute__((cleanup (CLEANUPFUNC ## _ctx))); \ > ^ > ../sysdeps/generic/math_private.h:661:3: note: in expansion of macro 'SET_RESTORE_ROUND_GENERIC' > SET_RESTORE_ROUND_GENERIC (RM, libc_feholdsetround, libc_feresetround) > ^ > ../sysdeps/ieee754/dbl-64/e_exp.c:63:5: note: in expansion of macro 'SET_RESTORE_ROUND' > SET_RESTORE_ROUND (FE_TONEAREST); > ^ Does putting the DIAG_* macros (with appropriate comments) in the relevant inline functions (possibly libc_feresetround_aarch64_ctx or libc_feresetround_noex_aarch64_ctx) help? That would seem to be the right approach for this (if the issue is that ctx.env.__fpcr is only used if ctx.updated_status, which is only true if ctx.env.__fpcr was set earlier, but the compiler can't see this). (Failing that, maybe selected files need -Wno-uninitialized for aarch64, which would still be better than having it globally for all of math/ on all architectures.)
On 08/21/2015 12:04 PM, Steve Ellcey wrote: > On Thu, 2015-08-20 at 18:00 +0000, Joseph Myers wrote: >> The uninitialized variable warnings in math/ having been fixed for all >> the supported floating-point formats, this patch removes the use of >> -Wno-uninitialized there, continuing with the goal of avoiding -Wno- >> options in makefiles as far as possible.. >> >> Tested for x86_64 and x86 (full build and testsuite runs), and for >> powerpc and mips64 (verified that glibc builds without errors). >> Committed. > I am seeing a mips32 build failure (using Top-of-tree GCC) with this > change: > > In file included from ../soft-fp/soft-fp.h:321:0, > from ../soft-fp/fmatf4.c:30, > from ../sysdeps/mips/ieee754/s_fmal.c:7: > ../soft-fp/fmatf4.c: In function �~@~X__fmal�~@~Y: > ../soft-fp/op-common.h:281:10: error: �~@~XR_e�~@~Y may be used uninitialized in this function [-Werror=maybe-uninitialized] > X##_e++; \ > ^ > ../soft-fp/fmatf4.c:40:14: note: �~@~XR_e�~@~Y was declared here > FP_DECL_Q (R); > ^ > ../soft-fp/op-common.h:38:14: note: in definition of macro �~@~X_FP_DECL�~@~Y > _FP_I_TYPE X##_e __attribute__ ((unused)) _FP_ZERO_INIT; \ > ^ > ../soft-fp/fmatf4.c:40:3: note: in expansion of macro �~@~XFP_DECL_Q�~@~Y > FP_DECL_Q (R); Doing a tilegx build just now against the tip, I see something similar, using gcc 4.8.2: In file included from ../soft-fp/soft-fp.h:321:0, from ../soft-fp/fmadf4.c:30, from ../sysdeps/tile/s_fma.c:1: ../soft-fp/fmadf4.c: In function ‘__fma’: ../soft-fp/op-common.h:274:10: error: ‘R_e’ may be used uninitialized in this function [-Werror=maybe-uninitialized] X##_e += _FP_EXPBIAS_##fs; \ ^ ../soft-fp/fmadf4.c:40:14: note: ‘R_e’ was declared here FP_DECL_D (R); ^ ../soft-fp/op-common.h:38:14: note: in definition of macro ‘_FP_DECL’ _FP_I_TYPE X##_e __attribute__ ((unused)) _FP_ZERO_INIT; \ ^ ../soft-fp/fmadf4.c:40:3: note: in expansion of macro ‘FP_DECL_D’ FP_DECL_D (R); ^
On Wed, 26 Aug 2015, Chris Metcalf wrote: > Doing a tilegx build just now against the tip, I see something similar, > using gcc 4.8.2: > > In file included from ../soft-fp/soft-fp.h:321:0, > from ../soft-fp/fmadf4.c:30, > from ../sysdeps/tile/s_fma.c:1: > ../soft-fp/fmadf4.c: In function ‘__fma’: > ../soft-fp/op-common.h:274:10: error: ‘R_e’ may be used uninitialized in this > function [-Werror=maybe-uninitialized] I wonder if it's something to do with macro locations, and moving the push / ignore right up above the include of soft-fp.h helps (in which case there isn't much point in the pop)? It works for me as-is with GCC 5, but given the various reports of problems, maybe it's too fragile and the warning needs disabling for the whole file in soft-fp/fma*.c.
On Wed, 2015-08-26 at 21:18 +0000, Joseph Myers wrote: > On Wed, 26 Aug 2015, Chris Metcalf wrote: > > > Doing a tilegx build just now against the tip, I see something similar, > > using gcc 4.8.2: > > > > In file included from ../soft-fp/soft-fp.h:321:0, > > from ../soft-fp/fmadf4.c:30, > > from ../sysdeps/tile/s_fma.c:1: > > ../soft-fp/fmadf4.c: In function ‘__fma’: > > ../soft-fp/op-common.h:274:10: error: ‘R_e’ may be used uninitialized in this > > function [-Werror=maybe-uninitialized] > > I wonder if it's something to do with macro locations, and moving the push > / ignore right up above the include of soft-fp.h helps (in which case > there isn't much point in the pop)? It works for me as-is with GCC 5, but > given the various reports of problems, maybe it's too fragile and the > warning needs disabling for the whole file in soft-fp/fma*.c. I tried moving the diag/ignore to just after the include of libc-internal.h and I got errors about __GNUC_PREREQ not being defined. If I put the diag/ignore's after the include of math.h (and libc-internal.h) then things seemed to work. Any idea why I need to include math.h to get __GNUC_PREREQ defined? Steve Ellcey sellcey@imgtec.com
On Wed, 26 Aug 2015, Steve Ellcey wrote: > I tried moving the diag/ignore to just after the include of > libc-internal.h and I got errors about __GNUC_PREREQ not being defined. > If I put the diag/ignore's after the include of math.h (and > libc-internal.h) then things seemed to work. Any idea why I need to > include math.h to get __GNUC_PREREQ defined? Public headers include <features.h>. Internal headers don't necessarily do so.
diff --git a/math/Makefile b/math/Makefile index d3b483d..c98c3c4 100644 --- a/math/Makefile +++ b/math/Makefile @@ -249,9 +249,6 @@ ifneq ($(long-double-fcts),yes) math-CPPFLAGS += -DNO_LONG_DOUBLE -D_Mlong_double_=double endif -# The fdlibm code generates a lot of these warnings but is otherwise clean. -override CFLAGS += -Wno-uninitialized - # The -lieee library is actually an object file. # The module just defines the _LIB_VERSION_ variable. # It's not a library to make sure it is linked in instead of s_lib_version.o.