fix #19444 - build failures with -O1 due to -Wmaybe-uninitialized

Message ID 56968319.3070802@gmail.com
State New, archived
Headers

Commit Message

Martin Sebor Jan. 13, 2016, 5:02 p.m. UTC
  The Glibc FAQ on the Wiki documents the limitation that the library
prevents users from building it without optimization (-O0).  However,
the mechanism in place doesn't prevent using -O1, and the FAQ doesn't
mention that this level cannot or should not be used.  Using a lower
level of optimization than -O2 can be helpful in debugging.

However, trunk fails to build with -O1 due to a number of warnings
about possibly uninitialized that are treated as errors.  It turns
out that the warnings are false positives caused by a documented
limitation of the GCC implementation (the value range propagation
optimization is disabled at -O1, which prevents GCC from seeing
that in some of the non-trivial cases of apparently uninitialized
variables the variables are in fact initialized when used).

In his comments on the bug, Carlos suggests to fix these instances
of false positives and to get -O1 to work.  The attached patch
does just that.  In the patch, to minimize the impact of the
(otherwise unnecessary) initialization, rather than initializing
them for all the code paths, I reduced the scope of the local
variables that are subject to the warning and added the redundant
initialization only for the problem code paths.  This led to more
changes that would otherwise be required but resulted in code
that's easier to follow.

The patch also adds -Wno-error=maybe-uninitialized to the warning
options when -O1 or lower is set in CFLAGS to prevent these false
positives from causing build failures.  This change renders the
changes above strictly unnecessary.  I include both since I think
both are worthwhile but I can remove one or the other if others
have a different preference.

Tested on x86_64.

Martin
  

Comments

Joseph Myers Jan. 13, 2016, 5:46 p.m. UTC | #1
On Wed, 13 Jan 2016, Martin Sebor wrote:

> In his comments on the bug, Carlos suggests to fix these instances
> of false positives and to get -O1 to work.  The attached patch
> does just that.  In the patch, to minimize the impact of the
> (otherwise unnecessary) initialization, rather than initializing
> them for all the code paths, I reduced the scope of the local
> variables that are subject to the warning and added the redundant
> initialization only for the problem code paths.  This led to more
> changes that would otherwise be required but resulted in code
> that's easier to follow.

In general, we want to avoid unnecessary initializations.  I'd prefer 
adding a default case to the relevant switch statements that calls 
__builtin_unreachable (with a comment explaining why it's unreachable, and 
that when VRP is disabled spurious uninitialized warnings occur without 
the __builtin_unreachable call).

> The patch also adds -Wno-error=maybe-uninitialized to the warning
> options when -O1 or lower is set in CFLAGS to prevent these false

In general, we want to avoid -Wno- options, only adding them if the issue 
can't be fixed in the code (e.g. for code imported verbatim from 
elsewhere, or for testcases whose purpose is to test what the library does 
for a dubious usage that GCC warns about, such as the fortification tests, 
and where the diagnostic control pragmas can't be used for some reason).  
And we want to migrate existing -Wno- options to source fixes / uses of 
DIAG_* macros where possible.

That is, I think we should fix only the sources, without a makefile 
change, and should fix the sources using __builtin_unreachable calls.

(I agree that -O1 and -Os builds ought to work without any extra build or 
test failures, and preferably -O0 should work except for the specific 
files requiring optimization.)
  
Mike Frysinger Jan. 13, 2016, 5:50 p.m. UTC | #2
On 13 Jan 2016 10:02, Martin Sebor wrote:
> In his comments on the bug, Carlos suggests to fix these instances
> of false positives and to get -O1 to work.  The attached patch
> does just that.  In the patch, to minimize the impact of the
> (otherwise unnecessary) initialization, rather than initializing
> them for all the code paths, I reduced the scope of the local
> variables that are subject to the warning and added the redundant
> initialization only for the problem code paths.  This led to more
> changes that would otherwise be required but resulted in code
> that's easier to follow.

any time code is changed to address warnings, especially "harmless" ones,
the first question is "has the compiled output changed".  if gcc produces
worse code at -O2, then this is probably not the right direction.

