From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 112339 invoked by alias); 25 Feb 2018 00:11:30 -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 112328 invoked by uid 89); 25 Feb 2018 00:11:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-ot0-f182.google.com Received: from mail-ot0-f182.google.com (HELO mail-ot0-f182.google.com) (74.125.82.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 25 Feb 2018 00:11:27 +0000 Received: by mail-ot0-f182.google.com with SMTP id l5so3173741otf.9 for ; Sat, 24 Feb 2018 16:11:27 -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=kAIwP3w4u8V+OJ2fsz+tSQd8cTze8gu6YSLqbDh+5tY=; b=uDPE1/kFBPGBqa3VirJ/aItmf6UzTy+HmHwSsWUAoaj7XaS2sPNfd1IH/L7paXixbl TRMvq2wUXR2Un5Rwv8nbsK/Lblqu6z6A80woRuHIsKeYIWEvfa+uHd/SuZ9rDkDksGqm WHFVWhOpVh9vxND4QRGCqt3iaT8qMFMl+uhnAw28Il0ZdazyoM4+aIKNhJw/04nEPeNz MdD/3ihmcQG+PL5l3pXzVfx+6jgQwyKReD/xjTqYg+4jsn/89mUmd13OsyodBQSUNIH4 ndqe1wcIxeaaY7FQQowOV/aIzCcR/uH3XzcFzI1O2UJotnb62k6W4TQC8816/WS9Me5c Q91g== X-Gm-Message-State: APf1xPBQwTdboU+KkMLaXGKFzr4OkmPCmn33woQuCTHaxnVUP4f+s2c5 2d2yBpqArBN5hFSPtsl9HR+laA== X-Google-Smtp-Source: AG47ELtUCkJX2Q00rz1DAtN5A9jfVATZrF3u+n0ZMTrnhHV2i6Rx56fzZGQmz0T7sd06i+1siQBt8Q== X-Received: by 10.157.71.25 with SMTP id a25mr4606091otf.42.1519517484894; Sat, 24 Feb 2018 16:11:24 -0800 (PST) Received: from localhost.localdomain (75-171-228-29.hlrn.qwest.net. [75.171.228.29]) by smtp.gmail.com with ESMTPSA id u23sm2594596oiv.15.2018.02.24.16.11.22 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 24 Feb 2018 16:11:23 -0800 (PST) Subject: [PING] [PATCH] consider successor blocks when avoiding -Wstringop-truncation (PR 84468) To: Gcc Patch List References: From: Martin Sebor Message-ID: Date: Sun, 25 Feb 2018 00:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------6A62F8194C45FE6ACFEF65AA" X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg01374.txt.bz2 This is a multi-part message in MIME format. --------------6A62F8194C45FE6ACFEF65AA Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 979 Attached is an updated patch with a fix for a bad assumption exposed by building the linux kernel. On 02/19/2018 07:50 PM, Martin Sebor wrote: > PR 84468 points out a false positive in -Wstringop-truncation > in code like this: > > struct A { char a[4]; }; > > void f (struct A *p, const struct A *q) > { > if (p->a) > strncpy (p->a, q->a, sizeof p->a - 1); // warning here > > p->a[3] = '\0'; > } > > The warning is due to the code checking only the same basic block > as the one with the strncpy call for an assignment to the destination > to avoid it, but failing to check the successor basic block if there > is no subsequent statement in the current block. (Eliminating > the conditional is being tracked in PR 21474.) > > The attached test case adds logic to avoid this false positive. > I don't know under what circumstances there could be more than > one successor block here so I don't handle that case. > > Tested on x86_64-linux. > > Martin --------------6A62F8194C45FE6ACFEF65AA Content-Type: text/x-patch; name="gcc-84468.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-84468.diff" Content-length: 5380 PR tree-optimization/84468 - bogus -Wstringop-truncation despite assignment after conditional strncpy gcc/testsuite/ChangeLog: PR tree-optimization/84468 * gcc.dg/Wstringop-truncation.c: New test. gcc/ChangeLog: PR tree-optimization/84468 * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Also consider successor basic blocks. Index: gcc/tree-ssa-strlen.c =================================================================== --- gcc/tree-ssa-strlen.c (revision 257963) +++ gcc/tree-ssa-strlen.c (working copy) @@ -1856,8 +1856,20 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi avoid the truncation warning. */ gsi_next_nondebug (&gsi); gimple *next_stmt = gsi_stmt (gsi); + if (!next_stmt) + { + /* When there is no statement in the same basic block check + the immediate successor block. */ + if (basic_block bb = gimple_bb (stmt)) + { + basic_block nextbb + = EDGE_COUNT (bb->succs) ? EDGE_SUCC (bb, 0)->dest : NULL; + gimple_stmt_iterator it = gsi_start_bb (nextbb); + next_stmt = gsi_stmt (it); + } + } - if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt)) + if (next_stmt && is_gimple_assign (next_stmt)) { tree lhs = gimple_assign_lhs (next_stmt); tree_code code = TREE_CODE (lhs); Index: gcc/testsuite/gcc.dg/Wstringop-truncation.c =================================================================== --- gcc/testsuite/gcc.dg/Wstringop-truncation.c (nonexistent) +++ gcc/testsuite/gcc.dg/Wstringop-truncation.c (working copy) @@ -0,0 +1,132 @@ +/* PR tree-optimization/84468 - Inconsistent -Wstringop-truncation warnings + with -O2 + { dg-do compile } + { dg-options "-O2 -Wstringop-truncation -ftrack-macro-expansion=0" } */ + +#define strncpy __builtin_strncpy + +struct A +{ + char a[4]; +}; + +void no_pred_succ_lit (struct A *p) +{ + /* The following is folded early on, before the strncpy statement + has a basic block. Verify that the case is handled gracefully + (i.e., there's no assumption that the statement does have + a basic block). */ + strncpy (p->a, "1234", sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */ +} + +/* Verify a strncpy call in a basic block with no predecessor or + successor. */ +void no_pred_succ (struct A *p, const struct A *q) +{ + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */ +} + + +/* Verify a strncpy call in a basic block with no successor. */ +void no_succ (struct A *p, const struct A *q) +{ + if (q->a) + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */ +} + + +/* Verify a strncpy call in a basic block with nul assignment in + a successor block. */ +void succ (struct A *p, const struct A *q) +{ + /* Verify that the assignment suppresses the warning for the conditional + strcnpy call. The conditional should be folded to true since the + address of an array can never be null (see bug 84470). */ + if (q->a) + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */ + + p->a[sizeof p->a - 1] = 0; +} + + +void succ_2 (struct A *p, const struct A *q, int i) +{ + /* Same as above but with a conditional that cannot be eliminated. */ + if (i < 0) + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */ + + p->a[sizeof p->a - 1] = 0; +} + + +/* Verify a strncpy call in a basic block with nul assignment in + the next successor block. */ +int next_succ (struct A *p, const struct A *q, int i, int j) +{ + /* Same as above but with a nested conditionals with else clauses. */ + if (i < 0) + { + if (j < 0) + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */ + } + else + __builtin_strcpy (p->a, q->a); + + p->a[sizeof p->a - 1] = 0; + return 0; +} + + +int next_succ_1 (struct A *p, const struct A *q, int i, int j) +{ + /* Same as above but with a nested conditionals with else clauses. */ + if (i < 0) + { + if (j < 0) + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */ + else + strncpy (p->a, q->a, sizeof p->a - 2); /* { dg-bogus "\\\[-Wstringop-truncation" } */ + } + + p->a[sizeof p->a - 2] = 0; + return 1; +} + + +int next_succ_2 (struct A *p, const struct A *q, int i, int j) +{ + /* Same as above but with a nested conditionals with else clauses. */ + if (i < 0) + { + if (j < 0) + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */ + else + strncpy (p->a, q->a, sizeof p->a - 2); /* { dg-bogus "\\\[-Wstringop-truncation" } */ + } + else + __builtin_strcpy (p->a, q->a); + + p->a[sizeof p->a - 2] = 0; + return 2; +} + + +void cond_succ_warn (struct A *p, const struct A *q, int i) +{ + /* Verify that a conditional assignment doesn't suppress the warning. */ + strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */ + + if (i < 0) + p->a[sizeof p->a - 1] = 0; +} + +void cond_succ_nowarn (struct A *p, const struct A *q) +{ + /* Verify that distinct but provably equivalent conditionals are + recognized and don't trigger the warning. */ + if (p != q) + strncpy (p->a, q->a, sizeof p->a - 1); + + if (p->a != q->a) + p->a[sizeof p->a - 1] = 0; +} --------------6A62F8194C45FE6ACFEF65AA--