From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30767 invoked by alias); 5 Dec 2018 23:11:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 30757 invoked by uid 89); 5 Dec 2018 23:11:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=no, No, so, pm X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Dec 2018 23:11:11 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B842231256B9; Wed, 5 Dec 2018 23:11:09 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-17.rdu2.redhat.com [10.10.112.17]) by smtp.corp.redhat.com (Postfix) with ESMTP id ABF925D756; Wed, 5 Dec 2018 23:11:08 +0000 (UTC) Subject: Re: [PING #4][PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028) To: Martin Sebor , Richard Biener Cc: GCC Patches References: <88de1ee3-6ee4-d8d9-3e57-3a42474a4169@redhat.com> <27235a6d-6f2c-1f2d-d456-d4cd9b941894@redhat.com> <23ea3d13-d9c5-1b02-f01c-d2a0e11f3a10@redhat.com> <9e3f6d62-47b9-b80f-b8ac-5711628579c5@redhat.com> <09ce3b57-33a3-86ae-1308-39fd02f25228@gmail.com> <1437ae83-c0c2-e18f-0dc8-92717c2fdcfe@gmail.com> <8a346afb-382f-cac9-a2b7-7107ef678dee@redhat.com> From: Jeff Law Openpgp: preference=signencrypt Message-ID: Date: Wed, 05 Dec 2018 23:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00323.txt.bz2 On 11/29/18 4:43 PM, Martin Sebor wrote: > On 11/29/18 4:07 PM, Jeff Law wrote: >> On 11/29/18 1:34 PM, Martin Sebor wrote: >>> On 11/16/2018 02:07 AM, Richard Biener wrote: >>>> On Fri, Nov 16, 2018 at 4:12 AM Martin Sebor  wrote: >>>>> >>>>> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01818.html >>>>> >>>>> Please let me know if there is something I need to change here >>>>> to make the fix acceptable or if I should stop trying. >>>> >>>> I have one more comment about >>>> >>>> +  /* Defer warning (and folding) until the next statement in the basic >>>> +     block is reachable.  */ >>>> +  if (!gimple_bb (stmt)) >>>> +    return false; >>>> + >>>> >>>> it's not about the next statement in the basic-block being "reachable" >>>> (even w/o a CFG you can use gsi_next()) but rather that the next >>>> stmt isn't yet gimplified and thus not inserted into the gimple sequence, >>>> >>>> right? >>> >>> No, it's about the current statement not being associated with >>> a basic block yet when the warning code runs for the first time >>> (during gimplify_expr), and so gsi_next() returning null. >>> >>>> You apply this to gimple_fold_builtin_strncpy but I'd rather >>>> see us not sprinkling this over gimple-fold.c but instead do this >>>> in gimplify.c:maybe_fold_stmt, delaying folding until say lowering. >>>> >>>> See the attached (untested). >>> >>> I would also prefer this solution.  I had tested it (in response >>> to you first mentioning it back in September) and it causes quite >>> a bit of fallout in tests that look for the folding to take place >>> very early.  See the end of my reply here: >>> >>>    https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01248.html >>> >>> But I'm willing to do the test suite cleanup if you think it's >>> suitable for GCC 9.  (If you're thinking GCC 10 please let me >>> know now.) >> The fallout on existing tests is minimal.  What's more concerning is >> that it doesn't actually pass the new test from Martin's original >> submission.  We get bogus warnings. >> >> At least part of the problem is weakness in maybe_diag_stxncpy_trunc. >> It can't handle something like this: >> >> test_literal (char * d, struct S * s) >> { >>    strncpy (d, "1234567890", 3); >>    _1 = d + 3; >>    *_1 = 0; >> } >> >> >> Note the pointer arithmetic between the strncpy and storing the NUL >> terminator. > > Right.  I'm less concerned about this case because it involves > a literal that's obviously longer than the destination but it > would be nice if the suppression worked here as well in case > the literal comes from macro expansion.  It will require > another tweak. > > But the test from my patch passes with the changes to calls.c > from my patch, so that's an improvement. > > I have done the test suite cleanup in the attached patch.  It > was indeed minimal -- not sure why I saw so many failures with > my initial approach. > > I can submit a patch to handle the literal case above as > a followup unless you would prefer it done at the same time. > > Martin > > gcc-87028.diff > > PR tree-optimization/87028 - false positive -Wstringop-truncation strncpy with global variable source string > > gcc/ChangeLog: > > PR tree-optimization/87028 > * calls.c (get_attr_nonstring_decl): Avoid setting *REF to > SSA_NAME_VAR. > * gcc/gimple-low.c (lower_stmt): Delay foldin built-ins. > * gimplify (maybe_fold_stmt): Avoid folding statements that > don't belong to a basic block. > * tree.h (SSA_NAME_VAR): Update comment. > * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Simplify. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/87028 > * c-c++-common/Wstringop-truncation.c: Remove xfails. > * gcc.dg/Wstringop-truncation-5.c: New test. > * gcc.dg/strcmpopt_1.c: Adjust. > * gcc.dg/tree-ssa/pr79697.c: Same. I fixed up the ChangeLog a little and installed the patch. Thanks, jeff