> The patch also adds -Wno-error=maybe-uninitialized to the warning
> options when -O1 or lower is set in CFLAGS to prevent these false
> positives from causing build failures.  This change renders the
> changes above strictly unnecessary.  I include both since I think
> both are worthwhile but I can remove one or the other if others
> have a different preference.

i'm in favor of this myself.  we clearly do not test -O1 as the normal
course of things, so it'll be perpetually broken.
-mike
  
Andreas Schwab Jan. 13, 2016, 5:52 p.m. UTC | #3
Martin Sebor <msebor@gmail.com> writes:

> +# GCC's -Wmaybe-uninitialized depends on optimization and is documented
> +# to issue false positives at -O1 (or lower).  If -O1 or lower is found
> +# in CFLAGS, prevent such warnings from being treated as errors.
> +ifneq ($(filter-out -O -O1, $(CFLAGS)),$(CFLAGS))

That doesn't grasp the true effect of CFLAGS, since it is easy to get
multiple optimisation options into it.

Andreas.
  
H.J. Lu Jan. 13, 2016, 5:52 p.m. UTC | #4
On Wed, Jan 13, 2016 at 9:46 AM, Joseph Myers
> (I agree that -O1 and -Os builds ought to work without any extra build or
> test failures, and preferably -O0 should work except for the specific
> files requiring optimization.)

I opened an -Os bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=15105
  
Joseph Myers Jan. 13, 2016, 5:58 p.m. UTC | #5
On Wed, 13 Jan 2016, H.J. Lu wrote:

> On Wed, Jan 13, 2016 at 9:46 AM, Joseph Myers
> > (I agree that -O1 and -Os builds ought to work without any extra build or
> > test failures, and preferably -O0 should work except for the specific
> > files requiring optimization.)
> 
> I opened an -Os bug:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=15105

It would be good to have an updated list of issues seen.  I'd expect most 
such issues to be easy to fix (adding missing *_hidden_* for functions 
where PLT avoidance currently relies on inlining, for example).
  
H.J. Lu Jan. 13, 2016, 6:11 p.m. UTC | #6
On Wed, Jan 13, 2016 at 9:58 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 13 Jan 2016, H.J. Lu wrote:
>
>> On Wed, Jan 13, 2016 at 9:46 AM, Joseph Myers
>> > (I agree that -O1 and -Os builds ought to work without any extra build or
>> > test failures, and preferably -O0 should work except for the specific
>> > files requiring optimization.)
>>
>> I opened an -Os bug:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=15105
>
> It would be good to have an updated list of issues seen.  I'd expect most
> such issues to be easy to fix (adding missing *_hidden_* for functions
> where PLT avoidance currently relies on inlining, for example).

This is a regression:

https://sourceware.org/bugzilla/show_bug.cgi?id=19462
  
Carlos O'Donell Jan. 13, 2016, 6:14 p.m. UTC | #7
On 01/13/2016 01:11 PM, H.J. Lu wrote:
> On Wed, Jan 13, 2016 at 9:58 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Wed, 13 Jan 2016, H.J. Lu wrote:
>>
>>> On Wed, Jan 13, 2016 at 9:46 AM, Joseph Myers
>>>> (I agree that -O1 and -Os builds ought to work without any extra build or
>>>> test failures, and preferably -O0 should work except for the specific
>>>> files requiring optimization.)
>>>
>>> I opened an -Os bug:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=15105
>>
>> It would be good to have an updated list of issues seen.  I'd expect most
>> such issues to be easy to fix (adding missing *_hidden_* for functions
>> where PLT avoidance currently relies on inlining, for example).
> 
> This is a regression:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19462
> 

I'm still setting up VMs locally to do this kind of testing, CI builds
with -O1, -O0, and -Os. So hopefully we'll cover common configurations
this way on x86_64.

Cheers,
Carlos.
  
