From patchwork Fri Jan 12 21:30:49 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 860208 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-471061-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="gW7wi44g"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zJGBx3r1Mz9s81 for ; Sat, 13 Jan 2018 08:31:05 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=RpFZ80Ktqo+pgH0ldrRN8qbUP9OTJV7gh+kBmiH4lZXZnN4SmM FDhZ3f+vxbOF+Nq83qol1zBHhJeAF+V6F3P/NDlJTBPd/sFHlGek++ayfKT+IVEp adeD5M/h5zTJeiyf09AkmitbRk7WA2wc+16IKequ0PPttQ/YFqhmtMEUA= 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:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=EQ9V1s8vsOKMc6HmWVTvHIXrZ6g=; b=gW7wi44gqJtYxYHavCb0 1zizQW0cYOKHGudNkjroCkbkxDBuyHMEK0ghhOuuppfNOcLnKOFa+MzKycmEvO0H r738e1ASO8TYtUiSFWULaNhW00RlHoi5h1rj8gOIYMLOoYxL5YbbBepNTxi1QHVJ M5VqcT6TwjVKblyEypPIsp8= Received: (qmail 98580 invoked by alias); 12 Jan 2018 21:30:57 -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 98566 invoked by uid 89); 12 Jan 2018 21:30:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.2 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=terminating, 1234, Reset, UD:s1 X-HELO: mail-oi0-f49.google.com Received: from mail-oi0-f49.google.com (HELO mail-oi0-f49.google.com) (209.85.218.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 12 Jan 2018 21:30:54 +0000 Received: by mail-oi0-f49.google.com with SMTP id a70so4848752oib.1 for ; Fri, 12 Jan 2018 13:30:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:from:subject:message-id:date:user-agent :mime-version; bh=iWWblsiO54Wt5T7vaTCeNvLvlPI3zINUygYLNc4ug+I=; b=Y//7npToDYRQPBwJXUbo1NwKweG6Jbhe4wEwXFMsto6fOVLxYDtCfhSuhh/kcphtEO 8ixD7Ajd7lAGuRnAW0GtQ2I6h8RP0i1Ix1E44x7Gj+brdjNZKlVflcirhMys4u5MRjdM 6JbceeFGwo9zCTF+9dheYgZRsfEW/ax2Fo3XE6bMyvz1aluwxFpD+5i4YYSnAqC3ODwS SD82+XnPeNRJIr3BXZf1CES2LvfbKQJ25pLQSc5FgvJbKL2hqAXtafCsdriSBY7GHpTM dULrY0Ohziijgc/aLbwg1Kj39CGX01kT52DtOyT2oAEEYqlBFqbyQ/UC5c9GtMZynLwQ kcLg== X-Gm-Message-State: AKwxytdJRwCGPo5wmzSLQ/fdJmucTRzHa4FVNM8pA9+GqTzrHjsxY76W Myh2PMYa629hOzzWOZz4LxrElA== X-Google-Smtp-Source: ACJfBosp8fMGlujSWgxot+DOAzxvEGegTGoWf04hxpdej+WYDKSYgE32z/HYmbt0tuBltRGjJQJyKA== X-Received: by 10.202.106.203 with SMTP id f194mr12657484oic.353.1515792652826; Fri, 12 Jan 2018 13:30:52 -0800 (PST) Received: from localhost.localdomain ([97.118.124.0]) by smtp.gmail.com with ESMTPSA id k7sm6975844otc.18.2018.01.12.13.30.50 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 12 Jan 2018 13:30:51 -0800 (PST) To: Gcc Patch List From: Martin Sebor Subject: [PATCH] handle local aggregate initialization in strlen (PR 83821) Message-ID: <3c35d53d-41e6-2813-d447-c38931ddd535@gmail.com> Date: Fri, 12 Jan 2018 14:30:49 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 X-IsSubscribed: yes A failure in a test for the recently enhanced -Warray-bounds warning exposed an unnecessarily broad restriction in the strlen pass that prevents it from tracking the length of a member string of locally defined and initialized struct: void f (void) { struct { char s[8]; int i } a = { "1234", 5 }; if (strlen (a.s) != 4) // not folded abort (); } IIUC, the restriction was in place to account for writes into an array changing or invalidating the length of a string stored in its initial elements. This would happen if the write either changed the string's terminating nul byte, or if it reset one of the prior non-nul bytes. To reflect just this intent the restriction can be tightened up to improve the pass' ability to track even the lengths of string members of locally initialized aggregates. Besides leading to better code this change also clears up the test failure. Tested on x86_64-linux. Martin PR tree-optimization/83821 - local aggregate initialization defeats strlen optimization gcc/ChangeLog: PR tree-optimization/83821 * tree-ssa-strlen.c (maybe_invalidate): Consider the length of a string when available. (handle_char_store): Reset calloc statement on a non-nul store. gcc/testsuite/ChangeLog: PR tree-optimization/83821 * c-c++-common/Warray-bounds-4.c: Remove XFAIL. * gcc.dg/strlenopt-43.c: New test. * gcc.dg/strlenopt-44.c: Same. * gcc.dg/tree-ssa/calloc-4.c: Same. Index: gcc/testsuite/gcc.dg/strlenopt-43.c =================================================================== --- gcc/testsuite/gcc.dg/strlenopt-43.c (nonexistent) +++ gcc/testsuite/gcc.dg/strlenopt-43.c (working copy) @@ -0,0 +1,178 @@ +/* PR tree-optimization/83821 - local aggregate initialization defeats + strlen optimization + { dg-do compile } + { dg-options "-O2 -Wall -fdump-tree-optimized" } */ + +#include "strlenopt.h" + +#define CAT(x, y) x ## y +#define CONCAT(x, y) CAT (x, y) +#define FAILNAME(name) CONCAT (call_ ## name ##_on_line_, __LINE__) + +#define FAIL(name) do { \ + extern void FAILNAME (name) (void); \ + FAILNAME (name)(); \ + } while (0) + +/* Macro to emit a call to funcation named + call_in_true_branch_not_eliminated_on_line_NNN() + for each call that's expected to be eliminated. The dg-final + scan-tree-dump-time directive at the bottom of the test verifies + that no such call appears in output. */ +#define ELIM(expr) \ + if (!(expr)) FAIL (in_true_branch_not_eliminated); else (void)0 + +/* Macro to emit a call to a function named + call_made_in_{true,false}_branch_on_line_NNN() + for each call that's expected to be retained. The dg-final + scan-tree-dump-time directive at the bottom of the test verifies + that the expected number of both kinds of calls appears in output + (a pair for each line with the invocation of the KEEP() macro. */ +#define KEEP(expr) \ + if (expr) \ + FAIL (made_in_true_branch); \ + else \ + FAIL (made_in_false_branch) + +#define STR10 "0123456789" +#define STR20 STR10 STR10 +#define STR30 STR20 STR10 +#define STR40 STR20 STR20 + +struct Consec +{ + char s1[sizeof STR40]; + char s2[sizeof STR40]; + const char *p1; + const char *p2; +}; + +void elim_init_consecutive (void) +{ + struct Consec a = { STR10, STR10, STR10, STR10 }; + + ELIM (strlen (a.s1) == sizeof STR10 - 1); + ELIM (strlen (a.s2) == sizeof STR10 - 1); + ELIM (strlen (a.p1) == sizeof STR10 - 1); + ELIM (strlen (a.p2) == sizeof STR10 - 1); +} + + +void elim_array_init_consecutive (void) +{ + struct Consec a[2] = { + { STR10, STR20, STR30, STR40 }, + { STR40, STR30, STR20, STR10 } + }; + + ELIM (strlen (a[0].s1) == sizeof STR10 - 1); + ELIM (strlen (a[0].s2) == sizeof STR20 - 1); + ELIM (strlen (a[0].p1) == sizeof STR30 - 1); + ELIM (strlen (a[0].p2) == sizeof STR40 - 1); + + ELIM (strlen (a[1].s1) == sizeof STR40 - 1); + ELIM (strlen (a[1].s2) == sizeof STR30 - 1); + ELIM (strlen (a[1].p1) == sizeof STR20 - 1); + ELIM (strlen (a[1].p2) == sizeof STR10 - 1); +} + + +struct NonConsec +{ + char s1[sizeof STR40]; + int i1; + char s2[sizeof STR40]; + int i2; + const char *p1; + int i3; + const char *p2; + int i4; +}; + +void elim_init_nonconsecutive (void) +{ + struct NonConsec b = { STR10, 123, STR20, 456, b.s1, 789, b.s2, 123 }; + + ELIM (strlen (b.s1) == sizeof STR10 - 1); + ELIM (strlen (b.s2) == sizeof STR20 - 1); + ELIM (strlen (b.p1) == sizeof STR10 - 1); + ELIM (strlen (b.p2) == sizeof STR20 - 1); +} + +void elim_assign_tmp_nonconsecutive (void) +{ + struct NonConsec b = { "a", 1, "b", 2, "c", 3, "d", 4 }; + + b = (struct NonConsec){ STR10, 123, STR20, 456, STR30, 789, STR40, 123 }; + + ELIM (strlen (b.s1) == sizeof STR10 - 1); + ELIM (strlen (b.s2) == sizeof STR20 - 1); + ELIM (strlen (b.p1) == sizeof STR30 - 1); + ELIM (strlen (b.p2) == sizeof STR40 - 1); +} + +const struct NonConsec bcst = { + STR40, -1, STR30, -2, STR20, -3, STR10, -4 +}; + +void elim_assign_cst_nonconsecutive (void) +{ + struct NonConsec b = { "a", 1, "b", 2, "c", 3, "d" }; + + b = bcst; + + ELIM (strlen (b.s1) == sizeof STR40 - 1); + ELIM (strlen (b.s2) == sizeof STR30 - 1); + ELIM (strlen (b.p1) == sizeof STR20 - 1); + ELIM (strlen (b.p2) == sizeof STR10 - 1); +} + +void elim_copy_cst_nonconsecutive (void) +{ + struct NonConsec b = { "a", 1, "b", 2, "c", 3, "d" }; + memcpy (&b, &bcst, sizeof b); + + /* ELIM (strlen (b.s1) == sizeof STR40 - 1); + ELIM (strlen (b.s2) == sizeof STR30 - 1); */ + ELIM (strlen (b.p1) == sizeof STR20 - 1); + ELIM (strlen (b.p2) == sizeof STR10 - 1); +} + + +#line 1000 + +int sink (void*); + +void keep_init_nonconsecutive (void) +{ + struct NonConsec b = { + STR10, 123, STR20, 456, b.s1, 789, b.s2, + sink (&b) + }; + + KEEP (strlen (b.s1) == sizeof STR10 - 1); + KEEP (strlen (b.s2) == sizeof STR10 - 1); + KEEP (strlen (b.p1) == sizeof STR10 - 1); + KEEP (strlen (b.p2) == sizeof STR10 - 1); +} + +void keep_assign_tmp_nonconsecutive (void) +{ + struct NonConsec b = { "a", 1, "b", 2, "c", 3, "d", 4 }; + + b = (struct NonConsec){ + STR10, 123, STR20, 456, STR30, 789, STR40, + sink (&b) + }; + + KEEP (strlen (b.s1) == sizeof STR10 - 1); + KEEP (strlen (b.s2) == sizeof STR20 - 1); + KEEP (strlen (b.p1) == sizeof STR30 - 1); + KEEP (strlen (b.p2) == sizeof STR40 - 1); +} + +/* { dg-final { scan-tree-dump-times "call_in_true_branch_not_eliminated_" 0 "optimized" } } + + { dg-final { scan-tree-dump-times "call_made_in_true_branch_on_line_1\[0-9\]\[0-9\]\[0-9\]" 8 "optimized" } } + { dg-final { scan-tree-dump-times "call_made_in_false_branch_on_line_1\[0-9\]\[0-9\]\[0-9\]" 8 "optimized" } } */ + Index: gcc/testsuite/gcc.dg/strlenopt-44.c =================================================================== --- gcc/testsuite/gcc.dg/strlenopt-44.c (nonexistent) +++ gcc/testsuite/gcc.dg/strlenopt-44.c (working copy) @@ -0,0 +1,41 @@ +/* PR tree-optimization/83821 - local aggregate initialization defeats + strlen optimization + Verify that a strlen() call is eliminated for a pointer to a region + of memory allocated by calloc() even if one or more nul bytes are + written into it. + { dg-do compile } + { dg-options "-O2 -fdump-tree-optimized" } */ + +unsigned n; + +void* elim_strlen_calloc_store_memset_1 (unsigned a, unsigned b) +{ + char *p = __builtin_calloc (a, 1); + + p[1] = '\0'; + + __builtin_memset (p, 0, b); + + n = __builtin_strlen (p); + + return p; +} + +void* elim_strlen_calloc_store_memset_2 (unsigned a, unsigned b, unsigned c) +{ + char *p = __builtin_calloc (a, 1); + + p[1] = '\0'; + __builtin_memset (p, 0, b); + + n = __builtin_strlen (p); + + p[3] = 0; + __builtin_memset (p, 0, c); + + n = __builtin_strlen (p); + + return p; +} + +/* { dg-final { scan-tree-dump-not "strlen" "optimized" } } */ Index: gcc/testsuite/gcc.dg/tree-ssa/calloc-4.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/calloc-4.c (nonexistent) +++ gcc/testsuite/gcc.dg/tree-ssa/calloc-4.c (working copy) @@ -0,0 +1,39 @@ +/* PR tree-optimization/83821 - local aggregate initialization defeats + strlen optimization + Verify that a memset() call to zero out a subregion of memory + allocated by calloc() is eliminated even if a zero byte is written + into it in between the two calls. See the calloc-2.c test that + verifies that the memset() calls isn't eliminated if the written + value is non-zero. + { dg-do compile } + { dg-options "-O2 -fdump-tree-optimized" } */ + +void* elim_calloc_store_memset_1 (unsigned a, unsigned b) +{ + char *p = __builtin_calloc (a, 1); + + p[1] = '\0'; + + __builtin_memset (p, 0, b); + + n = __builtin_strlen (p); + + return p; +} + +void* elim_calloc_store_memset_2 (unsigned a, unsigned b, unsigned c) +{ + char *p = __builtin_calloc (a, 1); + + p[1] = '\0'; + __builtin_memset (p, 0, b); + + p[3] = 0; + __builtin_memset (p, 0, c); + + return p; +} + +/* { dg-final { scan-tree-dump-not "malloc" "optimized" } } + { dg-final { scan-tree-dump-times "calloc" 2 "optimized" } } + { dg-final { scan-tree-dump-not "memset" "optimized" } } */ Index: gcc/tree-ssa-strlen.c =================================================================== --- gcc/tree-ssa-strlen.c (revision 256590) +++ gcc/tree-ssa-strlen.c (working copy) @@ -693,8 +693,16 @@ maybe_invalidate (gimple *stmt) if (!si->dont_invalidate) { ao_ref r; - /* Do not use si->nonzero_chars. */ - ao_ref_init_from_ptr_and_size (&r, si->ptr, NULL_TREE); + tree size = NULL_TREE; + if (si->nonzero_chars) + { + /* Include the terminating nul in the size of the string + to consider when determining possible clobber. */ + tree type = TREE_TYPE (si->nonzero_chars); + size = fold_build2 (PLUS_EXPR, type, si->nonzero_chars, + build_int_cst (type, 1)); + } + ao_ref_init_from_ptr_and_size (&r, si->ptr, size); if (stmt_may_clobber_ref_p_1 (stmt, &r)) { set_strinfo (i, NULL); @@ -2839,8 +2847,23 @@ handle_char_store (gimple_stmt_iterator *gsi) si = get_strinfo (idx); if (offset == 0) ssaname = TREE_OPERAND (lhs, 0); - else if (si == NULL || compare_nonzero_chars (si, offset) < 0) - return true; + else if (si == NULL) + return true; + else if (compare_nonzero_chars (si, offset) < 0) + { + if (si->stmt && !initializer_zerop (rhs)) + { + /* Reset the calloc statement that created the string + if a non-zero value is being written into it, even + if it's past the leading nul byte, so that subsequent + memset calls aren't eliminated that may clear that + byte. */ + tree fn = gimple_call_fndecl (si->stmt); + if (DECL_FUNCTION_CODE (fn) == BUILT_IN_CALLOC) + si->stmt = NULL; + } + return true; + } } } else