* [PATCH] Fix i486 builds with gcc 5.3
@ 2016-10-27 20:25 Carlos O'Donell
2016-10-27 20:30 ` Joseph Myers
0 siblings, 1 reply; 3+ messages in thread
From: Carlos O'Donell @ 2016-10-27 20:25 UTC (permalink / raw)
To: GNU C Library
While investigating bug 20729 I found that the i486 builds fail with
gcc 5.3 with two sets of warnings like the following, one for normal
characters and another for wide characters.
The exact warnings look like this:
~~~
In file included from ../string/strcoll_l.c:43:0,
from wcscoll_l.c:32:
../string/strcoll_l.c: In function â__wcscoll_lâ:
../string/../locale/weightwc.h:63:25: error: âseq2.back_usâ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (cp[cnt] != usrc[cnt])
^
In file included from wcscoll_l.c:32:0:
../string/strcoll_l.c:295:18: note: âseq2.back_usâ was declared here
coll_seq seq1, seq2;
^
In file included from ../string/strcoll_l.c:43:0,
from wcscoll_l.c:32:
../string/../locale/weightwc.h:63:25: error: âseq1.back_usâ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (cp[cnt] != usrc[cnt])
^
In file included from wcscoll_l.c:32:0:
../string/strcoll_l.c:295:12: note: âseq1.back_usâ was declared here
coll_seq seq1, seq2;
^
cc1: all warnings being treated as errors
~~~
strcoll_l.c: In function â__strcoll_lâ:
strcoll_l.c:174:24: error: âseq2.save_idxâ may be used uninitialized in this function [-Werror=maybe-uninitialized]
len = weights[idx++];
^
strcoll_l.c:283:18: note: âseq2.save_idxâ was declared here
coll_seq seq1, seq2;
^
strcoll_l.c:174:20: error: âseq1.save_idxâ may be used uninitialized in this function [-Werror=maybe-uninitialized]
len = weights[idx++];
^
strcoll_l.c:283:12: note: âseq1.save_idxâ was declared here
coll_seq seq1, seq2;
^
cc1: all warnings being treated as errors
~~~
All four warnings are false positives, we have guards or other variables
which would prevent the uninitialized value from being used.
I am purposely _not_ initializing the values to zero because every such
initialization will slow down strcoll and we care about the performance
more than we care about compiler warnings.
I have specifically chosen to narrow down the warning suppression to
gcc 5.3 because I'm not sure exactly how wide-spread the warnings is and
I'd rather be conservative. Others who run into the same error can expand
the case to gcc 5 if we see a pattern of failures.
If nobody objects I'll check this in within 48 hours.
2016-10-27 Carlos O'Donell <carlos@redhat.com>
* locale/weight.h (findix): Ignore -Wmaybe-uninitialized error
for seq2.back_us and seq1.back_us.
* locale/weightwc.h (findix): Likewise.
* string/strcoll_l.c (get_next_seq): Ignore -Wmaybe-uninitialized
for seq2.save_idx and seq1.save_idx.
diff --git a/locale/weight.h b/locale/weight.h
index c99730c..8cab4df 100644
--- a/locale/weight.h
+++ b/locale/weight.h
@@ -61,9 +61,17 @@ findidx (const int32_t *table,
already. */
size_t cnt;
+ /* In GCC 5.3 compiling for i486 the compiler complains that
+ seq2.back_us, which becomes usrc, might be used
+ uninitialized. This can't be true because we pass a length
+ of -1 for len at the same time which means that this loop
+ never executes. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT (5.3, "-Wmaybe-uninitialized");
for (cnt = 0; cnt < nhere && cnt < len; ++cnt)
if (cp[cnt] != usrc[cnt])
break;
+ DIAG_POP_NEEDS_COMMENT;
if (cnt == nhere)
{
diff --git a/locale/weightwc.h b/locale/weightwc.h
index ab26482..982ce69 100644
--- a/locale/weightwc.h
+++ b/locale/weightwc.h
@@ -59,9 +59,17 @@ findidx (const int32_t *table,
already. */
size_t cnt;
+ /* In GCC 5.3 compiling for i486 the compiler complains that
+ seq2.back_us, which becomes usrc, might be used
+ uninitialized. This can't be true because we pass a length
+ of -1 for len at the same time which means that this loop
+ never executes. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT (5.3, "-Wmaybe-uninitialized");
for (cnt = 0; cnt < nhere && cnt < len; ++cnt)
if (cp[cnt] != usrc[cnt])
break;
+ DIAG_POP_NEEDS_COMMENT;
if (cnt == nhere)
{
diff --git a/string/strcoll_l.c b/string/strcoll_l.c
index 4d1e3ab..fd61497 100644
--- a/string/strcoll_l.c
+++ b/string/strcoll_l.c
@@ -24,6 +24,7 @@
#include <stdint.h>
#include <string.h>
#include <sys/param.h>
+#include <libc-internal.h>
#ifndef STRING_TYPE
# define STRING_TYPE char
@@ -170,7 +171,19 @@ get_next_seq (coll_seq *seq, int nrules, const unsigned char *rulesets,
}
}
+ /* In GCC 5.3 compiling for i486 the compiler complains that idx,
+ taken from seq->idx (seq1 or seq2 from STRCOLL) may be used
+ uninitialized. In general this can't possibly be true since
+ seq1.idx and seq2.idx are initialized to zero in the outer
+ function. Only one case where seq->idx is restored from
+ seq->save_idx might result in an uninitialized idx value, but
+ it is guarded by a sequence of checks against backw_stop which
+ ensures that seq->save_idx was saved to first and contains a
+ valid value. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT (5.3, "-Wmaybe-uninitialized");
len = weights[idx++];
+ DIAG_POP_NEEDS_COMMENT;
/* Skip over indices of previous levels. */
for (int i = 0; i < pass; i++)
{
---
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix i486 builds with gcc 5.3
2016-10-27 20:25 [PATCH] Fix i486 builds with gcc 5.3 Carlos O'Donell
@ 2016-10-27 20:30 ` Joseph Myers
2016-10-28 2:23 ` Carlos O'Donell
0 siblings, 1 reply; 3+ messages in thread
From: Joseph Myers @ 2016-10-27 20:30 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: GNU C Library
On Thu, 27 Oct 2016, Carlos O'Donell wrote:
> I have specifically chosen to narrow down the warning suppression to
> gcc 5.3 because I'm not sure exactly how wide-spread the warnings is and
> I'd rather be conservative. Others who run into the same error can expand
> the case to gcc 5 if we see a pattern of failures.
The version number in DIAG_IGNORE_NEEDS_COMMENT is not a restriction on
where the warning is suppressed, it's for documentation purposes only, the
most recent version known to need the suppression (so any such calls
naming a version older than the oldest now supported for building glibc
are appropriate for review to see if they are still needed).
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix i486 builds with gcc 5.3
2016-10-27 20:30 ` Joseph Myers
@ 2016-10-28 2:23 ` Carlos O'Donell
0 siblings, 0 replies; 3+ messages in thread
From: Carlos O'Donell @ 2016-10-28 2:23 UTC (permalink / raw)
To: Joseph Myers; +Cc: GNU C Library
On 10/27/2016 04:30 PM, Joseph Myers wrote:
> On Thu, 27 Oct 2016, Carlos O'Donell wrote:
>
>> I have specifically chosen to narrow down the warning suppression to
>> gcc 5.3 because I'm not sure exactly how wide-spread the warnings is and
>> I'd rather be conservative. Others who run into the same error can expand
>> the case to gcc 5 if we see a pattern of failures.
>
> The version number in DIAG_IGNORE_NEEDS_COMMENT is not a restriction on
> where the warning is suppressed, it's for documentation purposes only, the
> most recent version known to need the suppression (so any such calls
> naming a version older than the oldest now supported for building glibc
> are appropriate for review to see if they are still needed).
Thanks. I mis-remembered that we did something with that number.
I'm going to drop this patch in favour of another one that actually fixes
all the issues compiling with -Os (unrelated to i486).
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-28 2:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 20:25 [PATCH] Fix i486 builds with gcc 5.3 Carlos O'Donell
2016-10-27 20:30 ` Joseph Myers
2016-10-28 2:23 ` Carlos O'Donell
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).