Martin Sebor Jan. 13, 2016, 7:18 p.m. UTC | #8
On 01/13/2016 10:50 AM, Mike Frysinger wrote:
> On 13 Jan 2016 10:02, Martin Sebor wrote:
>> In his comments on the bug, Carlos suggests to fix these instances
>> of false positives and to get -O1 to work.  The attached patch
>> does just that.  In the patch, to minimize the impact of the
>> (otherwise unnecessary) initialization, rather than initializing
>> them for all the code paths, I reduced the scope of the local
>> variables that are subject to the warning and added the redundant
>> initialization only for the problem code paths.  This led to more
>> changes that would otherwise be required but resulted in code
>> that's easier to follow.
>
> any time code is changed to address warnings, especially "harmless" ones,
> the first question is "has the compiled output changed".  if gcc produces
> worse code at -O2, then this is probably not the right direction.

I did look at the code emitted for sysdeps/ieee754/dbl-64/e_jn.c
on x86_64, both with the added initialization and with the
default case with __builtin_unreachable.

The code changes in small but but IMO not significant ways with
either approach.  Aside from a few changes in the used general
purpose registers, the patch results in three more SSE2 register
to register moves and a few more bytes of padding.  The default
case approach results in bigger overall changes to the emitted
code but (AFACT) fewer added instructions.

FWIW, between the two approaches, my general preference is to
initialize variables rather than adding special annotation.
It makes code cleaner and easier to follow, and in tricky cases
it takes the "what if?" question off the table.  In the case of
the simple __ieee754_jn function, if adding the default case
with __builtin_unreachable is preferable, that's fine with me
too.

In the other cases, such as do_sincos_1() in
sysdeps/ieee754/dbl-64/s_sin.c, there's no case and switch
statement to add __builtin_unreachable to.  Other similar
functions in this file already initialize the retval local
variable, so doing it consistently makes sense.

>> The patch also adds -Wno-error=maybe-uninitialized to the warning
>> options when -O1 or lower is set in CFLAGS to prevent these false
>> positives from causing build failures.  This change renders the
>> changes above strictly unnecessary.  I include both since I think
>> both are worthwhile but I can remove one or the other if others
>> have a different preference.
>
> i'm in favor of this myself.  we clearly do not test -O1 as the normal
> course of things, so it'll be perpetually broken.
> -mike

Thanks
Martin
  
Joseph Myers Jan. 13, 2016, 11:23 p.m. UTC | #9
On Wed, 13 Jan 2016, Martin Sebor wrote:

> FWIW, between the two approaches, my general preference is to
> initialize variables rather than adding special annotation.

Established glibc practice is not to add such initializations where the 
initial value can't be used.

> In the other cases, such as do_sincos_1() in
> sysdeps/ieee754/dbl-64/s_sin.c, there's no case and switch
> statement to add __builtin_unreachable to.  Other similar

There certainly seems to be such a switch statement to me.

  int k1 = (n + k) & 3;
  switch (k1)

> functions in this file already initialize the retval local
> variable, so doing it consistently makes sense.

Well, consistently (with glibc practice) would be to remove the 
unnecessary initializations.
  
Joseph Myers Jan. 14, 2016, 6:06 p.m. UTC | #10
On Wed, 13 Jan 2016, H.J. Lu wrote:

> > It would be good to have an updated list of issues seen.  I'd expect most
> > such issues to be easy to fix (adding missing *_hidden_* for functions
> > where PLT avoidance currently relies on inlining, for example).
> 
> This is a regression:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19462

I think the right way to fix those _STRING_ARCH_unaligned issues is: get 
Wilco's patch <https://sourceware.org/ml/libc-alpha/2016-01/msg00170.html> 
reviewed (which we need to do for 2.23 anyway as an ABI issue for 
AArch64), then move _STRING_ARCH_unaligned to a separate non-installed 
header, not in bits/, which is included directly by the files that need 
it.  (It's possible that Wilco's patch depends on other patches to 
eliminate _STRING_ARCH_unaligned tests from installed headers except where 
they are replaced by _STRING_INLINE_unaligned.)
  
