From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94138 invoked by alias); 27 Oct 2016 20:25:00 -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 94111 invoked by uid 89); 27 Oct 2016 20:25:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=suppression, 619 X-HELO: mail-qk0-f175.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:to:from:subject:organization:message-id:date :user-agent:mime-version:content-transfer-encoding; bh=rbJqGOEygeEBKp0qnkm9sMHEN2z7eQ/Qs5K9RDLXaJU=; b=CA/GOT24ZurKIf6LHd1syD+CbGlf4KwaZ8Kd9/zTWOvMk+Yu7aGHX3FT1lzKlob07n 1lmUlmP5VvSJdo0U5f3DuDv3ltx/ddn8eS1AKB7imWWWlQg+AwcrirTyoKDKgN3A4fol QtD+y+a952dJEjUwadJ7jUTdoaQV8TdVdxYJLyU2tABhdJzHZtB3e0J+Gc0b41/p5Wi5 IpIoMxpToxCKuUHjsKrlt+J2vz8GBmIysKcFBduMZWcNoVwE5QbGrR3KyVB/N0cfTlJf t1DOtt0TspdMoVFaM+eX4viYhoFtI+xN0x+KdI01/3JGvz42eYPdyvb7tgOdmHpgaV3N 6hwA== X-Gm-Message-State: ABUngvf4Ji/7eEwXnLYWMTjzKFdxhId1JwlpFqsKcKeIAIjSslkjRqUSofSZGbafSYlGgZBx X-Received: by 10.55.108.194 with SMTP id h185mr7244516qkc.37.1477599895073; Thu, 27 Oct 2016 13:24:55 -0700 (PDT) To: GNU C Library From: Carlos O'Donell Subject: [PATCH] Fix i486 builds with gcc 5.3 Message-ID: Date: Thu, 27 Oct 2016 20:25:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2016-10/txt/msg00495.txt.bz2 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 * 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 #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, } } + /* 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.