From patchwork Mon Jan 20 14:25:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 312565 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 1CC3E2C00A8 for ; Tue, 21 Jan 2014 01:26:03 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=Fyzb9eBQhbAi5d/TY UCR7WgMSiXqURhxiFvqVmTQ3jfmNNZTxqy7Z9o9ZL1MtkYXC5yHfFvieiXdQrHei 2JnJQ7fwc6RczfO7eNFqA9S/7FIAnXgnrKuu41O891jdrv69+P5nbb6kqvMV85E0 Ncr1qMA3gCp9cBMftDEj3urcF8= 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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; s=default; bh=PG39h5lTy7hu2jmz59690HQ M9vo=; b=NNe5LBz3wo0Q8UnuKGoBPtlpXzzSz6QeorkenH+41B8RzP/WoF1I9Go 2T4dC3fTikz6BYdKPWBlSepwdHPvreYp4HdEWHRFe4sTdvSJ/rsVEl4FUEHPCKof HF4gBJf8pXvLQnTbfWoq5Q07NAFPoLu4CVUUMa1iTSMWeTr19Ac8= Received: (qmail 6483 invoked by alias); 20 Jan 2014 14:25:56 -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 6473 invoked by uid 89); 20 Jan 2014 14:25:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 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; Mon, 20 Jan 2014 14:25:54 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s0KEPrr0031287 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 20 Jan 2014 09:25:53 -0500 Received: from tucnak.zalov.cz (vpn1-7-114.ams2.redhat.com [10.36.7.114]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s0KEPpwB008502 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 20 Jan 2014 09:25:52 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.7/8.14.7) with ESMTP id s0KEPoFY027883; Mon, 20 Jan 2014 15:25:50 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.7/8.14.7/Submit) id s0KEPo0C027882; Mon, 20 Jan 2014 15:25:50 +0100 Date: Mon, 20 Jan 2014 15:25:50 +0100 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH][RFC] Fix PR59860 Message-ID: <20140120142550.GA892@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20140119181859.GS892@tucnak.redhat.com> <20140120120928.GY892@tucnak.redhat.com> <20140120132813.GZ892@tucnak.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes On Mon, Jan 20, 2014 at 02:37:21PM +0100, Richard Biener wrote: > Well, strcat itself can do that ... but yes, as I said, if you can > CSE that strlen call then it's a win. But you can't know this without It can't, strcat doesn't know the length of the src string, we don't have any 3 argument strcat alternative API where the compiler tells it the length of the string that the compiler knows, but the function doesn't. > > Normally, if folding of a builtin folds it into a call to some other > > builtin, that other builtin is folded right away, so the common case is > > optimized immediately, the only problem is if gimple_fold_builtin tries > > harder to optimize using maximum lengths or exact length (as in this case). > > And, for this it wouldn't even help if we handled STRCAT/STRCAT_CHK > > specially too and passed the src length to the folder routine, because > > gimple_fold_builtin first folds normally and only if that fails, attempts > > harder. > > But we still can re-introduce the folding I removed from builtins.c > below the avoid_folding_inline_builtin section as in that case > builtins.c didn't do the folding and the gimple folding has > strictly more information. No? I don't really like a special There are two cases. One is where the length of the second string is really variable or not known to the compiler. Then I agree library strcat ought to do the exact same job, it is strlen + movstr instruction. The other case is your testcase, where it is just because the compiler did a poor job. I guess we can use something like following untested patch for that. Or supposedly we could only change fold_builtin_strcat and leave strcat_chk unmodified, after all strcat_chk folding is only handling the case where the size is -1UL (i.e. unknown) and then folds to strcat. 2014-01-20 Jakub Jelinek * tree.h (fold_builtin_strcat, fold_builtin_strcat_chk): New prototypes. * builtins.c (fold_builtin_strcat): No longer static. Add len argument, if non-NULL, don't call c_strlen. Optimize directly into __builtin_memcpy instead of __builtin_strcpy. (fold_builtin_strcat_chk): No longer static. Add len argument, if non-NULL, call fold_builtin_strcat. (fold_builtin_2): Adjust fold_builtin_strcat{,_chk} callers. * gimple-fold.c (gimple_fold_builtin): Handle BUILT_IN_STRCAT and BUILT_IN_STRCAT_CHK. Jakub --- gcc/tree.h.jj 2014-01-07 17:49:40.000000000 +0100 +++ gcc/tree.h 2014-01-20 15:04:48.273418236 +0100 @@ -5854,12 +5854,14 @@ extern tree fold_call_expr (location_t, extern tree fold_builtin_fputs (location_t, tree, tree, bool, bool, tree); extern tree fold_builtin_strcpy (location_t, tree, tree, tree, tree); extern tree fold_builtin_strncpy (location_t, tree, tree, tree, tree, tree); +extern tree fold_builtin_strcat (location_t, tree, tree, tree); extern tree fold_builtin_memory_chk (location_t, tree, tree, tree, tree, tree, tree, bool, enum built_in_function); extern tree fold_builtin_stxcpy_chk (location_t, tree, tree, tree, tree, tree, bool, enum built_in_function); extern tree fold_builtin_stxncpy_chk (location_t, tree, tree, tree, tree, tree, bool, enum built_in_function); +extern tree fold_builtin_strcat_chk (location_t, tree, tree, tree, tree, tree); extern tree fold_builtin_snprintf_chk (location_t, tree, tree, enum built_in_function); extern bool fold_builtin_next_arg (tree, bool); extern enum built_in_function builtin_mathfn_code (const_tree); --- gcc/builtins.c.jj 2014-01-20 12:41:48.000000000 +0100 +++ gcc/builtins.c 2014-01-20 15:19:23.585947825 +0100 @@ -180,7 +180,6 @@ static tree fold_builtin_varargs (locati static tree fold_builtin_strpbrk (location_t, tree, tree, tree); static tree fold_builtin_strstr (location_t, tree, tree, tree); static tree fold_builtin_strrchr (location_t, tree, tree, tree); -static tree fold_builtin_strcat (location_t, tree, tree); static tree fold_builtin_strncat (location_t, tree, tree, tree); static tree fold_builtin_strspn (location_t, tree, tree); static tree fold_builtin_strcspn (location_t, tree, tree); @@ -194,7 +193,6 @@ static void maybe_emit_chk_warning (tree static void maybe_emit_sprintf_chk_warning (tree, enum built_in_function); static void maybe_emit_free_warning (tree); static tree fold_builtin_object_size (tree, tree); -static tree fold_builtin_strcat_chk (location_t, tree, tree, tree, tree); static tree fold_builtin_strncat_chk (location_t, tree, tree, tree, tree, tree); static tree fold_builtin_sprintf_chk (location_t, tree, enum built_in_function); static tree fold_builtin_printf (location_t, tree, tree, tree, bool, enum built_in_function); @@ -10770,7 +10768,7 @@ fold_builtin_2 (location_t loc, tree fnd return fold_builtin_strstr (loc, arg0, arg1, type); case BUILT_IN_STRCAT: - return fold_builtin_strcat (loc, arg0, arg1); + return fold_builtin_strcat (loc, arg0, arg1, NULL_TREE); case BUILT_IN_STRSPN: return fold_builtin_strspn (loc, arg0, arg1); @@ -10963,7 +10961,8 @@ fold_builtin_3 (location_t loc, tree fnd ignore, fcode); case BUILT_IN_STRCAT_CHK: - return fold_builtin_strcat_chk (loc, fndecl, arg0, arg1, arg2); + return fold_builtin_strcat_chk (loc, fndecl, arg0, arg1, + NULL_TREE, arg2); case BUILT_IN_PRINTF_CHK: case BUILT_IN_VPRINTF_CHK: @@ -11813,8 +11812,9 @@ fold_builtin_strpbrk (location_t loc, tr COMPOUND_EXPR in the chain will contain the tree for the simplified form of the builtin function call. */ -static tree -fold_builtin_strcat (location_t loc ATTRIBUTE_UNUSED, tree dst, tree src) +tree +fold_builtin_strcat (location_t loc ATTRIBUTE_UNUSED, tree dst, tree src, + tree len) { if (!validate_arg (dst, POINTER_TYPE) || !validate_arg (src, POINTER_TYPE)) @@ -11832,14 +11832,15 @@ fold_builtin_strcat (location_t loc ATTR /* See if we can store by pieces into (dst + strlen(dst)). */ tree newdst, call; tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN); - tree strcpy_fn = builtin_decl_implicit (BUILT_IN_STRCPY); + tree memcpy_fn = builtin_decl_implicit (BUILT_IN_MEMCPY); - if (!strlen_fn || !strcpy_fn) + if (!strlen_fn || !memcpy_fn) return NULL_TREE; /* If the length of the source string isn't computable don't - split strcat into strlen and strcpy. */ - tree len = c_strlen (src, 1); + split strcat into strlen and memcpy. */ + if (! len) + len = c_strlen (src, 1); if (! len || TREE_SIDE_EFFECTS (len)) return NULL_TREE; @@ -11853,7 +11854,11 @@ fold_builtin_strcat (location_t loc ATTR newdst = fold_build_pointer_plus_loc (loc, dst, newdst); newdst = builtin_save_expr (newdst); - call = build_call_expr_loc (loc, strcpy_fn, 2, newdst, src); + len = fold_convert_loc (loc, size_type_node, len); + len = size_binop_loc (loc, PLUS_EXPR, len, + build_int_cst (size_type_node, 1)); + + call = build_call_expr_loc (loc, memcpy_fn, 3, newdst, src, len); return build2 (COMPOUND_EXPR, TREE_TYPE (dst), call, dst); } return NULL_TREE; @@ -13005,9 +13010,9 @@ fold_builtin_stxncpy_chk (location_t loc /* Fold a call to the __strcat_chk builtin FNDECL. DEST, SRC, and SIZE are the arguments to the call. */ -static tree +tree fold_builtin_strcat_chk (location_t loc, tree fndecl, tree dest, - tree src, tree size) + tree src, tree len, tree size) { tree fn; const char *p; @@ -13030,6 +13035,12 @@ fold_builtin_strcat_chk (location_t loc, if (!fn) return NULL_TREE; + if (len) + { + tree ret = fold_builtin_strcat (loc, dest, src, len); + if (ret) + return ret; + } return build_call_expr_loc (loc, fn, 2, dest, src); } --- gcc/gimple-fold.c.jj 2013-11-05 13:06:02.000000000 +0100 +++ gcc/gimple-fold.c 2014-01-20 15:12:42.297991142 +0100 @@ -866,6 +866,8 @@ gimple_fold_builtin (gimple stmt) break; case BUILT_IN_STRCPY: case BUILT_IN_STRNCPY: + case BUILT_IN_STRCAT: + case BUILT_IN_STRCAT_CHK: arg_idx = 1; type = 0; break; @@ -941,6 +943,22 @@ gimple_fold_builtin (gimple stmt) val[1]); break; + case BUILT_IN_STRCAT: + if (val[1] && is_gimple_val (val[1]) && nargs == 2) + result = fold_builtin_strcat (loc, gimple_call_arg (stmt, 0), + gimple_call_arg (stmt, 1), + val[1]); + break; + + case BUILT_IN_STRCAT_CHK: + if (val[1] && is_gimple_val (val[1]) && nargs == 3) + result = fold_builtin_strcat_chk (loc, callee, + gimple_call_arg (stmt, 0), + gimple_call_arg (stmt, 1), + val[1], + gimple_call_arg (stmt, 2)); + break; + case BUILT_IN_FPUTS: if (nargs == 2) result = fold_builtin_fputs (loc, gimple_call_arg (stmt, 0),