H.J. Lu Jan. 14, 2016, 6:12 p.m. UTC | #11
On Thu, Jan 14, 2016 at 10:06 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 13 Jan 2016, H.J. Lu wrote:
>
>> > It would be good to have an updated list of issues seen.  I'd expect most
>> > such issues to be easy to fix (adding missing *_hidden_* for functions
>> > where PLT avoidance currently relies on inlining, for example).
>>
>> This is a regression:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=19462
>
> I think the right way to fix those _STRING_ARCH_unaligned issues is: get
> Wilco's patch <https://sourceware.org/ml/libc-alpha/2016-01/msg00170.html>
> reviewed (which we need to do for 2.23 anyway as an ABI issue for
> AArch64), then move _STRING_ARCH_unaligned to a separate non-installed
> header, not in bits/, which is included directly by the files that need
> it.  (It's possible that Wilco's patch depends on other patches to
> eliminate _STRING_ARCH_unaligned tests from installed headers except where
> they are replaced by _STRING_INLINE_unaligned.)
>

This is the right direction, but may not work with -Os.  I am working
on a patch to add bits/string-1.h to define _STRING_ARCH_unaligned,
which is included unconditionally.
  
Joseph Myers Jan. 14, 2016, 6:49 p.m. UTC | #12
On Thu, 14 Jan 2016, H.J. Lu wrote:

> This is the right direction, but may not work with -Os.  I am working
> on a patch to add bits/string-1.h to define _STRING_ARCH_unaligned,
> which is included unconditionally.

Suppose you apply all Wilco's patches related to string inlines (including 
those that remove many of them from bits/string2.h).  Are there any 
remaining references to _STRING_ARCH_unaligned in installed headers other 
than the definition?

If so, what are they?  If not, it should be moved out of installed headers 
as I said (and bits/ is a namespace exclusively for installed headers).  
Installed headers should not define things only relevant for building 
glibc, or contain _LIBC conditionals, except where we don't have a cleaner 
alternative for how to avoid internals creeping into installed headers.  
The first preference should be that internal things go in internal headers 
(which includes the include/ wrappers, though arguably those should be 
slimmed down as well with more internal interfaces going in separate 
internal headers that are explicitly included, not in a header with the 
same name as one that gets installed).
  
H.J. Lu Jan. 14, 2016, 6:59 p.m. UTC | #13
On Thu, Jan 14, 2016 at 10:49 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Thu, 14 Jan 2016, H.J. Lu wrote:
>
>> This is the right direction, but may not work with -Os.  I am working
>> on a patch to add bits/string-1.h to define _STRING_ARCH_unaligned,
>> which is included unconditionally.
>
> Suppose you apply all Wilco's patches related to string inlines (including
> those that remove many of them from bits/string2.h).  Are there any

His patch can't be applied since it contains odd characters.

> remaining references to _STRING_ARCH_unaligned in installed headers other
> than the definition?
>
> If so, what are they?  If not, it should be moved out of installed headers
> as I said (and bits/ is a namespace exclusively for installed headers).

crypt/md5.c:#if !_STRING_ARCH_unaligned
crypt/sha256.c:#if _STRING_ARCH_unaligned
crypt/sha256.c:#if !_STRING_ARCH_unaligned
crypt/sha512.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/loop.c:#if _STRING_ARCH_unaligned || !defined DEFINE_UNALIGNED
iconv/loop.c:#if !defined DEFINE_UNALIGNED && !_STRING_ARCH_unaligned \
iconv/skeleton.c:#if _STRING_ARCH_unaligned
iconv/skeleton.c:  (!_STRING_ARCH_unaligned              \
include/arpa/nameser.h:#if _STRING_ARCH_unaligned
nscd/nscd_gethst_r.c:#if !_STRING_ARCH_unaligned
nscd/nscd_getserv_r.c:#if !_STRING_ARCH_unaligned
nscd/nscd_helper.c:#if !_STRING_ARCH_unaligned
nscd/nscd_helper.c:#if !_STRING_ARCH_unaligned
nscd/nscd_helper.c:#if !_STRING_ARCH_unaligned
resolv/res_send.c:#if _STRING_ARCH_unaligned
resolv/res_send.c:#if _STRING_ARCH_unaligned
stdlib/getenv.c:#if __BYTE_ORDER == __LITTLE_ENDIAN || !_STRING_ARCH_unaligned
stdlib/getenv.c:#if _STRING_ARCH_unaligned
stdlib/getenv.c:#if _STRING_ARCH_unaligned
stdlib/getenv.c:#if _STRING_ARCH_unaligned

> Installed headers should not define things only relevant for building
> glibc, or contain _LIBC conditionals, except where we don't have a cleaner
> alternative for how to avoid internals creeping into installed headers.
> The first preference should be that internal things go in internal headers
> (which includes the include/ wrappers, though arguably those should be
> slimmed down as well with more internal interfaces going in separate
> internal headers that are explicitly included, not in a header with the
> same name as one that gets installed).
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
  
Joseph Myers Jan. 14, 2016, 9 p.m. UTC | #14
On Thu, 14 Jan 2016, H.J. Lu wrote:

> > Suppose you apply all Wilco's patches related to string inlines (including
> > those that remove many of them from bits/string2.h).  Are there any
> 
> His patch can't be applied since it contains odd characters.

