From patchwork Mon Nov 15 17:33:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 47693 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B8D793857405 for ; Mon, 15 Nov 2021 17:35:56 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from burlywood.elm.relay.mailchannels.net (burlywood.elm.relay.mailchannels.net [23.83.212.26]) by sourceware.org (Postfix) with ESMTPS id 692D33857027 for ; Mon, 15 Nov 2021 17:34:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 692D33857027 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 6D3B121B9A; Mon, 15 Nov 2021 17:34:18 +0000 (UTC) Received: from pdx1-sub0-mail-a307.dreamhost.com (100-96-11-115.trex.outbound.svc.cluster.local [100.96.11.115]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 0CCB421961; Mon, 15 Nov 2021 17:34:18 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a307.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.96.11.115 (trex/6.4.3); Mon, 15 Nov 2021 17:34:18 +0000 X-MC-Relay: Junk X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Chemical-Towering: 693a29873432ef5c_1636997658291_2875370130 X-MC-Loop-Signature: 1636997658290:1828507113 X-MC-Ingress-Time: 1636997658290 Received: from rhbox.redhat.com (unknown [1.186.223.29]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a307.dreamhost.com (Postfix) with ESMTPSA id 4HtGX82PS3z21; Mon, 15 Nov 2021 09:34:15 -0800 (PST) From: Siddhesh Poyarekar To: gcc-patches@gcc.gnu.org Subject: [PATCH v2 3/3] gimple-fold: Use ranges to simplify strncat and snprintf Date: Mon, 15 Nov 2021 23:03:15 +0530 Message-Id: <20211115173315.1857598-4-siddhesh@gotplt.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211115173315.1857598-1-siddhesh@gotplt.org> References: <20211111194116.1626980-1-siddhesh@gotplt.org> <20211115173315.1857598-1-siddhesh@gotplt.org> MIME-Version: 1.0 X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_BL_SPAMCOP_NET, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Use ranges for lengths and object sizes in strncat and snprintf to determine if they can be transformed into simpler operations. gcc/ChangeLog: * gimple-fold.c (gimple_fold_builtin_strncat): Use ranges to determine if it is safe to transform to strcat. (gimple_fold_builtin_snprintf): Likewise. gcc/testsuite/ChangeLog: * gcc.dg/fold-stringops-2.c: Define size_t. (safe1): Adjust. (safe4): New test. * gcc.dg/fold-stringops-3.c: New test. Signed-off-by: Siddhesh Poyarekar --- gcc/gimple-fold.c | 102 ++++++++++++------------ gcc/testsuite/gcc.dg/fold-stringops-2.c | 16 +++- gcc/testsuite/gcc.dg/fold-stringops-3.c | 18 +++++ 3 files changed, 82 insertions(+), 54 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-3.c diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index f3362287c0d..50b9ba8d558 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -2485,72 +2485,73 @@ gimple_fold_builtin_strncat (gimple_stmt_iterator *gsi) tree dst = gimple_call_arg (stmt, 0); tree src = gimple_call_arg (stmt, 1); tree len = gimple_call_arg (stmt, 2); - - const char *p = c_getstr (src); + tree src_len = c_strlen (src, 1); /* If the requested length is zero, or the src parameter string length is zero, return the dst parameter. */ - if (integer_zerop (len) || (p && *p == '\0')) + if (integer_zerop (len) || (src_len && integer_zerop (src_len))) { replace_call_with_value (gsi, dst); return true; } - if (TREE_CODE (len) != INTEGER_CST || !p) - return false; - - unsigned srclen = strlen (p); - - int cmpsrc = compare_tree_int (len, srclen); - /* Return early if the requested len is less than the string length. Warnings will be issued elsewhere later. */ - if (cmpsrc < 0) + if (!src_len || known_lower (stmt, len, src_len, true)) return false; unsigned HOST_WIDE_INT dstsize; + bool found_dstsize = compute_builtin_object_size (dst, 1, &dstsize); - bool nowarn = warning_suppressed_p (stmt, OPT_Wstringop_overflow_); - - if (!nowarn && compute_builtin_object_size (dst, 1, &dstsize)) + /* Warn on constant LEN. */ + if (TREE_CODE (len) == INTEGER_CST) { - int cmpdst = compare_tree_int (len, dstsize); + bool nowarn = warning_suppressed_p (stmt, OPT_Wstringop_overflow_); - if (cmpdst >= 0) + if (!nowarn && found_dstsize) { - tree fndecl = gimple_call_fndecl (stmt); + int cmpdst = compare_tree_int (len, dstsize); + + if (cmpdst >= 0) + { + tree fndecl = gimple_call_fndecl (stmt); + + /* Strncat copies (at most) LEN bytes and always appends + the terminating NUL so the specified bound should never + be equal to (or greater than) the size of the destination. + If it is, the copy could overflow. */ + location_t loc = gimple_location (stmt); + nowarn = warning_at (loc, OPT_Wstringop_overflow_, + cmpdst == 0 + ? G_("%qD specified bound %E equals " + "destination size") + : G_("%qD specified bound %E exceeds " + "destination size %wu"), + fndecl, len, dstsize); + if (nowarn) + suppress_warning (stmt, OPT_Wstringop_overflow_); + } + } - /* Strncat copies (at most) LEN bytes and always appends - the terminating NUL so the specified bound should never - be equal to (or greater than) the size of the destination. - If it is, the copy could overflow. */ + if (!nowarn && TREE_CODE (src_len) == INTEGER_CST + && tree_int_cst_compare (src_len, len) == 0) + { + tree fndecl = gimple_call_fndecl (stmt); location_t loc = gimple_location (stmt); - nowarn = warning_at (loc, OPT_Wstringop_overflow_, - cmpdst == 0 - ? G_("%qD specified bound %E equals " - "destination size") - : G_("%qD specified bound %E exceeds " - "destination size %wu"), - fndecl, len, dstsize); - if (nowarn) + + /* To avoid possible overflow the specified bound should also + not be equal to the length of the source, even when the size + of the destination is unknown (it's not an uncommon mistake + to specify as the bound to strncpy the length of the source). */ + if (warning_at (loc, OPT_Wstringop_overflow_, + "%qD specified bound %E equals source length", + fndecl, len)) suppress_warning (stmt, OPT_Wstringop_overflow_); } } - if (!nowarn && cmpsrc == 0) - { - tree fndecl = gimple_call_fndecl (stmt); - location_t loc = gimple_location (stmt); - - /* To avoid possible overflow the specified bound should also - not be equal to the length of the source, even when the size - of the destination is unknown (it's not an uncommon mistake - to specify as the bound to strncpy the length of the source). */ - if (warning_at (loc, OPT_Wstringop_overflow_, - "%qD specified bound %E equals source length", - fndecl, len)) - suppress_warning (stmt, OPT_Wstringop_overflow_); - } + if (!known_lower (stmt, src_len, len)) + return false; tree fn = builtin_decl_implicit (BUILT_IN_STRCAT); @@ -3623,10 +3624,6 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi) if (gimple_call_num_args (stmt) == 4) orig = gimple_call_arg (stmt, 3); - if (!tree_fits_uhwi_p (destsize)) - return false; - unsigned HOST_WIDE_INT destlen = tree_to_uhwi (destsize); - /* Check whether the format is a literal string constant. */ fmt_str = c_getstr (fmt); if (fmt_str == NULL) @@ -3646,6 +3643,8 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi) if (orig) return false; + tree len = build_int_cstu (TREE_TYPE (destsize), strlen (fmt_str)); + /* We could expand this as memcpy (str, fmt, cst - 1); str[cst - 1] = '\0'; or to @@ -3653,8 +3652,7 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi) but in the former case that might increase code size and in the latter case grow .rodata section too much. So punt for now. */ - size_t len = strlen (fmt_str); - if (len >= destlen) + if (!known_lower (stmt, len, destsize, true)) return false; gimple_seq stmts = NULL; @@ -3663,7 +3661,7 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi) if (tree lhs = gimple_call_lhs (stmt)) { repl = gimple_build_assign (lhs, - build_int_cst (TREE_TYPE (lhs), len)); + fold_convert (TREE_TYPE (lhs), len)); gimple_seq_add_stmt_without_update (&stmts, repl); gsi_replace_with_seq_vops (gsi, stmts); /* gsi now points at the assignment to the lhs, get a @@ -3694,8 +3692,6 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi) return false; tree orig_len = get_maxval_strlen (orig, SRK_STRLEN); - if (!orig_len || TREE_CODE (orig_len) != INTEGER_CST) - return false; /* We could expand this as memcpy (str1, str2, cst - 1); str1[cst - 1] = '\0'; @@ -3704,7 +3700,7 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi) but in the former case that might increase code size and in the latter case grow .rodata section too much. So punt for now. */ - if (compare_tree_int (orig_len, destlen) >= 0) + if (!known_lower (stmt, orig_len, destsize, true)) return false; /* Convert snprintf (str1, cst, "%s", str2) into diff --git a/gcc/testsuite/gcc.dg/fold-stringops-2.c b/gcc/testsuite/gcc.dg/fold-stringops-2.c index 0b415dfaf57..ac7d29eac50 100644 --- a/gcc/testsuite/gcc.dg/fold-stringops-2.c +++ b/gcc/testsuite/gcc.dg/fold-stringops-2.c @@ -1,10 +1,12 @@ /* { dg-do compile } */ /* { dg-options "-O2" } */ +typedef __SIZE_TYPE__ size_t; + #define bos(__d) __builtin_object_size ((__d), 0) char * -safe1 (const char *src, int cond, __SIZE_TYPE__ len) +safe1 (const char *src, int cond, size_t len) { char *dst; @@ -44,6 +46,18 @@ safe3 (const char *src, int cond, unsigned char len) return __builtin___snprintf_chk (dst, len, 0, bos (dst), "%s", src); } +char dst[1024]; + +void +safe4 (size_t len) +{ + len = len > sizeof (dst) - 1 ? sizeof (dst) - 1 : len; + len = len < sizeof (dst) / 2 ? sizeof (dst) / 2 : len; + + __builtin_strncat (dst, "hello", len); +} + /* { dg-final { scan-assembler-not "__memcpy_chk" } } */ /* { dg-final { scan-assembler-not "__strncpy_chk" } } */ /* { dg-final { scan-assembler-not "__snprintf_chk" } } */ +/* { dg-final { scan-assembler-not "strncat" } } */ diff --git a/gcc/testsuite/gcc.dg/fold-stringops-3.c b/gcc/testsuite/gcc.dg/fold-stringops-3.c new file mode 100644 index 00000000000..ae2efbf9967 --- /dev/null +++ b/gcc/testsuite/gcc.dg/fold-stringops-3.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +typedef __SIZE_TYPE__ size_t; + +char dst[1024]; + +void +safe1 (size_t len) +{ + len = len > sizeof (dst) ? sizeof (dst) : len; + len = len < sizeof (dst) / 2 ? sizeof (dst) / 2 : len; + + __builtin_snprintf (dst, len, "hello"); + __builtin_snprintf (dst + 5, len, "%s", " world"); +} + +/* { dg-final { scan-assembler-not "snprintf" } } */