From patchwork Tue Mar 15 05:31:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 51969 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 D0548385DC19 for ; Tue, 15 Mar 2022 05:32:39 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from hedgehog.birch.relay.mailchannels.net (hedgehog.birch.relay.mailchannels.net [23.83.209.81]) by sourceware.org (Postfix) with ESMTPS id C41043858D28 for ; Tue, 15 Mar 2022 05:32:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C41043858D28 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 32B59621260; Tue, 15 Mar 2022 05:32:21 +0000 (UTC) Received: from pdx1-sub0-mail-a307.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 9C37862120A; Tue, 15 Mar 2022 05:32:19 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1647322339; a=rsa-sha256; cv=none; b=6m1Ef271ItfcP+UMuybL7tNP1PjxV6Wpc2tfmIMWxLNjKiD7gIPYO6blgeR4OcOtFtTkc8 WktGOm64tXQLCdnHqlIGkNyc9/KoFDWnFzlbzGuWdAmaEA6S0ivKn1Z0P40neN+lYFKzsS mvQibbicanGWn0J+u6koI9l1pepOczneo73PpHwACwtwAo+DHDMR+pdrAYXdrdKCX0voTg YAHqXIH2SYQ004zED/HoP7lC9Vd6YkU7h1s4tXHMrFx2ZwoMtmipXiV14G/IvJitpMiiKe ATfoVV13rjNUtwMgZd6CamkrN/JBQXakShWTTr/zao0eqYIigOvWepGi+x7MNQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1647322339; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=zt0SNHbGxvKjt1DQEYxQIs438QbKzEUVdN0i9xg/XfI=; b=u+jYDuNScV5Ay3tp5ZGq/0PZz5U/csG1LQOpNgH5zrPlUQYEtyD5fAfHcUZ+VBeMwSco8q 5jM5bjhe1U2gH6mJVQy2xxZGU4tm019dj1sGesle2/q1n8gV1Fjybf+iYbtK88Vji0pZ26 d0UqOvCYdi5nclaLzTy8dmdTIQw1RN4dDdz7uy77KiSmCzzZ5NW5/qdbJoevO2P1mZo/1K jwFZJbZG0YZXegYNa4vmEzoVeoxw2wCOsOBSzwqJ3vqo689m62e7anzWZogCS3ts82nTza fpcRKXCL31AmVGjGI4BzGsE4Akyr9K5NPH2fb7y90vINN+VcqdMGzKuR9rgVMw== ARC-Authentication-Results: i=1; rspamd-c9cb649d9-m6sw7; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org 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.119.141.221 (trex/6.5.3); Tue, 15 Mar 2022 05:32:21 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Versed-White: 213f7ccb124eb901_1647322340432_857305400 X-MC-Loop-Signature: 1647322340430:664783551 X-MC-Ingress-Time: 1647322340429 Received: from rhbox.redhat.com (unknown [1.186.122.132]) (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 4KHhqj0dm7z22; Mon, 14 Mar 2022 22:32:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1647322339; bh=zt0SNHbGxvKjt1DQEYxQIs438QbKzEUVdN0i9xg/XfI=; h=From:To:Cc:Subject:Date:Content-Transfer-Encoding; b=jdOwHlZbAVcPBenesuxmGdDD6maHeIw/9+soNmT7x2JR+HTe5IQXJSSkfrCbWmiVh I40VAULRfTEZLxgMmoEFIwvxSXVMDeMgzjuuHr7dzuy85dSxnqetqFX49REbVmgPzq 2SvcQt6vYzL5FQC52a/zPvuprYeuCfuTAyLPPbWjGEi7O2hwmJ6m20evItQDfVWTm6 61RNPhqyUJ0m04JjaCPkDNVvDv3wZxgsG4yZmtAZjU4bqouJ+GaydR6YW+W7/myRbA ltEJHkd9WNqoIrytsJ//vuZinCDDiErBkw2bpY9axBIUQIsUmjHyr2+izqPyZzKzrF y89Rz1N4Leaiw== From: Siddhesh Poyarekar To: gcc-patches@gcc.gnu.org Subject: [PATCH v2] middle-end/104854: Limit strncmp overread warnings Date: Tue, 15 Mar 2022 11:01:58 +0530 Message-Id: <20220315053158.1486555-1-siddhesh@gotplt.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220311071236.2349381-1-siddhesh@gotplt.org> References: <20220311071236.2349381-1-siddhesh@gotplt.org> MIME-Version: 1.0 X-Spam-Status: No, score=-3037.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, RCVD_IN_SBL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: , Cc: jakub@redhat.com, msebor@redhat.com Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" The size argument in strncmp only describe the maximum length to which to compare two strings and is not an indication of sizes of the two source strings. Do not warn if it is larger than the two input strings because it is entirely likely that the size argument is a conservative maximum to accommodate inputs of different lengths and only a subset is reachable through the current code path or that it is some other application-specific property completely unrelated to the sizes of the input strings. gcc/ChangeLog: middle-end/104854 * gimple-ssa-warn-access.cc (pass_waccess::warn_zero_sized_strncmp_inputs): New function. (pass_waccess::check_strncmp): Use it. gcc/testsuite/ChangeLog: middle-end/104854 * gcc.dg/Wstringop-overread.c (test_strncmp_array): Don't expect failures for non-zero sizes. Signed-off-by: Siddhesh Poyarekar --- Changes from v1: A little better approach, ensuring that it tries to warn on zero length inputs if the size of at least one of the two sources is known. Also cc'ing Martin so that we can discuss approach on the list instead of on the bug. To summarize the discussion so far, Martin suggests that the warning be split into levels but I'm contesting the utility of the heuristics as a compiler warning given the looseness of the relationship between the size argument and the inputs in the case of these functions. gcc/gimple-ssa-warn-access.cc | 69 +++++++++-------------- gcc/testsuite/gcc.dg/Wstringop-overread.c | 2 +- 2 files changed, 28 insertions(+), 43 deletions(-) diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 75297ed7c9e..15299770e29 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -2137,6 +2137,9 @@ private: /* Return true if use follows an invalidating statement. */ bool use_after_inval_p (gimple *, gimple *, bool = false); + /* Emit an overread warning for zero sized inputs to strncmp. */ + void warn_zero_sized_strncmp_inputs (gimple *, tree *, access_data *); + /* A pointer_query object to store information about pointers and their targets in. */ pointer_query m_ptr_qry; @@ -2619,8 +2622,20 @@ pass_waccess::check_stxncpy (gcall *stmt) data.mode, &data, m_ptr_qry.rvals); } -/* Check a call STMT to stpncpy() or strncpy() for overflow and warn - if it does. */ +/* Warn for strncmp on a zero sized source or when an argument isn't + nul-terminated. */ +void +pass_waccess::warn_zero_sized_strncmp_inputs (gimple *stmt, tree *bndrng, + access_data *pad) +{ + tree func = get_callee_fndecl (stmt); + location_t loc = gimple_location (stmt); + maybe_warn_for_bound (OPT_Wstringop_overread, loc, stmt, func, bndrng, + size_zero_node, pad); +} + +/* Check a call STMT to strncmp () for overflow and warn if it does. This is + limited to checking for NUL terminated arrays for now. */ void pass_waccess::check_strncmp (gcall *stmt) @@ -2678,46 +2693,16 @@ pass_waccess::check_strncmp (gcall *stmt) if (!bndrng[0] || integer_zerop (bndrng[0])) return; - if (len1 && tree_int_cst_lt (len1, bndrng[0])) - bndrng[0] = len1; - if (len2 && tree_int_cst_lt (len2, bndrng[0])) - bndrng[0] = len2; - - /* compute_objsize almost never fails (and ultimately should never - fail). Don't bother to handle the rare case when it does. */ - if (!compute_objsize (arg1, stmt, 1, &adata1.src, &m_ptr_qry) - || !compute_objsize (arg2, stmt, 1, &adata2.src, &m_ptr_qry)) - return; - - /* Compute the size of the remaining space in each array after - subtracting any offset into it. */ - offset_int rem1 = adata1.src.size_remaining (); - offset_int rem2 = adata2.src.size_remaining (); - - /* Cap REM1 and REM2 at the other if the other's argument is known - to be an unterminated array, either because there's no space - left in it after adding its offset or because it's constant and - has no nul. */ - if (rem1 == 0 || (rem1 < rem2 && lendata1.decl)) - rem2 = rem1; - else if (rem2 == 0 || (rem2 < rem1 && lendata2.decl)) - rem1 = rem2; - - /* Point PAD at the array to reference in the note if a warning - is issued. */ - access_data *pad = len1 ? &adata2 : &adata1; - offset_int maxrem = wi::max (rem1, rem2, UNSIGNED); - if (lendata1.decl || lendata2.decl - || maxrem < wi::to_offset (bndrng[0])) - { - /* Warn when either argument isn't nul-terminated or the maximum - remaining space in the two arrays is less than the bound. */ - tree func = get_callee_fndecl (stmt); - location_t loc = gimple_location (stmt); - maybe_warn_for_bound (OPT_Wstringop_overread, loc, stmt, func, - bndrng, wide_int_to_tree (sizetype, maxrem), - pad); - } + /* compute_objsize almost never fails (and ultimately should never fail). + Don't bother to handle the rare case when it does. Warn if either the + source or destination has zero size, since the minimum bound is non-zero, + hence guaranteeing an overread. */ + if (compute_objsize (arg1, stmt, 1, &adata1.src, &m_ptr_qry) + && adata1.src.size_remaining () == 0) + warn_zero_sized_strncmp_inputs (stmt, bndrng, &adata1); + if (compute_objsize (arg2, stmt, 1, &adata2.src, &m_ptr_qry) + && adata2.src.size_remaining () == 0) + warn_zero_sized_strncmp_inputs (stmt, bndrng, &adata2); } /* Determine and check the sizes of the source and the destination diff --git a/gcc/testsuite/gcc.dg/Wstringop-overread.c b/gcc/testsuite/gcc.dg/Wstringop-overread.c index 7db74029819..fb8e626439d 100644 --- a/gcc/testsuite/gcc.dg/Wstringop-overread.c +++ b/gcc/testsuite/gcc.dg/Wstringop-overread.c @@ -431,7 +431,7 @@ void test_strncmp_array (const char *s, int i) T (strncmp (a1, b1, 0)); T (strncmp (a1, b1, 1)); - T (strncmp (a1, b1, 2)); // { dg-warning "'strncmp' specified bound 2 exceeds source size 1" } + T (strncmp (a1, b1, 2)); }