Well, the patches should be reviewed.  Presumably he has clean copies of 
them.

> > remaining references to _STRING_ARCH_unaligned in installed headers other
> > than the definition?
> >
> > If so, what are they?  If not, it should be moved out of installed headers
> > as I said (and bits/ is a namespace exclusively for installed headers).
> 
> crypt/md5.c:#if !_STRING_ARCH_unaligned

This is a list of references outside installed headers.  What I want is a 
list inside such headers, after all relevant patches have been applied.

For example: there are lots of references in the installed header 
bits/string2.h at present.  So we can't move the macro out of installed 
headers until those references go away.  Does 
<https://sourceware.org/ml/libc-alpha/2015-12/msg00386.html>, together 
with <https://sourceware.org/ml/libc-alpha/2015-10/msg00265.html>, move 
all those references to a .c file or not?
  
H.J. Lu Jan. 14, 2016, 9:30 p.m. UTC | #15
On Thu, Jan 14, 2016 at 1:00 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Thu, 14 Jan 2016, H.J. Lu wrote:
>
>> > Suppose you apply all Wilco's patches related to string inlines (including
>> > those that remove many of them from bits/string2.h).  Are there any
>>
>> His patch can't be applied since it contains odd characters.
>
> Well, the patches should be reviewed.  Presumably he has clean copies of
> them.
>
>> > remaining references to _STRING_ARCH_unaligned in installed headers other
>> > than the definition?
>> >
>> > If so, what are they?  If not, it should be moved out of installed headers
>> > as I said (and bits/ is a namespace exclusively for installed headers).
>>
>> crypt/md5.c:#if !_STRING_ARCH_unaligned
>
> This is a list of references outside installed headers.  What I want is a
> list inside such headers, after all relevant patches have been applied.

-Os problem is reference of _STRING_ARCH_unaligned in
glibc, which isn't defined with -Os.  If it is allowed to use in
glibc, its definition should always be available for glibc build.

> For example: there are lots of references in the installed header
> bits/string2.h at present.  So we can't move the macro out of installed
> headers until those references go away.  Does
> <https://sourceware.org/ml/libc-alpha/2015-12/msg00386.html>, together
> with <https://sourceware.org/ml/libc-alpha/2015-10/msg00265.html>, move
> all those references to a .c file or not?

They look promising.  I will wait until they are settled down.
  

Patch

