From patchwork Fri Dec 16 02:27:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 88250 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp1149350qgi; Thu, 15 Dec 2016 18:27:42 -0800 (PST) X-Received: by 10.84.179.67 with SMTP id a61mr1502540plc.98.1481855262387; Thu, 15 Dec 2016 18:27:42 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id y70si5084140plh.264.2016.12.15.18.27.42 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Dec 2016 18:27:42 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-444575-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-444575-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-444575-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=gmail.com DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=NihFHoz+mPM6FADAN UPO28D+tOixixWpPF6U4lGDFlo0eH6ApH/3WjW15dxOJJX+N95zOdKXOOTNL2jw/ ypH8mEtQNgJi9tWuV5apzUn2XPO5H+B4kyFWkEAO2LsGrY1cdW8J8EidO0mHr8+J yPWoTSERtIG7ihZ4cuK98OgK0g= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=KEzwlSFvlw+r1Qu3ieh35yd b93Y=; b=fiXDQdt8gyf7zeC67gl+0g4V9R+b3hGlxynIJY+3eaGL0A7xFR/H5eH VK6WfR21Pfq39NcD7cZwrOI0SBI+Y8XPvb5Vo0JlMdw9mduQXq+RjADU3Te29HWV +KS0RgTgOJ7LGFt4dmjNboqZBQVEL+k5p+B4dcSlTfkY0WcRHdZU= Received: (qmail 63245 invoked by alias); 16 Dec 2016 02:27:24 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 63235 invoked by uid 89); 16 Dec 2016 02:27:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=H*MI:sk:75198a6, H*i:sk:75198a6, H*f:sk:75198a6, LENGTH X-HELO: mail-qk0-f194.google.com Received: from mail-qk0-f194.google.com (HELO mail-qk0-f194.google.com) (209.85.220.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 16 Dec 2016 02:27:21 +0000 Received: by mail-qk0-f194.google.com with SMTP id n204so9752891qke.2 for ; Thu, 15 Dec 2016 18:27:21 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to; bh=z80uqoAQM5oy8Z2u9of+9KDsXJIJeUESS2GwWJcjrXY=; b=tCTPRwBqyqr8HLIg7ZfKNWvUrtwZBpWdNQJ84PQN0CCcmh2RibhJwGIkb7InFMBG5c LLjmuw+BTbDCBJuaHPaceXNCVKQf47kvnO3190/jkiE5W96t2gCe9RJJKLpl5Nwxwq4u QkUrN41P0wCM8abjX1F32r+0QacSetr+Q0/8Rpj2wwRSXwCIfvvLpYezq0RRFAdTOQuI FfbcmAscDzgjm/vsGm5NNouUTY/ZRbjKu94xjDcc8qjNi/JyAv6kWro+yjsk9ZmKBYxn CEMAs022rXqIlUHqC9TjDfrW3PvfqRmWCXL5PqWksHhGqufW+2BRrIMlYjN+wu+OqCRc eTrA== X-Gm-Message-State: AIkVDXLDPohaOpmnvdZhcMfw33MeyiZOnIFX12JsiJWeDFOAIXwgAGWLB8MLvrym2Nj3mA== X-Received: by 10.55.162.79 with SMTP id l76mr559335qke.17.1481855239707; Thu, 15 Dec 2016 18:27:19 -0800 (PST) Received: from [192.168.0.26] (97-124-188-210.hlrn.qwest.net. [97.124.188.210]) by smtp.gmail.com with ESMTPSA id 1sm2613986qtb.49.2016.12.15.18.27.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Dec 2016 18:27:19 -0800 (PST) Subject: Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817) To: Jeff Law , Gcc Patch List References: <47244405-9a99-a24b-f2f1-6b18343ec18d@gmail.com> <75198a66-b6c5-c5d7-d516-136ea4a873c7@redhat.com> From: Martin Sebor Message-ID: Date: Thu, 15 Dec 2016 19:27:16 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <75198a66-b6c5-c5d7-d516-136ea4a873c7@redhat.com> X-IsSubscribed: yes On 12/14/2016 09:19 PM, Jeff Law wrote: > On 12/14/2016 03:56 PM, Martin Sebor wrote: >> The -Wnonnull warning improvement (PR c/17308 - nonnull attribute >> not as useful as it could be) causes a couple of false positives >> in a powerpc64le bootstrap. The attached fix suppresses them. >> It passes bootstrap on powerpc64le but tests are still running. >> >> I couldn't reproduce the bootstrap comparison failure that Segher >> mentioned in his comment on the bug. I've gone over the patch >> again to see if I could spot what could possibly be behind it >> but couldn't really see anything. Jeff, you mentioned attribute >> nonnull is exploited by the null pointer optimization. Do you >> think it could possibly have this effect in GCC? > It's certainly possible. It would be an indicator that something in GCC > is passing a NULL pointer into a routine where it shouldn't at runtime > and that GCC is using its new knowledge to optimize the code assuming > that can't happen. > > ie, it's an indicator of a latent bug in GCC. Depending on the > difficulty in tracking down, you may need to revert the change. This is > exactly the kind of scenario I was concerned about in that approval > message. I couldn't reproduce the comparison error either on powerpc64-linux or on powerpc64le-linux. > The change to the vec is OK. Can you please verify > that if you install just that change that ppc bootstraps are restored > and if so, please install. Done. A profiledbootstrap exposed a few more instances of the enhanced -Wnonnull warning. I spent some time analyzing them and decided that three of them are justified (those in gengtype-parse.c and gengtype.c) and the other three false positives. The attached patch silences them. The gengtype code alternates between checking for null pointers and assuming that the same pointers are not null (and passing nulls to nonnull functions like libiberty's lbasename). It could probably benefit from some more cleanup but I'm out of cycles for that. There are two outstanding instances of -Wnonnull in the profiled- bootstrap log that both point to the same function that I haven't figured out yet: In function ‘void check_function_format(tree, int, tree_node**)’: cc1plus: warning: argument 1 null where non-null expected [-Wnonnull] /usr/include/string.h:398:15: note: in a call to function ‘size_t strlen(const char*)’ declared here I'll deal with them next but I want to get this patch reviewed and approved so bootstrap can be restored on the targets affected by the vec.h warning. While waiting for builds to finish I also built Binutils, Busybox, and the Linux kernel to see if the warning shows up there. It does not. The following is a breakdown of warnings that do show up in those builds (including GCC's latest profiledbootstrap with the attached patch), for reference. Diagnostic Count Unique Files -Wstrict-aliasing 348 4 1 -Wimplicit-fallthrough= 144 121 2 -Wformat-length= 49 47 4 -Wunused-const-variable= 12 12 1 -Wint-in-bool-context 10 10 1 -Wmaybe-uninitialized 10 6 1 -Wdangling-else 2 2 1 -Wframe-address 2 1 1 -Wnonnull 2 1 1 -Wbool-operation 1 1 1 Martin PR bootstrap/78817 - stage2 bootstrap failure in vec.h:1613:5: error: argument 1 null where non-null expected after r243661 gcc/ChangeLog: PR bootstrap/78817 * calls.c (maybe_warn_null_arg): Add a missing '='. * gengtype-parse.c (type): Guard against dereferncing a null pointer. (get_file_realbasename): Avoid passing a null pointer to a function expecting non-null. * tree-vect-data-refs.c (vect_permute_store_chain): Assert pointer is non-null. (vect_permute_load_chain): Same. * vec.h (vec::safe_grow_cleared): Assert a pointer is non-null. diff --git a/gcc/calls.c b/gcc/calls.c index 8466427..463f99c 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1573,7 +1573,7 @@ maybe_warn_null_arg (tree fndecl, tree exp, tree arg, ++argpos; - location_t exploc EXPR_LOCATION (exp); + location_t exploc = EXPR_LOCATION (exp); if (warning_at (exploc, OPT_Wnonnull, "argument %u null where non-null expected", argpos)) diff --git a/gcc/gengtype-parse.c b/gcc/gengtype-parse.c index 954ac2a..b11da84 100644 --- a/gcc/gengtype-parse.c +++ b/gcc/gengtype-parse.c @@ -946,15 +946,18 @@ type (options_p *optsp, bool nested) inheritance specifications. We require single-inheritance from a non-template type. */ advance (); - const char *basename = require (ID); - /* This may be either an access specifier, or the base name. */ - if (0 == strcmp (basename, "public") - || 0 == strcmp (basename, "protected") - || 0 == strcmp (basename, "private")) - basename = require (ID); - base_class = find_structure (basename, TYPE_STRUCT); - if (!base_class) - parse_error ("unrecognized base class: %s", basename); + if (const char *basename = require (ID)) + { + /* This may be either an access specifier, or the base + name. */ + if (0 == strcmp (basename, "public") + || 0 == strcmp (basename, "protected") + || 0 == strcmp (basename, "private")) + basename = require (ID); + base_class = find_structure (basename, TYPE_STRUCT); + if (!base_class) + parse_error ("unrecognized base class: %s", basename); + } require_without_advance ('{'); } else diff --git a/gcc/gengtype.c b/gcc/gengtype.c index dcc2ff5..c63bd8e 100644 --- a/gcc/gengtype.c +++ b/gcc/gengtype.c @@ -1747,7 +1747,10 @@ open_base_files (void) static const char * get_file_realbasename (const input_file *inpf) { - return lbasename (get_input_file_name (inpf)); + if (const char *fname = get_input_file_name (inpf)) + return lbasename (fname); + + return NULL; } /* For INPF a filename, return the relative path to INPF from @@ -1756,12 +1759,13 @@ get_file_realbasename (const input_file *inpf) const char * get_file_srcdir_relative_path (const input_file *inpf) { - const char *f = get_input_file_name (inpf); - if (strlen (f) > srcdir_len - && IS_DIR_SEPARATOR (f[srcdir_len]) - && strncmp (f, srcdir, srcdir_len) == 0) - return f + srcdir_len + 1; - else + if (const char *f = get_input_file_name (inpf)) + { + if (strlen (f) > srcdir_len + && IS_DIR_SEPARATOR (f[srcdir_len]) + && strncmp (f, srcdir, srcdir_len) == 0) + return f + srcdir_len + 1; + } return NULL; } diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c index 1b9c3b3..967fa44 100644 --- a/gcc/tree-vect-data-refs.c +++ b/gcc/tree-vect-data-refs.c @@ -4954,7 +4954,8 @@ vect_permute_store_chain (vec dr_chain, } else { - /* If length is not equal to 3 then only power of 2 is supported. */ + /* If length is not equal to 3 then only positive powers of 2 are + supported. */ gcc_assert (pow2p_hwi (length)); for (i = 0, n = nelt / 2; i < n; i++) @@ -4994,6 +4995,11 @@ vect_permute_store_chain (vec dr_chain, vect_finish_stmt_generation (stmt, perm_stmt, gsi); (*result_chain)[2*j+1] = low; } + + /* The assert below is strictly redundant because RESULT_CHAIN + has a size equal to LENGTH which is asserted to be positive + above. Unfortunately, GCC doesn't see that. */ + gcc_assert (result_chain->address ()); memcpy (dr_chain.address (), result_chain->address (), length * sizeof (tree)); } @@ -5557,6 +5563,11 @@ vect_permute_load_chain (vec dr_chain, vect_finish_stmt_generation (stmt, perm_stmt, gsi); (*result_chain)[j/2+length/2] = data_ref; } + + /* The assert below is strictly redundant because RESULT_CHAIN + has a size equal to LENGTH which is asserted to be positive + above. Unfortunately, GCC doesn't see that. */ + gcc_assert (result_chain->address ()); memcpy (dr_chain.address (), result_chain->address (), length * sizeof (tree)); } diff --git a/gcc/vec.h b/gcc/vec.h index aa93411..b6b54ef 100644 --- a/gcc/vec.h +++ b/gcc/vec.h @@ -1607,10 +1608,16 @@ inline void vec::safe_grow_cleared (unsigned len MEM_STAT_DECL) { unsigned oldlen = length (); - size_t sz = sizeof (T) * (len - oldlen); - safe_grow (len PASS_MEM_STAT); - if (sz != 0) - memset (&(address ()[oldlen]), 0, sz); + gcc_checking_assert (oldlen <= len); + + if (size_t sz = sizeof (T) * (len - oldlen)) + { + safe_grow (len PASS_MEM_STAT); + + T *p = address (); + gcc_assert (p != NULL); + memset (p + oldlen, 0, sz); + } }