From: Carlos O'Donell <carlos@redhat.com>
To: Joseph Myers <joseph@codesourcery.com>
Cc: Florian Weimer <fweimer@redhat.com>,
GNU C Library <libc-alpha@sourceware.org>
Subject: [PATCH v4] Fix -Os related build and test failures.
Date: Sun, 30 Oct 2016 03:51:00 -0000 [thread overview]
Message-ID: <428b3741-9228-68f3-76ec-d042e4075ded@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1610291734390.11193@digraph.polyomino.org.uk>
On 10/29/2016 01:35 PM, Joseph Myers wrote:
> On Sat, 29 Oct 2016, Carlos O'Donell wrote:
>
>> +#if __OPTIMIZE_SIZE__
>> +#define DIAG_IGNORE_Os_NEEDS_COMMENT(version, option) \
>> + _Pragma (_DIAG_STR (GCC diagnostic ignored option))
>> +#else
>> +#define DIAG_IGNORE_Os_NEEDS_COMMENT(version, option)
>> +#endif
>
> That should have "# define" preprocessor indentation inside #if.
Fixed.
I've committed the following after testing on i486 -Os and
x86_64 -O2.
v4
- Correct preprocessor indentation.
2016-10-28 Carlos O'Donell <carlos@redhat.com>
[BZ #20729]
* include/libc-internal.h (DIAG_IGNORE_Os_NEEDS_COMMENT):
Define.
* iso-2022-cn-ext.c: Include libc-internal.h and ignore
-Wmaybe-uninitialized for BODY macro only for -Os compiles.
* locale/weight.h (findix): Ignore -Wmaybe-uninitialized error
for seq2.back_us and seq1.back_us only for -Os compiles.
* locale/weightwc.h (findix): Likewise.
* nptl_db/thread_dbP.h: Ignore -Wmaybe-uninitialized error for
DB_GET_FIELD_ADDRESS only for -Os compiles.
* resolv/res_send (reopen): Ignore -Wmaybe-uninitialized error
for slen only for -Os compiles.
* string/strcoll_l.c (get_next_seq): Ignore
-Wmaybe-uninitialized for seq2.save_idx and seq1.save_idx only
for -Os compiles.
diff --git a/iconvdata/iso-2022-cn-ext.c b/iconvdata/iso-2022-cn-ext.c
index df5b5df..92970a0 100644
--- a/iconvdata/iso-2022-cn-ext.c
+++ b/iconvdata/iso-2022-cn-ext.c
@@ -27,6 +27,7 @@
#include "cns11643.h"
#include "cns11643l1.h"
#include "cns11643l2.h"
+#include <libc-internal.h>
#include <assert.h>
@@ -394,6 +395,16 @@ enum
#define MIN_NEEDED_OUTPUT TO_LOOP_MIN_NEEDED_TO
#define MAX_NEEDED_OUTPUT TO_LOOP_MAX_NEEDED_TO
#define LOOPFCT TO_LOOP
+/* With GCC 5.3 when compiling with -Os the compiler emits a warning
+ that buf[0] and buf[1] may be used uninitialized. This can only
+ happen in the case where tmpbuf[3] is used, and in that case the
+ write to the tmpbuf[1] and tmpbuf[2] was assured because
+ ucs4_to_cns11643 would have filled in those entries. The difficulty
+ is in getting the compiler to see this logic because tmpbuf[0] is
+ involved in determining the code page and is the indicator that
+ tmpbuf[2] is initialized. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_Os_NEEDS_COMMENT (5, "-Wmaybe-uninitialized");
#define BODY \
{ \
uint32_t ch; \
@@ -645,6 +656,7 @@ enum
/* Now that we wrote the output increment the input pointer. */ \
inptr += 4; \
}
+DIAG_POP_NEEDS_COMMENT;
#define EXTRA_LOOP_DECLS , int *setp
#define INIT_PARAMS int set = (*setp >> 3) & CURRENT_MASK; \
int ann = (*setp >> 3) & ~CURRENT_MASK
diff --git a/include/libc-internal.h b/include/libc-internal.h
index 7a185bb..1a386e7 100644
--- a/include/libc-internal.h
+++ b/include/libc-internal.h
@@ -111,4 +111,19 @@ extern __typeof (__profile_frequency) __profile_frequency attribute_hidden;
#define DIAG_IGNORE_NEEDS_COMMENT(version, option) \
_Pragma (_DIAG_STR (GCC diagnostic ignored option))
+/* Similar to DIAG_IGNORE_NEEDS_COMMENT the following macro ignores the
+ diagnostic OPTION but only if optimizations for size are enabled.
+ This is required because different warnings may be generated for
+ different optimization levels. For example a key piece of code may
+ only generate a warning when compiled at -Os, but at -O2 you could
+ still want the warning to be enabled to catch errors. In this case
+ you would use DIAG_IGNORE_Os_NEEDS_COMMENT to disable the warning
+ only for -Os. */
+#ifdef __OPTIMIZE_SIZE__
+# define DIAG_IGNORE_Os_NEEDS_COMMENT(version, option) \
+ _Pragma (_DIAG_STR (GCC diagnostic ignored option))
+#else
+# define DIAG_IGNORE_Os_NEEDS_COMMENT(version, option)
+#endif
+
#endif /* _LIBC_INTERNAL */
diff --git a/locale/weight.h b/locale/weight.h
index c99730c..1f61f01 100644
--- a/locale/weight.h
+++ b/locale/weight.h
@@ -61,9 +61,17 @@ findidx (const int32_t *table,
already. */
size_t cnt;
+ /* With GCC 5.3 when compiling with -Os the compiler warns
+ 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_Os_NEEDS_COMMENT (5, "-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..e42ce13 100644
--- a/locale/weightwc.h
+++ b/locale/weightwc.h
@@ -59,9 +59,17 @@ findidx (const int32_t *table,
already. */
size_t cnt;
+ /* With GCC 5.3 when compiling with -Os the compiler warns
+ 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_Os_NEEDS_COMMENT (5, "-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/nptl_db/thread_dbP.h b/nptl_db/thread_dbP.h
index 39c3061..b53f1c1 100644
--- a/nptl_db/thread_dbP.h
+++ b/nptl_db/thread_dbP.h
@@ -160,10 +160,19 @@ extern ps_err_e td_mod_lookup (struct ps_prochandle *ps, const char *modname,
SYM_##type##_FIELD_##field, \
(psaddr_t) 0 + (idx), (ptr), &(var))
+/* With GCC 5.3 when compiling with -Os the compiler emits a warning
+ that slot may be used uninitialized. This is never the case since
+ the dynamic loader initializes the slotinfo list and
+ dtv_slotinfo_list will point slot at the first entry. Therefore
+ when DB_GET_FIELD_ADDRESS is called with a slot for ptr, the slot is
+ always initialized. */
+DIAG_PUSH_NEEDS_COMMENT;
+DIAG_IGNORE_Os_NEEDS_COMMENT (5, "-Wmaybe-uninitialized");
#define DB_GET_FIELD_ADDRESS(var, ta, ptr, type, field, idx) \
((var) = (ptr), _td_locate_field ((ta), (ta)->ta_field_##type##_##field, \
SYM_##type##_FIELD_##field, \
(psaddr_t) 0 + (idx), &(var)))
+DIAG_POP_NEEDS_COMMENT;
extern td_err_e _td_locate_field (td_thragent_t *ta,
db_desc_t desc, int descriptor_name,
diff --git a/resolv/res_send.c b/resolv/res_send.c
index 6d46bb2..4ec8c1a 100644
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -664,7 +664,7 @@ send_vc(res_state statp,
a false-positive.
*/
DIAG_PUSH_NEEDS_COMMENT;
- DIAG_IGNORE_NEEDS_COMMENT (5, "-Wmaybe-uninitialized");
+ DIAG_IGNORE_Os_NEEDS_COMMENT (5, "-Wmaybe-uninitialized");
int resplen;
DIAG_POP_NEEDS_COMMENT;
struct iovec iov[4];
@@ -930,7 +930,16 @@ reopen (res_state statp, int *terrno, int ns)
* error message is received. We can thus detect
* the absence of a nameserver without timing out.
*/
+ /* With GCC 5.3 when compiling with -Os the compiler
+ emits a warning that slen may be used uninitialized,
+ but that is never true. Both slen and
+ EXT(statp).nssocks[ns] are initialized together or
+ the function return -1 before control flow reaches
+ the call to connect with slen. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT (5, "-Wmaybe-uninitialized");
if (connect(EXT(statp).nssocks[ns], nsap, slen) < 0) {
+ DIAG_POP_NEEDS_COMMENT;
Aerror(statp, stderr, "connect(dg)", errno, nsap);
__res_iclose(statp, false);
return (0);
diff --git a/string/strcoll_l.c b/string/strcoll_l.c
index 4d1e3ab..5605dbf 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,
}
}
+ /* With GCC 5.3 when compiling with -Os 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_Os_NEEDS_COMMENT (5, "-Wmaybe-uninitialized");
len = weights[idx++];
+ DIAG_POP_NEEDS_COMMENT;
/* Skip over indices of previous levels. */
for (int i = 0; i < pass; i++)
{
---
--
Cheers,
Carlos.
next prev parent reply other threads:[~2016-10-30 3:51 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-28 4:48 [PATCH] Fix -Os related -Werror failures Carlos O'Donell
2016-10-28 6:25 ` Andreas Schwab
2016-10-28 6:32 ` Florian Weimer
2016-10-28 6:44 ` Jeff Law
2016-10-28 8:12 ` Arnd Bergmann
2016-10-28 8:17 ` Andrew Pinski
2016-10-28 13:28 ` Jeff Law
2016-10-28 20:10 ` Paul Eggert
2016-10-29 3:03 ` Jeff Law
2016-10-30 4:25 ` Paul Eggert
2016-10-28 12:09 ` Carlos O'Donell
2016-10-28 12:43 ` Florian Weimer
2016-10-28 13:04 ` Joseph Myers
2016-10-28 13:07 ` Carlos O'Donell
2016-10-28 12:49 ` Joseph Myers
2016-10-28 12:55 ` Florian Weimer
2016-10-28 13:18 ` Carlos O'Donell
2016-10-28 13:58 ` [PATCH v2] Fix -Os related build and test failures Carlos O'Donell
2016-10-28 14:17 ` Joseph Myers
2016-10-29 2:59 ` [PATCH v3] " Carlos O'Donell
2016-10-29 3:26 ` Carlos O'Donell
2016-10-29 17:35 ` Joseph Myers
2016-10-30 3:51 ` Carlos O'Donell [this message]
2016-10-31 8:33 ` [PATCH v4] " Andreas Schwab
2016-10-31 9:16 ` Carlos O'Donell
2016-10-31 9:22 ` Florian Weimer
2016-10-31 12:56 ` David Miller
2016-10-31 19:56 ` Carlos O'Donell
2016-11-01 22:59 ` Joseph Myers
2016-11-02 12:52 ` Carlos O'Donell
2016-11-01 9:17 ` Andreas Schwab
2016-11-01 11:13 ` Joseph Myers
2016-11-01 15:58 ` Tamar Christina
2016-11-01 16:06 ` David Miller
2016-11-01 16:15 ` Tamar Christina
2016-11-02 11:53 ` Carlos O'Donell
2016-11-02 17:03 ` Carlos O'Donell
2016-11-02 13:22 ` Carlos O'Donell
2016-10-31 18:38 ` [PATCH v3] " Steve Ellcey
2016-10-31 19:50 ` Carlos O'Donell
2016-10-31 19:57 ` Steve Ellcey
2016-10-31 20:50 ` Carlos O'Donell
2016-10-31 21:00 ` Steve Ellcey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=428b3741-9228-68f3-76ec-d042e4075ded@redhat.com \
--to=carlos@redhat.com \
--cc=fweimer@redhat.com \
--cc=joseph@codesourcery.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).