2016-01-13  Martin Sebor  <msebor@redhat.com>

	[BZ #19444]
	* Makeconfig (gccwarn): Add -Wno-error=maybe-uninitialized when
	-O or -O1 is found in CFLAGS.
	* sysdeps/ieee754/dbl-64/e_jn.c (__ieee754_jn): Move variable
	declaration closer to its use and initialize to avoid false
	positive -Wmaybe-uninitialized warnings.
	* sysdeps/ieee754/dbl-64/s_sin.c (reduce_and_compute): Same.
	(do_sincos_1, do_sincos_2): Same.
	* sysdeps/ieee754/ldbl-96/e_jnl.c (__ieee754_jnl): Same.
	(__ieee754_ynl): Same.

diff --git a/Makeconfig b/Makeconfig
index 87a22e8..081ece6 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -750,7 +750,15 @@  endif
 +gccwarn += -Wundef
 ifeq ($(enable-werror),yes)
 +gccwarn += -Werror
+
+# GCC's -Wmaybe-uninitialized depends on optimization and is documented
+# to issue false positives at -O1 (or lower).  If -O1 or lower is found
+# in CFLAGS, prevent such warnings from being treated as errors.
+ifneq ($(filter-out -O -O1, $(CFLAGS)),$(CFLAGS))
+gccwarn += -Wno-error=maybe-uninitialized
+endif
 endif
+
 +gccwarn-c = -Wstrict-prototypes -Wold-style-definition
 
 # We do not depend on the address of constants in different files to be
diff --git a/sysdeps/ieee754/dbl-64/e_jn.c b/sysdeps/ieee754/dbl-64/e_jn.c
index 3fecf82..f6eceae 100644
--- a/sysdeps/ieee754/dbl-64/e_jn.c
+++ b/sysdeps/ieee754/dbl-64/e_jn.c
@@ -52,7 +52,7 @@  double
 __ieee754_jn (int n, double x)
 {
   int32_t i, hx, ix, lx, sgn;
-  double a, b, temp, di, ret;
+  double a, b, di, ret;
   double z, w;
 
   /* J(-n,x) = (-1)^n * J(n, x), J(n, -x) = (-1)^n * J(n, x)
@@ -100,6 +100,8 @@  __ieee754_jn (int n, double x)
 	    double s;
 	    double c;
 	    __sincos (x, &s, &c);
+
+	    double temp = 0;
 	    switch (n & 3)
 	      {
 	      case 0: temp = c + s; break;
@@ -115,7 +117,7 @@  __ieee754_jn (int n, double x)
 	    b = __ieee754_j1 (x);
 	    for (i = 1; i < n; i++)
 	      {
-		temp = b;
+		double temp = b;
 		b = b * ((double) (i + i) / x) - a; /* avoid underflow */
 		a = temp;
 	      }
@@ -131,7 +133,8 @@  __ieee754_jn (int n, double x)
 	      b = zero;
 	    else
 	      {
-		temp = x * 0.5; b = temp;
+		double temp = x * 0.5;
+                b = temp;
 		for (a = one, i = 2; i <= n; i++)
 		  {
 		    a *= (double) i;              /* a = n! */
@@ -202,7 +205,7 @@  __ieee754_jn (int n, double x)
 	      {
 		for (i = n - 1, di = (double) (i + i); i > 0; i--)
 		  {
-		    temp = b;
+		    double temp = b;
 		    b *= di;
 		    b = b / x - a;
 		    a = temp;
@@ -213,7 +216,7 @@  __ieee754_jn (int n, double x)
 	      {
 		for (i = n - 1, di = (double) (i + i); i > 0; i--)
 		  {
-		    temp = b;
+		    double temp = b;
 		    b *= di;
 		    b = b / x - a;
 		    a = temp;
@@ -261,7 +264,7 @@  __ieee754_yn (int n, double x)
 {
   int32_t i, hx, ix, lx;
   int32_t sign;
-  double a, b, temp, ret;
+  double a, b, ret;
 
   EXTRACT_WORDS (hx, lx, x);
   ix = 0x7fffffff & hx;
@@ -307,6 +310,8 @@  __ieee754_yn (int n, double x)
 	double c;
 	double s;
 	__sincos (x, &s, &c);
+
+        double temp = 0;
 	switch (n & 3)
 	  {
 	  case 0: temp = s - c; break;
@@ -325,7 +330,7 @@  __ieee754_yn (int n, double x)
 	GET_HIGH_WORD (high, b);
 	for (i = 1; i < n && high != 0xfff00000; i++)
 	  {
-	    temp = b;
+	    double temp = b;
 	    b = ((double) (i + i) / x) * b - a;
 	    GET_HIGH_WORD (high, b);
 	    a = temp;
diff --git a/sysdeps/ieee754/dbl-64/s_sin.c b/sysdeps/ieee754/dbl-64/s_sin.c
index ca2532f..3cd3e83 100644
--- a/sysdeps/ieee754/dbl-64/s_sin.c
+++ b/sysdeps/ieee754/dbl-64/s_sin.c
@@ -244,8 +244,11 @@  static inline double
 __always_inline
 reduce_and_compute (double x, unsigned int k)
 {
-  double retval = 0, a, da;
+  double a, da;
   unsigned int n = __branred (x, &a, &da);
+
+  double retval = 0;
+
   k = (n + k) % 4;
   switch (k)
     {
@@ -297,11 +300,13 @@  static double
 __always_inline
 do_sincos_1 (double a, double da, double x, int4 n, int4 k)
 {
-  double xx, retval, res, cor, y;
+  double xx, res, cor, y;
   mynumber u;
   int m;
   double eps = fabs (x) * 1.2e-30;
 
+  double retval = 0;
+
   int k1 = (n + k) & 3;
   switch (k1)
     {			/* quarter of unit circle */
@@ -387,11 +392,13 @@  static double
 __always_inline
 do_sincos_2 (double a, double da, double x, int4 n, int4 k)
 {
-  double res, retval, cor, xx;
+  double res, cor, xx;
   mynumber u;
 
   double eps = 1.0e-24;
 
+  double retval = 0;
+
   k = (n + k) & 3;
 
   switch (k)
diff --git a/sysdeps/ieee754/ldbl-96/e_jnl.c b/sysdeps/ieee754/ldbl-96/e_jnl.c
index 92f9692..66d467b 100644
--- a/sysdeps/ieee754/ldbl-96/e_jnl.c
+++ b/sysdeps/ieee754/ldbl-96/e_jnl.c
@@ -71,7 +71,7 @@  __ieee754_jnl (int n, long double x)
 {
   u_int32_t se, i0, i1;
   int32_t i, ix, sgn;
-  long double a, b, temp, di, ret;
+  long double a, b, di, ret;
   long double z, w;
 
   /* J(-n,x) = (-1)^n * J(n, x), J(n, -x) = (-1)^n * J(n, x)
@@ -126,6 +126,7 @@  __ieee754_jnl (int n, long double x)
 	     */
 	    long double s;
 	    long double c;
+            long double temp = 0;
 	    __sincosl (x, &s, &c);
 	    switch (n & 3)
 	      {
@@ -150,7 +151,7 @@  __ieee754_jnl (int n, long double x)
 	    b = __ieee754_j1l (x);
 	    for (i = 1; i < n; i++)
 	      {
-		temp = b;
+		long double temp = b;
 		b = b * ((long double) (i + i) / x) - a;	/* avoid underflow */
 		a = temp;
 	      }
@@ -167,7 +168,7 @@  __ieee754_jnl (int n, long double x)
 	      b = zero;
 	    else
 	      {
-		temp = x * 0.5;
+		long double temp = x * 0.5;
 		b = temp;
 		for (a = one, i = 2; i <= n; i++)
 		  {
@@ -246,7 +247,7 @@  __ieee754_jnl (int n, long double x)
 	      {
 		for (i = n - 1, di = (long double) (i + i); i > 0; i--)
 		  {
-		    temp = b;
+		    long double temp = b;
 		    b *= di;
 		    b = b / x - a;
 		    a = temp;
@@ -257,7 +258,7 @@  __ieee754_jnl (int n, long double x)
 	      {
 		for (i = n - 1, di = (long double) (i + i); i > 0; i--)
 		  {
-		    temp = b;
+		    long double temp = b;
 		    b *= di;
 		    b = b / x - a;
 		    a = temp;
@@ -305,7 +306,7 @@  __ieee754_ynl (int n, long double x)
   u_int32_t se, i0, i1;
   int32_t i, ix;
   int32_t sign;
-  long double a, b, temp, ret;
+  long double a, b, ret;
 
 
   GET_LDOUBLE_WORDS (se, i0, i1, x);
@@ -355,6 +356,7 @@  __ieee754_ynl (int n, long double x)
 	 */
 	long double s;
 	long double c;
+	long double temp = 0;
 	__sincosl (x, &s, &c);
 	switch (n & 3)
 	  {
@@ -382,7 +384,7 @@  __ieee754_ynl (int n, long double x)
 	/* Use 0xffffffff since GET_LDOUBLE_WORDS sign-extends SE.  */
 	for (i = 1; i < n && se != 0xffffffff; i++)
 	  {
-	    temp = b;
+	    long double temp = b;
 	    b = ((long double) (i + i) / x) * b - a;
 	    GET_LDOUBLE_WORDS (se, i0, i1, b);
 	    a = temp;