From patchwork Mon May 14 22:41:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 913342 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-477680-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.b="KK4WxB/t"; 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 40lG0G112Dz9s1R for ; Tue, 15 May 2018 08:41:48 +1000 (AEST) 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=IiNrZSE2R/iYjIIYJDfh+OnuZ8iCpaNebkBNymKhCJUmwkoQn2 hXAdde7W/ZEGJpgpfZsGhhkiy+vDp9nra7CHDv/ly8Pxgue7lcMjMd+69sI1pEW4 jKZjZnOeP0eASZs9mBXRE7GyCKBx3oBaWBp98y+ShxVBDCarl83rb/wHc= 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=ywMyCrDPCwS0e5vaW1c96d0Nnw4=; b=KK4WxB/tQuAmDxPsTTE4 jJ8Mrrw+uJgXhvaQfy23AEp9Jxlps7oYoXINJlgYGOVYq/LTej0Ap/G6CShkaC3A 9023iE8hxgyQuLsJeSyjKTvLPqbtj5KF7ypwHkT6Dx7DKqakR0+isII/Hcg3uPZT j0tuHe29V8OxHQhwO9YwOJk= Received: (qmail 115729 invoked by alias); 14 May 2018 22:41:40 -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 115699 invoked by uid 89); 14 May 2018 22:41:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.0 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=wi, gimple.h, gimpleh, UD:gimple.h X-HELO: mail-ot0-f173.google.com Received: from mail-ot0-f173.google.com (HELO mail-ot0-f173.google.com) (74.125.82.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 14 May 2018 22:41:36 +0000 Received: by mail-ot0-f173.google.com with SMTP id t1-v6so16207529ott.13 for ; Mon, 14 May 2018 15:41:36 -0700 (PDT) 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=HOgshtLHRPIIiQDSET+2ugygkkQ+lLOC1sJz0Na32uY=; b=ok0YQqDYDlbI9RjskfL81W4iq5O9w5ND986aHWG2flbR2HYY4CjgGO/q/PsRcpm7Lw rtwl9b9aZWLVY20Sy0VLCg94fQzTe3YWt0MNjOODrpO9QHXzyz4M+8F8gFRn0b0UWUbK FCczwOsHUCvBVuRSvSPQkDANBj8umzyfxtUvdPZdYuUBRnNFu1vgoD3w/FqYYE2sqqUR lIIXSr5red0XuUpkYUBes7Cb5qm1WuybO78ru21PqRKJIsRcIzd1Gabxb8XaBnt1Wstj aELj9jDezmuHoxMHDePgajvUJq+BmAw5TxNXna5yXHLR6xodJmhBtd4YnTMgWaaUq5VS TBXg== X-Gm-Message-State: ALKqPwcy5lFhMoDYsCfTTZMBfruDS23aP0yIVclOYaLTE295Q4iZ9duD nQspZm7RwGDoDKgz8/W4TggXww== X-Google-Smtp-Source: AB8JxZry4GY2RSdh3Do1fwqR1WyPRiCKclZakmHC0ks07pn1zlv+71eUBCOBp+z7L9zBbMEFnrPmSA== X-Received: by 2002:a9d:24a7:: with SMTP id z36-v6mr8785954ota.153.1526337694646; Mon, 14 May 2018 15:41:34 -0700 (PDT) Received: from localhost.localdomain (174-16-119-16.hlrn.qwest.net. [174.16.119.16]) by smtp.gmail.com with ESMTPSA id r129-v6sm5717361oig.26.2018.05.14.15.41.32 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 14 May 2018 15:41:33 -0700 (PDT) To: Gcc Patch List From: Martin Sebor Subject: [PATCH] restore -Wstringop-overflow for checked strcpy/strcat (PR 85259) Message-ID: <9a8db8a9-acd7-665a-1300-a295e1945bf3@gmail.com> Date: Mon, 14 May 2018 16:41:31 -0600 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 r256683 committed to GCC 8 to avoiding duplicate instances of -Wstringop-overflow warnings on some targets has the unintended side-effect of suppressing even singleton instances of the warning in cases such as 'strcat (strcpy (buf, "hello "), "world!")' when _FORTIFY_SOURCE is defined. The attached patch restores the warning for the trunk. Since this is a regression in a security feature and the warning isn't prone to false positives (I don't think I've seen any when _FORTIFY_SOURCE is defined), I'd also like the fix considered for the 8 branch. Thanks Martin PR tree-optimization/85259 - Missing -Wstringop-overflow= since r256683 gcc/ChangeLog: PR tree-optimization/85259 * builtins.c (compute_objsize): Handle constant offsets. * gimple-ssa-warn-restrict.c (maybe_diag_offset_bounds): Return true iff a warning has been issued. * gimple.h (gimple_nonartificial_location): New function. * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Call gimple_nonartificial_location and handle -Wno-system-headers. (handle_builtin_stxncpy): Same. gcc/testsuite/ChangeLog: PR tree-optimization/85259 * gcc.dg/Wstringop-overflow-5.c: New test. * gcc.dg/Wstringop-overflow-6.c: New test. Index: gcc/builtins.c =================================================================== --- gcc/builtins.c (revision 259298) +++ gcc/builtins.c (working copy) @@ -3329,10 +3329,29 @@ compute_objsize (tree dest, int ostype) { /* compute_builtin_object_size fails for addresses with non-constant offsets. Try to determine the range of - such an offset here and use it to adjus the constant + such an offset here and use it to adjust the constant size. */ tree off = gimple_assign_rhs2 (stmt); - if (TREE_CODE (off) == SSA_NAME + if (TREE_CODE (off) == INTEGER_CST) + { + if (tree size = compute_objsize (dest, ostype)) + { + wide_int wioff = wi::to_wide (off); + wide_int wisiz = wi::to_wide (size); + + /* Ignore negative offsets for now. For others, + use the lower bound as the most optimistic + estimate of the (remaining) size. */ + if (wi::sign_mask (wioff)) + ; + else if (wi::ltu_p (wioff, wisiz)) + return wide_int_to_tree (TREE_TYPE (size), + wi::sub (wisiz, wioff)); + else + return size_zero_node; + } + } + else if (TREE_CODE (off) == SSA_NAME && INTEGRAL_TYPE_P (TREE_TYPE (off))) { wide_int min, max; Index: gcc/gimple-ssa-warn-restrict.c =================================================================== --- gcc/gimple-ssa-warn-restrict.c (revision 259298) +++ gcc/gimple-ssa-warn-restrict.c (working copy) @@ -1603,8 +1603,6 @@ maybe_diag_offset_bounds (location_t loc, gcall *c loc = expansion_point_location_if_in_system_header (loc); - tree type; - char rangestr[2][64]; if (ooboff[0] == ooboff[1] || (ooboff[0] != ref.offrange[0] @@ -1615,6 +1613,8 @@ maybe_diag_offset_bounds (location_t loc, gcall *c (long long) ooboff[0].to_shwi (), (long long) ooboff[1].to_shwi ()); + bool warned = false; + if (oobref == error_mark_node) { if (ref.sizrange[0] == ref.sizrange[1]) @@ -1624,6 +1624,8 @@ maybe_diag_offset_bounds (location_t loc, gcall *c (long long) ref.sizrange[0].to_shwi (), (long long) ref.sizrange[1].to_shwi ()); + tree type; + if (DECL_P (ref.base) && TREE_CODE (type = TREE_TYPE (ref.base)) == ARRAY_TYPE) { @@ -1631,19 +1633,22 @@ maybe_diag_offset_bounds (location_t loc, gcall *c "%G%qD pointer overflow between offset %s " "and size %s accessing array %qD with type %qT", call, func, rangestr[0], rangestr[1], ref.base, type)) - inform (DECL_SOURCE_LOCATION (ref.base), - "array %qD declared here", ref.base); + { + inform (DECL_SOURCE_LOCATION (ref.base), + "array %qD declared here", ref.base); + warned = true; + } else - warning_at (loc, OPT_Warray_bounds, - "%G%qD pointer overflow between offset %s " - "and size %s", - call, func, rangestr[0], rangestr[1]); + warned = warning_at (loc, OPT_Warray_bounds, + "%G%qD pointer overflow between offset %s " + "and size %s", + call, func, rangestr[0], rangestr[1]); } else - warning_at (loc, OPT_Warray_bounds, - "%G%qD pointer overflow between offset %s " - "and size %s", - call, func, rangestr[0], rangestr[1]); + warned = warning_at (loc, OPT_Warray_bounds, + "%G%qD pointer overflow between offset %s " + "and size %s", + call, func, rangestr[0], rangestr[1]); } else if (oobref == ref.base) { @@ -1674,22 +1679,26 @@ maybe_diag_offset_bounds (location_t loc, gcall *c "of object %qD with type %qT"), call, func, rangestr[0], ref.base, TREE_TYPE (ref.base))) - inform (DECL_SOURCE_LOCATION (ref.base), - "%qD declared here", ref.base); + { + inform (DECL_SOURCE_LOCATION (ref.base), + "%qD declared here", ref.base); + warned = true; + } } else if (ref.basesize < maxobjsize) - warning_at (loc, OPT_Warray_bounds, - form - ? G_("%G%qD forming offset %s is out of the bounds " - "[0, %wu]") - : G_("%G%qD offset %s is out of the bounds [0, %wu]"), - call, func, rangestr[0], ref.basesize.to_uhwi ()); + warned = warning_at (loc, OPT_Warray_bounds, + form + ? G_("%G%qD forming offset %s is out " + "of the bounds [0, %wu]") + : G_("%G%qD offset %s is out " + "of the bounds [0, %wu]"), + call, func, rangestr[0], ref.basesize.to_uhwi ()); else - warning_at (loc, OPT_Warray_bounds, - form - ? G_("%G%qD forming offset %s is out of bounds") - : G_("%G%qD offset %s is out of bounds"), - call, func, rangestr[0]); + warned = warning_at (loc, OPT_Warray_bounds, + form + ? G_("%G%qD forming offset %s is out of bounds") + : G_("%G%qD offset %s is out of bounds"), + call, func, rangestr[0]); } else if (TREE_CODE (ref.ref) == MEM_REF) { @@ -1698,24 +1707,25 @@ maybe_diag_offset_bounds (location_t loc, gcall *c type = TREE_TYPE (type); type = TYPE_MAIN_VARIANT (type); - warning_at (loc, OPT_Warray_bounds, - "%G%qD offset %s from the object at %qE is out " - "of the bounds of %qT", - call, func, rangestr[0], ref.base, type); + warned = warning_at (loc, OPT_Warray_bounds, + "%G%qD offset %s from the object at %qE is out " + "of the bounds of %qT", + call, func, rangestr[0], ref.base, type); } else { - type = TYPE_MAIN_VARIANT (TREE_TYPE (ref.ref)); + tree type = TYPE_MAIN_VARIANT (TREE_TYPE (ref.ref)); - warning_at (loc, OPT_Warray_bounds, - "%G%qD offset %s from the object at %qE is out " - "of the bounds of referenced subobject %qD with type %qT " - "at offset %wu", - call, func, rangestr[0], ref.base, TREE_OPERAND (ref.ref, 1), - type, ref.refoff.to_uhwi ()); + warned = warning_at (loc, OPT_Warray_bounds, + "%G%qD offset %s from the object at %qE is out " + "of the bounds of referenced subobject %qD with " + "type %qT at offset %wu", + call, func, rangestr[0], ref.base, + TREE_OPERAND (ref.ref, 1), type, + ref.refoff.to_uhwi ()); } - return true; + return warned; } /* Check a CALL statement for restrict-violations and issue warnings @@ -1840,13 +1850,8 @@ bool check_bounds_or_overlap (gcall *call, tree dst, tree src, tree dstsize, tree srcsize, bool bounds_only /* = false */) { - location_t loc = gimple_location (call); - - if (tree block = gimple_block (call)) - if (location_t *pbloc = block_nonartificial_location (block)) - loc = *pbloc; - + location_t loc = gimple_nonartificial_location (call); loc = expansion_point_location_if_in_system_header (loc); tree func = gimple_call_fndecl (call); Index: gcc/gimple.h =================================================================== --- gcc/gimple.h (revision 260241) +++ gcc/gimple.h (working copy) @@ -1796,7 +1796,20 @@ gimple_has_location (const gimple *g) return LOCATION_LOCUS (gimple_location (g)) != UNKNOWN_LOCATION; } +/* Return non-artificial location information for statement G. */ +static inline location_t +gimple_nonartificial_location (const gimple *g) +{ + location_t *ploc = NULL; + + if (tree block = gimple_block (g)) + ploc = block_nonartificial_location (block); + + return ploc ? *ploc : gimple_location (g); +} + + /* Return the file name of the location of STMT. */ static inline const char * Index: gcc/tree-ssa-strlen.c =================================================================== --- gcc/tree-ssa-strlen.c (revision 259298) +++ gcc/tree-ssa-strlen.c (working copy) @@ -1947,7 +1947,9 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi } } - location_t callloc = gimple_location (stmt); + location_t callloc = gimple_nonartificial_location (stmt); + callloc = expansion_point_location_if_in_system_header (callloc); + tree func = gimple_call_fndecl (stmt); if (lenrange[0] != 0 || !wi::neg_p (lenrange[1])) @@ -2132,7 +2134,8 @@ handle_builtin_stxncpy (built_in_function, gimple_ to strlen(S)). */ strinfo *silen = get_strinfo (pss->first); - location_t callloc = gimple_location (stmt); + location_t callloc = gimple_nonartificial_location (stmt); + callloc = expansion_point_location_if_in_system_header (callloc); tree func = gimple_call_fndecl (stmt); Index: gcc/testsuite/gcc.dg/Wstringop-overflow-5.c =================================================================== --- gcc/testsuite/gcc.dg/Wstringop-overflow-5.c (nonexistent) +++ gcc/testsuite/gcc.dg/Wstringop-overflow-5.c (working copy) @@ -0,0 +1,58 @@ +/* PR tree-optimization/85259 - Missing -Wstringop-overflow= since r256683 + { dg-do compile } + { dg-options "-O2 -Wstringop-overflow" } */ + +extern char* strcpy (char*, const char*); +extern char* strcat (char*, const char*); + +char a1[1]; +char a2[2]; +char a3[3]; +char a4[4]; +char a5[5]; +char a6[6]; +char a7[7]; +char a8[8]; + +/* Verify that at least one instance of -Wstringop-overflow is issued + for each pair of strcpy/strcat calls. */ + +void test_strcpy_strcat_1 (void) +{ + strcpy (a1, "1"), strcat (a1, "2"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_2 (void) +{ + strcpy (a2, "12"), strcat (a2, "3"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_3 (void) +{ + strcpy (a3, "123"), strcat (a3, "4"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_4 (void) +{ + strcpy (a4, "1234"), strcat (a4, "5"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_5 (void) +{ + strcpy (a5, "12345"), strcat (a5, "6"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_6 (void) +{ + strcpy (a6, "123456"), strcat (a6, "7"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_7 (void) +{ + strcpy (a7, "1234567"), strcat (a7, "8"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_8 (void) +{ + strcpy (a8, "12345678"), strcat (a8, "9"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} Index: gcc/testsuite/gcc.dg/Wstringop-overflow-6.c =================================================================== --- gcc/testsuite/gcc.dg/Wstringop-overflow-6.c (nonexistent) +++ gcc/testsuite/gcc.dg/Wstringop-overflow-6.c (working copy) @@ -0,0 +1,59 @@ +/* PR tree-optimization/85259 - Missing -Wstringop-overflow= since r256683 + { dg-do compile } + { dg-options "-O2 -Wstringop-overflow -ftrack-macro-expansion=0" } */ + +#define bos1(p) __builtin_object_size (p, 1) +#define strcat(d, s) __builtin___strcat_chk (d, s, bos1 (d)) +#define strcpy(d, s) __builtin___strcpy_chk (d, s, bos1 (d)) + +char a1[1]; +char a2[2]; +char a3[3]; +char a4[4]; +char a5[5]; +char a6[6]; +char a7[7]; +char a8[8]; + +/* Verify that at least one instance of -Wstringop-overflow is issued + for each pair of strcpy/strcat calls. */ + +void test_strcpy_strcat_1 (void) +{ + strcpy (a1, "1"), strcat (a1, "2"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_2 (void) +{ + strcpy (a2, "12"), strcat (a2, "3"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_3 (void) +{ + strcpy (a3, "123"), strcat (a3, "4"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_4 (void) +{ + strcpy (a4, "1234"), strcat (a4, "5"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_5 (void) +{ + strcpy (a5, "12345"), strcat (a5, "6"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_6 (void) +{ + strcpy (a6, "123456"), strcat (a6, "7"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_7 (void) +{ + strcpy (a7, "1234567"), strcat (a7, "8"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +} + +void test_strcpy_strcat_8 (void) +{ + strcpy (a8, "12345678"), strcat (a8, "9"); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ +}