From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19668 invoked by alias); 28 Oct 2016 13:58:26 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 19644 invoked by uid 89); 28 Oct 2016 13:58:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.8 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 spammy=indicator, assured, sk:df5b5df, sk:ucs4_to X-HELO: mail-qk0-f182.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-transfer-encoding; bh=lYvJ8lQ3qwsJgdoZxvAWSrACOGNTmOpDxSSNtW1wBNA=; b=eoAmu6shQzNJC/jjLJgssaxbIRKjbUSoiqYd9SkoDpw8aMU/Rnj6xan/ksGVbNUzll z05iCu4Acn5eP34b6k3WlaFtBhhrX/n0nhzvDcdqOI8tKz8dvvH/fQPuaWhGsuKJnVH2 SlgvVHW14RG3GrYfIon9g+b86YJzgP+I4o/uz2FJQkvJaxRndNkEdqL2rJ72iDC4uKxm pRetf+ChXI9Kv7MkK7Xq+rr8Vd7yCMQvF4fFSnpRPipAWwDE8qXPlux8IxzyzDKZGSfH 1492FFhO27xFy8a+a1WlJKjVLHttTRLpXQ54Q8c4YOuWaQsIBsPxOHNgBjYxiNzrntM0 9YZw== X-Gm-Message-State: ABUngvceNpxIUsBTQxTAuWvlX8A02hkDw4yVjrtvHyeOTpvxHEs8J7iVsjheFUmg13cpGNhH X-Received: by 10.55.180.132 with SMTP id d126mr10445719qkf.19.1477663101347; Fri, 28 Oct 2016 06:58:21 -0700 (PDT) Subject: [PATCH v2] Fix -Os related build and test failures. To: Florian Weimer , Joseph Myers References: <6eac682f-26fa-6a47-9497-357206266ba1@redhat.com> <6be7dce5-bfa7-32c7-5bac-6c3b79776683@redhat.com> <38a493b5-e73f-1bf8-46f0-4121e547a05d@redhat.com> <2ff34c0c-7571-4198-890a-2b30dd7d2920@redhat.com> Cc: GNU C Library From: Carlos O'Donell Message-ID: <5379c2f2-74e6-2550-8d42-2d41d1f6478d@redhat.com> Date: Fri, 28 Oct 2016 13:58:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <2ff34c0c-7571-4198-890a-2b30dd7d2920@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00537.txt.bz2 On 10/28/2016 09:18 AM, Carlos O'Donell wrote: > I would rather follow Joseph's suggestion of adding optimization > specific DIAG_* macros. > > e.g. > DIAG_IGNORE_O3_NEEDS_COMMENT > DIAG_IGNORE_O2_NEEDS_COMMENT > DIAG_IGNORE_O1_NEEDS_COMMENT > DIAG_IGNORE_Os_NEEDS_COMMENT > DIAG_IGNORE_Og_NEEDS_COMMENT > > Where the diagnostic is only ignored for the relevant optimization > mode. > > This way the patch I just suggested would use the *_Os_* variants > and not interfere with -O2 builds. Since the kinds of warnings > generated are rather tightly coupled with the optimization passes > that are enabled, it makes sense to specialize them a bit. v2 - Use '5' only as the version identifier for the GCC impacted as requested by Andreas and noted in the comments for the diagnostic. - Fix math/test-nan-overflow.c which uses malloc but doesn't include stdlib.h. - Define DIAG_IGNORE_Ox_NEEDS_COMMENT and DIAG_IGNORE_Os_NEEDS_COMMENT for use with diagnostics which should be ignored only when optimizing, or when optimizing for size. If we need *Ox* to be finer grained, then we'll have to setup something more complicated, but for now we don't need it. 2016-10-27 Carlos O'Donell * include/libc-internal.h (DIAG_IGNORE_Ox_NEEDS_COMMENT): Define. (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. * math/test-nan-overflow.c: Include stdlib.h for malloc. 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 #include @@ -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..cba8c7c 100644 --- a/include/libc-internal.h +++ b/include/libc-internal.h @@ -111,4 +111,39 @@ 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 macros ignore the + diagnostic OPTION but only when: + + - optimizations are enabled (DIAG_IGNORE_Ox_NEEDS_COMMENT) or + + - optimizations for size are enabled (DIAG_IGNORE_Os_NEEDS_COMMENT). + + These are 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 in -Os, but at -O2 you would + still like the warning to be enabled to catch errors. In this case + you could use DIAG_IGNORE_Os_NEEDS_COMMENT to disable the warning + only for -Os. + + At present we don't actually distinguish between the various levels + of optimization because the compiler doesn't allow that easily, but + we do distinguish between -O[1,2,3,g] and -Os. Therefore use + DIAG_IGNORE_Ox_NEEDS_COMMENT for diagnostics that should only be + ignored when any optimizations are present (including -Os), and + DIAG_IGNORE_Os_NEEDS_COMMENT for diagnostics that should only be + ignored when space optimizations are in effect (only -Os). */ +#if __OPTIMIZE__ +#define DIAG_IGNORE_Ox_NEEDS_COMMENT(version, option) \ + _Pragma (_DIAG_STR (GCC diagnostic ignored option)) +#else +#define DIAG_IGNORE_Ox_NEEDS_COMMENT(version, option) +#endif + +#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 + #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/math/test-nan-overflow.c b/math/test-nan-overflow.c index 40aae2e..745a044 100644 --- a/math/test-nan-overflow.c +++ b/math/test-nan-overflow.c @@ -20,6 +20,7 @@ #include #include #include +#include #define STACK_LIM 1048576 #define STRING_SIZE (2 * STACK_LIM) 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..93efb7d 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.3, "-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..065392e 100644 --- a/string/strcoll_l.c +++ b/string/strcoll_l.c @@ -24,6 +24,7 @@ #include #include #include +#include #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.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.