From patchwork Mon Feb 10 22:47:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1236029 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-519287-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=dHTfeHVs; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=Y3qVotcb; 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 48GgzR5tHCz9sP7 for ; Tue, 11 Feb 2020 09:48:01 +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=k1ZdY9BZ+Sq+bVrOoZGmIna+Ein9d0Ec0f4tC9XVDfgXITXbyu Xja1pxm3NTYjvhZvYkNMTCntc8JQ3u11srPZKQOYsTpqSuUX+tjTMI7bd/qpibPk xCzlhGWbRfIDBv3CRnFmkUE9TJneja/X0kwJ4RugO2VEVmCIC/O/n755Y= 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=te5KvqoUzF7sFAunpkUNOwtYO5I=; b=dHTfeHVsZWaSicC51vXG Z8xN8sGaY4VBFmZ9KPcXjt1K7FDmTAcIS+lAzgQd+Dsg1rXLxq2d1D2TvNqbFmTi yGRQB0iio6nmMsBwJX31hOV/S0gMlVtx0bduA+D9ApypyvMPDwtlAwf6U/RzQDNN ncnP3iEBlTZ6CYOxtWnYnsc= Received: (qmail 114916 invoked by alias); 10 Feb 2020 22:47:54 -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 114908 invoked by uid 89); 10 Feb 2020 22:47:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=__SIZE_TYPE__, __size_type__ X-HELO: mail-qk1-f175.google.com Received: from mail-qk1-f175.google.com (HELO mail-qk1-f175.google.com) (209.85.222.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 10 Feb 2020 22:47:52 +0000 Received: by mail-qk1-f175.google.com with SMTP id p7so3337279qkh.10 for ; Mon, 10 Feb 2020 14:47:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=nSXuU0z1feqr7O2/kUxf5ragWTjB8nba0wbLOmq9l5Y=; b=Y3qVotcbUD8rc/3N2+IjcJQekgssRitaSHyVWu3P+Wtj4fosHkAVVM/DXZA61d/tS1 nFE4fUcgjhO2Ojtjzq3MBjpQn/QdxIAdt7Y4nMJrSukpvtLDYw1TqYAQgshCMSR2sndm 6kztbki8NXQpu0KPneFSeMMwAzhIzOEg4kuwGSgQlVeQ94TtBUBLYRpBVP7AT+ooV/x6 V+Tt/deYPRbSChvcLPayY1mCi/uJUAaYYBgO1tVL0+sxllE4rZXXNeySO5G8EVMjnkHu wJxKH/vyJc+Z58uPNnNSVbzeuNS82GGe0tBDx441W/7eKuM+zvEBMJUoszV6hnK5COQ8 nYug== Received: from [192.168.0.41] (174-16-112-158.hlrn.qwest.net. [174.16.112.158]) by smtp.gmail.com with ESMTPSA id y197sm885722qka.65.2020.02.10.14.47.49 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 Feb 2020 14:47:50 -0800 (PST) To: gcc-patches From: Martin Sebor Subject: [PATCH] issue -Wstringop-overflow for potential overflow, not -truncation (PR 93646) Message-ID: <1cd4381b-e1e9-9ae0-ab94-779eb5b07663@gmail.com> Date: Mon, 10 Feb 2020 15:47:48 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 X-IsSubscribed: yes The reporter of RHBZ #1798636 was mislead and confused by GCC issuing -Wstringop-truncation for a possible overflow in strncat. It took a few iterations to appreciate this subtlety and realize the warning was of the wrong kind. The attached patch adjusts the logic of the function responsible for the warning not to issue -Wstringop-truncation only for strncpy and -Wstringop-overflow for strncat. I'm not sure if the bug is strictly speaking a regression: GCC 8 didn't issue any warning so it could be viewed as one, but then again, issuing a warning in this instance is intended, but not a misleading one. Tested on x86_64-linux. Martin PR middle-end/93646 - confusing -Wstringop-truncation on strncat where -Wstringop-overflow is expected gcc/ChangeLog: PR middle-end/93646 * tree-ssa-strlen.c (handle_builtin_stxncpy): Rename... (handle_builtin_stxncpy_strncat): ...to this. Change first argument. Issue only -Wstringop-overflow strncat, never -Wstringop-truncation. (strlen_check_and_optimize_call): Adjust callee name. gcc/testsuite/ChangeLog: PR middle-end/93646 * gcc.dg/Wstringop-overflow-31.c: New test. diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-31.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-31.c new file mode 100644 index 00000000000..24be256a547 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-31.c @@ -0,0 +1,40 @@ +/* PR middle-end/93646 - confusing -Wstringop-truncation on strncat where + -Wstringop-overflow is expected + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +extern __SIZE_TYPE__ strlen (const char*); +extern char* strncat (char*, const char*, __SIZE_TYPE__); + + +char a[4]; + + +void f0 (char *d, const char *s) +{ + strncat (d, s, strlen (s)); // { dg-warning "specified bound depends on the length of the source argument" } + /* { dg-message "function 'f0'.*inlined from 'f1'" "inlining stack" { target *-*-* } 0 } */ + + // Prevent f0 from being replaced by g0. + *d = 'f'; +} + +void f1 (const char *s) +{ + f0 (a, s); +} + + +static void g0 (char *d, const char *s) +{ + strncat (d, s, strlen (s)); // { dg-warning "specified bound 3 equals source length" } + /* { dg-message "function 'g0'.*inlined from 'g1'" "inlining stack" { target *-*-* } 0 } */ + + // Prevent g0 from being replaced by f0. + *d = 'g'; +} + +void g1 (void) +{ + g0 (a, "123"); +} diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index 1cd64302d8b..9a88a85b07c 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -192,7 +192,7 @@ struct laststmt_struct } laststmt; static int get_stridx_plus_constant (strinfo *, unsigned HOST_WIDE_INT, tree); -static void handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *); +static void handle_builtin_stxncpy_strncat (bool, gimple_stmt_iterator *); /* Sets MINMAX to either the constant value or the range VAL is in and returns either the constant value or VAL on success or null @@ -2876,10 +2876,10 @@ handle_builtin_strcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi, and if so, issue an appropriate warning. */ static void -handle_builtin_strncat (built_in_function bcode, gimple_stmt_iterator *gsi) +handle_builtin_strncat (built_in_function, gimple_stmt_iterator *gsi) { /* Same as stxncpy(). */ - handle_builtin_stxncpy (bcode, gsi); + handle_builtin_stxncpy_strncat (true, gsi); } /* Return true if LEN depends on a call to strlen(SRC) in an interesting @@ -2974,8 +2974,8 @@ is_strlen_related_p (tree src, tree len) return false; } -/* Called by handle_builtin_stxncpy and by gimple_fold_builtin_strncpy - in gimple-fold.c. +/* Called by handle_builtin_stxncpy_strncat and by + gimple_fold_builtin_strncpy in gimple-fold.c. Check to see if the specified bound is a) equal to the size of the destination DST and if so, b) if it's immediately followed by DST[CNT - 1] = '\0'. If a) holds and b) does not, warn. Otherwise, @@ -3283,13 +3283,14 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) return false; } -/* Check the arguments to the built-in forms of stpncpy and strncpy for - out-of-bounds offsets or overlapping access, and to see if the size - is derived from calling strlen() on the source argument, and if so, - issue the appropriate warning. */ +/* Check the arguments to the built-in forms of stpncpy, strncpy, and + strncat, for out-of-bounds offsets or overlapping access, and to see + if the size is derived from calling strlen() on the source argument, + and if so, issue the appropriate warning. + APPEND_P is true for strncat. */ static void -handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi) +handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi) { if (!strlen_to_stridx) return; @@ -3385,7 +3386,8 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi) whether its value is known. Otherwise, issue the more generic -Wstringop-overflow which triggers for LEN arguments that in any meaningful way depend on strlen(SRC). */ - if (sisrc == silen + if (!append_p + && sisrc == silen && is_strlen_related_p (src, len) && warning_at (callloc, OPT_Wstringop_truncation, "%G%qD output truncated before terminating nul " @@ -5352,7 +5354,7 @@ strlen_check_and_optimize_call (gimple_stmt_iterator *gsi, bool *zero_write, case BUILT_IN_STPNCPY_CHK: case BUILT_IN_STRNCPY: case BUILT_IN_STRNCPY_CHK: - handle_builtin_stxncpy (DECL_FUNCTION_CODE (callee), gsi); + handle_builtin_stxncpy_strncat (false, gsi); break; case BUILT_IN_MEMCPY: