From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94926 invoked by alias); 27 Aug 2018 15:32:10 -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 94467 invoked by uid 89); 27 Aug 2018 15:32:09 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 27 Aug 2018 15:32:04 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B558F40241C8; Mon, 27 Aug 2018 15:32:02 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-8.rdu2.redhat.com [10.10.112.8]) by smtp.corp.redhat.com (Postfix) with ESMTP id D8DDB63A68; Mon, 27 Aug 2018 15:32:01 +0000 (UTC) Subject: Re: [PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028) To: Richard Biener Cc: Martin Sebor , GCC Patches References: <88de1ee3-6ee4-d8d9-3e57-3a42474a4169@redhat.com> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <27235a6d-6f2c-1f2d-d456-d4cd9b941894@redhat.com> Date: Mon, 27 Aug 2018 15:32:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg01690.txt.bz2 On 08/27/2018 02:29 AM, Richard Biener wrote: > On Sun, Aug 26, 2018 at 7:26 AM Jeff Law wrote: >> >> On 08/24/2018 09:58 AM, Martin Sebor wrote: >>> The warning suppression for -Wstringop-truncation looks for >>> the next statement after a truncating strncpy to see if it >>> adds a terminating nul. This only works when the next >>> statement can be reached using the Gimple statement iterator >>> which isn't until after gimplification. As a result, strncpy >>> calls that truncate their constant argument that are being >>> folded to memcpy this early get diagnosed even if they are >>> followed by the nul assignment: >>> >>> const char s[] = "12345"; >>> char d[3]; >>> >>> void f (void) >>> { >>> strncpy (d, s, sizeof d - 1); // -Wstringop-truncation >>> d[sizeof d - 1] = 0; >>> } >>> >>> To avoid the warning I propose to defer folding strncpy to >>> memcpy until the pointer to the basic block the strnpy call >>> is in can be used to try to reach the next statement (this >>> happens as early as ccp1). I'm aware of the preference to >>> fold things early but in the case of strncpy (a relatively >>> rarely used function that is often misused), getting >>> the warning right while folding a bit later but still fairly >>> early on seems like a reasonable compromise. I fear that >>> otherwise, the false positives will drive users to adopt >>> other unsafe solutions (like memcpy) where these kinds of >>> bugs cannot be as readily detected. >>> >>> Tested on x86_64-linux. >>> >>> Martin >>> >>> PS There still are outstanding cases where the warning can >>> be avoided. I xfailed them in the test for now but will >>> still try to get them to work for GCC 9. >>> >>> gcc-87028.diff >>> >>> >>> PR tree-optimization/87028 - false positive -Wstringop-truncation strncpy with global variable source string >>> gcc/ChangeLog: >>> >>> PR tree-optimization/87028 >>> * gimple-fold.c (gimple_fold_builtin_strncpy): Avoid folding when >>> statement doesn't belong to a basic block. >>> * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Handle MEM_REF on >>> the left hand side of assignment. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR tree-optimization/87028 >>> * c-c++-common/Wstringop-truncation.c: Remove xfails. >>> * gcc.dg/Wstringop-truncation-5.c: New test. >>> >>> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c >>> index 07341eb..284c2fb 100644 >>> --- a/gcc/gimple-fold.c >>> +++ b/gcc/gimple-fold.c >>> @@ -1702,6 +1702,11 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, >>> if (tree_int_cst_lt (ssize, len)) >>> return false; >>> >>> + /* Defer warning (and folding) until the next statement in the basic >>> + block is reachable. */ >>> + if (!gimple_bb (stmt)) >>> + return false; >> I think you want cfun->cfg as the test here. They should be equivalent >> in practice. > > Please do not add 'cfun' references. Note that the next stmt is also accessible > when there is no CFG. I guess the issue is that we fold this during > gimplification where the next stmt is not yet "there" (but still in GENERIC)? That was my assumption. I almost suggested peeking at gsi_next and avoiding in that case. > > We generally do not want to have unfolded stmts in the IL when we can avoid that > which is why we fold most stmts during gimplification. We also do that because > we now do less folding on GENERIC. But an unfolded call in the IL should always be safe and we've got plenty of opportunities to fold it later. Jeff