From patchwork Tue Oct 10 10:49:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 150681 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2908:b0:403:3b70:6f57 with SMTP id ib8csp103568vqb; Tue, 10 Oct 2023 03:49:49 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFRDUHqD+1vonwjkWt9dkxECrwfvulBxTHaU1YVQJxf1LyTySZjrMoXbYCSFkSJDgk2NZ2d X-Received: by 2002:a17:906:535e:b0:9ae:42da:803c with SMTP id j30-20020a170906535e00b009ae42da803cmr13696977ejo.48.1696934989216; Tue, 10 Oct 2023 03:49:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696934989; cv=none; d=google.com; s=arc-20160816; b=LEIHh7nyRknPsYh3R+kksyJ6rIAusMpN5YzMr5KcikDxuqnRVEJuRlmqks8Zq3BQaI 5GJw9MPhhBn6D4Vms/T4D8oOqac+iunMT0IwYW+nDZjHHA7VqmDy1nk66vKmjfRRQ0YP 7eoIZgkWE1RRCUmC5LyTJvyZe6e4vhXBTy6LsMk0uA8vr1AWHBMqrsceRHdzrEr/YU6g ZLDQnFQrTf2PG2+gojqvwEFYJwvUU2Rt54SaiSsDoNB/4FaX1ohKGzgbdAT7yB+nm3Hs dJBCSRNNwsk45/bBQeWDoK/T1OZRdHZHdAB+sqsWdpfmBrGjo8LSFB3kdExrLvw+/1uA FN7A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:errors-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:mime-version :user-agent:subject:cc:to:from:date:dkim-signature:dkim-signature :dmarc-filter:delivered-to; bh=9ZRtZ2yNuxsTfMMPxPb8cnETQkEX9bTovOG4Y9AQpaA=; fh=Hz/QWAL2vMAbrm3W16QrnQrLktFWGNewssxaKtdN1w4=; b=G9LPesKQtJ1YOpFu6so4RzJKpyJ1yifRlZ9KRF0BvbNOzu1rkMm2G+BbnElgL+yW1K 42oK3CZJhJZWx8Aeh+7k5lDbjE5Fa2q/mLpQwC3eVnzACggJoG3T4g7sM7E68fFeACMZ AqSTG4d53wPQjmjmG7huxASB+DgOHI/h5qt8wRwmn5BJ9N7DJSMuqhliow5gf3a1bNiG JR1cLi3jhUFaaJaG7GbXBPeM4mJeb561GtFwxUSh41NmZFnlvM0agGQ/Im59O7QpvZqT OrRn9ZVKDXOr2PPzv/fwCgMFr8e1K8eiE7QBHN9hdJync7XCvLP2FC/cDV4WPyoNzPsf UqTw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=isTMWU2g; dkim=neutral (no key) header.i=@suse.de; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Received: from server2.sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id e10-20020a170906248a00b009b9490eb802si5422057ejb.318.2023.10.10.03.49.48 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 03:49:49 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=isTMWU2g; dkim=neutral (no key) header.i=@suse.de; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D13B53857709 for ; Tue, 10 Oct 2023 10:49:29 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id DEB403858004 for ; Tue, 10 Oct 2023 10:49:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DEB403858004 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 1DCCE1F38A; Tue, 10 Oct 2023 10:49:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1696934944; h=from:from:reply-to:date:date:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=9ZRtZ2yNuxsTfMMPxPb8cnETQkEX9bTovOG4Y9AQpaA=; b=isTMWU2g9VbJKIpfX6vfsRc8/Rh0URBgZFXzn6i0DmkmNahIKgoe6mO3VEYlbonAmybeAR sOXT/9h1lHxnOb9jA0ELaeCY/NKplTswYHf+z4Ys3KQFSypNPIlugQwJM1aUEzg4Ul6eF9 QcFddZ8zctrLqFI0udaEqXqr9Xnmfno= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1696934944; h=from:from:reply-to:date:date:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=9ZRtZ2yNuxsTfMMPxPb8cnETQkEX9bTovOG4Y9AQpaA=; b=KgWApmtmg+d0AL93kQORcDvEg+ThSHER8S0IspJmcSOo1v5TE9659vbwzoo/2kvMy/qTJm 61FRXgUknBcXi9CA== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id E46612C407; Tue, 10 Oct 2023 10:49:03 +0000 (UTC) Date: Tue, 10 Oct 2023 10:49:04 +0000 (UTC) From: Richard Biener To: gcc-patches@gcc.gnu.org cc: Jakub Jelinek Subject: [PATCH] tree-optimization/111519 - strlen optimization skips clobbering store User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, MISSING_MID, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Message-Id: <20231010104929.D13B53857709@sourceware.org> X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1779365302933945736 X-GMAIL-MSGID: 1779365302933945736 The following fixes a mistake in count_nonzero_bytes which happily skips over stores clobbering the memory we load a value we store from and then performs analysis on the memory state before the intermediate store. The patch implements the most simple fix - guarantee that there are no intervening stores by tracking the original active virtual operand and comparing that to the one of a load we attempt to analyze. This simple approach breaks two subtests of gcc.dg/Wstringop-overflow-69.c which I chose to XFAIL. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK? Thanks, Richard. PR tree-optimization/111519 * tree-ssa-strlen.cc (strlen_pass::count_nonzero_bytes): Add virtual operand argument and pass it through. Compare the memory state we try to analyze to the memory state we are going to use the result oon. (strlen_pass::count_nonzero_bytes_addr): Add virtual operand argument and pass it through. * gcc.dg/torture/pr111519.c: New testcase. * gcc.dg/Wstringop-overflow-69.c: XFAIL two subtests. --- gcc/testsuite/gcc.dg/Wstringop-overflow-69.c | 4 +- gcc/testsuite/gcc.dg/torture/pr111519.c | 47 ++++++++++++++++++++ gcc/tree-ssa-strlen.cc | 27 ++++++----- 3 files changed, 64 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr111519.c diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c index be361fe620d..3c17fe13d8e 100644 --- a/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c @@ -57,9 +57,9 @@ void warn_vec_decl (void) { *(VC2*)a1 = c2; // { dg-warning "writing 2 bytes into a region of size 1" } *(VC4*)a2 = c4; // { dg-warning "writing 4 bytes into a region of size 2" } - *(VC4*)a3 = c4; // { dg-warning "writing 4 bytes into a region of size 3" } + *(VC4*)a3 = c4; // { dg-warning "writing 4 bytes into a region of size 3" "pr111519" { xfail *-*-* } } *(VC8*)a4 = c8; // { dg-warning "writing 8 bytes into a region of size 4" } - *(VC8*)a7 = c8; // { dg-warning "writing 8 bytes into a region of size 7" } + *(VC8*)a7 = c8; // { dg-warning "writing 8 bytes into a region of size 7" "pr111519" { xfail *-*-* } } *(VC16*)a15 = c16; // { dg-warning "writing 16 bytes into a region of size 15" } } diff --git a/gcc/testsuite/gcc.dg/torture/pr111519.c b/gcc/testsuite/gcc.dg/torture/pr111519.c new file mode 100644 index 00000000000..095bb1cd13b --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr111519.c @@ -0,0 +1,47 @@ +/* { dg-do run } */ + +int a, o; +char b, f, i; +long c; +static signed char d; +static char g; +unsigned *h; +signed char *e = &f; +static signed char **j = &e; +static long k[2]; +unsigned **l = &h; +short m; +volatile int z; + +__attribute__((noipa)) void +foo (char *p) +{ + (void) p; +} + +int +main () +{ + int p = z; + signed char *n = &d; + *n = 0; + while (c) + for (; i; i--) + ; + for (g = 0; g <= 1; g++) + { + *n = **j; + k[g] = 0 != &m; + *e = l && k[0]; + } + if (p) + foo (&b); + for (; o < 4; o++) + { + a = d; + if (p) + foo (&b); + } + if (a != 1) + __builtin_abort (); +} diff --git a/gcc/tree-ssa-strlen.cc b/gcc/tree-ssa-strlen.cc index 8b7ef919826..0ff3f2e308a 100644 --- a/gcc/tree-ssa-strlen.cc +++ b/gcc/tree-ssa-strlen.cc @@ -281,14 +281,14 @@ public: gimple *stmt, unsigned lenrange[3], bool *nulterm, bool *allnul, bool *allnonnul); - bool count_nonzero_bytes (tree exp, + bool count_nonzero_bytes (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, unsigned lenrange[3], bool *nulterm, bool *allnul, bool *allnonnul, ssa_name_limit_t &snlim); - bool count_nonzero_bytes_addr (tree exp, + bool count_nonzero_bytes_addr (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, @@ -4531,8 +4531,8 @@ nonzero_bytes_for_type (tree type, unsigned lenrange[3], } /* Recursively determine the minimum and maximum number of leading nonzero - bytes in the representation of EXP and set LENRANGE[0] and LENRANGE[1] - to each. + bytes in the representation of EXP at memory state VUSE and set + LENRANGE[0] and LENRANGE[1] to each. Sets LENRANGE[2] to the total size of the access (which may be less than LENRANGE[1] when what's being referenced by EXP is a pointer rather than an array). @@ -4546,7 +4546,7 @@ nonzero_bytes_for_type (tree type, unsigned lenrange[3], Returns true on success and false otherwise. */ bool -strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, +strlen_pass::count_nonzero_bytes (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, unsigned lenrange[3], bool *nulterm, @@ -4566,7 +4566,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, exact value is not known) recurse once to set the range for an arbitrary constant. */ exp = build_int_cst (type, 1); - return count_nonzero_bytes (exp, stmt, + return count_nonzero_bytes (exp, vuse, stmt, offset, 1, lenrange, nulterm, allnul, allnonnul, snlim); } @@ -4574,6 +4574,9 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, gimple *stmt = SSA_NAME_DEF_STMT (exp); if (gimple_assign_single_p (stmt)) { + /* Do not look across other stores. */ + if (gimple_vuse (stmt) != vuse) + return false; exp = gimple_assign_rhs1 (stmt); if (!DECL_P (exp) && TREE_CODE (exp) != CONSTRUCTOR @@ -4594,7 +4597,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, for (unsigned i = 0; i != n; i++) { tree def = gimple_phi_arg_def (stmt, i); - if (!count_nonzero_bytes (def, stmt, + if (!count_nonzero_bytes (def, vuse, stmt, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim)) return false; @@ -4652,7 +4655,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, return false; /* Handle MEM_REF = SSA_NAME types of assignments. */ - return count_nonzero_bytes_addr (arg, stmt, + return count_nonzero_bytes_addr (arg, vuse, stmt, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim); } @@ -4765,7 +4768,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, bytes that are pointed to by EXP, which should be a pointer. */ bool -strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt, +strlen_pass::count_nonzero_bytes_addr (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, unsigned lenrange[3], bool *nulterm, @@ -4835,7 +4838,7 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt, } if (TREE_CODE (exp) == ADDR_EXPR) - return count_nonzero_bytes (TREE_OPERAND (exp, 0), stmt, + return count_nonzero_bytes (TREE_OPERAND (exp, 0), vuse, stmt, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim); @@ -4855,7 +4858,7 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt, for (unsigned i = 0; i != n; i++) { tree def = gimple_phi_arg_def (stmt, i); - if (!count_nonzero_bytes_addr (def, stmt, + if (!count_nonzero_bytes_addr (def, NULL_TREE, stmt, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim)) @@ -4903,7 +4906,7 @@ strlen_pass::count_nonzero_bytes (tree expr_or_type, gimple *stmt, ssa_name_limit_t snlim; tree expr = expr_or_type; - return count_nonzero_bytes (expr, stmt, + return count_nonzero_bytes (expr, gimple_vuse (stmt), stmt, 0, 0, lenrange, nulterm, allnul, allnonnul, snlim); }