From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22104 invoked by alias); 14 Aug 2017 23:46:06 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 22014 invoked by uid 89); 14 Aug 2017 23:46:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.7 required=5.0 tests=FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=unavailable version=3.3.2 spammy= X-HELO: mail-qt0-f195.google.com Received: from mail-qt0-f195.google.com (HELO mail-qt0-f195.google.com) (209.85.216.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 14 Aug 2017 23:45:52 +0000 Received: by mail-qt0-f195.google.com with SMTP id u19so11060511qtc.0 for ; Mon, 14 Aug 2017 16:45:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to; bh=lCArycPfz8mPfqGircrlwSu2KThdQ+KAXqIqn05PMH8=; b=hcEjXZweEmQYrlK+js4OA1YMGdqkMwJXlL/hV4juv4ZfqT2aYnQcqrl0qdItINY+f4 9yElfcekjpPRiCrhUBK/aqtaOqxoi+3TIpm75fSw6l8huI9oKFPCdIyvSpVpEM4b1N4i s7iOb2m2zJ94liVYiwjRtC8qnKjY2/GFsCqOuu8K1V0oSaE65eQRRDm0+6lgFL5jni7r GrSpeAimAT4u+uFDuf+aUJCSPgls5JT/14MaSOFa18cbHC/RvoRLGFRLZ2jja35BD9QN Ij1Ly/qUWfU87XRA3YqKJppNhEDwQ0LgX6pNDxSf75114uCiRIpyzSM1WimtU6BCCUoV 6CEA== X-Gm-Message-State: AHYfb5ipKXqPpbz/XD08vYmv/I6brqfH9aUBq245DoWlBcZnMlmO3AXt 7KMkjTjSuCJP9VUE X-Received: by 10.200.39.81 with SMTP id h17mr34045114qth.162.1502750432798; Mon, 14 Aug 2017 15:40:32 -0700 (PDT) Received: from localhost.localdomain (174-16-125-25.hlrn.qwest.net. [174.16.125.25]) by smtp.gmail.com with ESMTPSA id l64sm5493940qkf.95.2017.08.14.15.40.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 14 Aug 2017 15:40:31 -0700 (PDT) Subject: Re: [PATCH 3/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117) To: Jeff Law , Gcc Patch List References: <13944863-99a8-4144-1703-c6e1a2f36425@gmail.com> <0bbc91cd-fcdb-be61-e1d0-4b230f23b1a9@redhat.com> <4f4fbd4c-cb46-b80d-5749-ebb6bb050bc4@gmail.com> From: Martin Sebor Message-ID: <57c7c1f0-4081-6f43-e874-4da3f34f1907@gmail.com> Date: Tue, 15 Aug 2017 03:06:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------186A909103B9426AFE6DC16F" X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00915.txt.bz2 This is a multi-part message in MIME format. --------------186A909103B9426AFE6DC16F Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 5513 On 08/10/2017 01:29 PM, Martin Sebor wrote: >>> diff --git a/gcc/builtins.c b/gcc/builtins.c >>> index 016f68d..1aa9e22 100644 >>> --- a/gcc/builtins.c >>> +++ b/gcc/builtins.c >> [ ... ] >>> + >>> + if (TREE_CODE (type) == ARRAY_TYPE) >>> + { >>> + /* Return the constant size unless it's zero (that's a >>> zero-length >>> + array likely at the end of a struct). */ >>> + tree size = TYPE_SIZE_UNIT (type); >>> + if (size && TREE_CODE (size) == INTEGER_CST >>> + && !integer_zerop (size)) >>> + return size; >>> + } >> Q. Do we have a canonical test for the trailing array idiom? In some >> contexts isn't it size 1? ISTM This test needs slight improvement. >> Ideally we'd use some canonical test for detect the trailing array idiom >> rather than open-coding it here. You might look at the array index >> warnings in tree-vrp.c to see if it's got a canonical test you can call >> or factor and use. > > You're right, there is an API for this (array_at_struct_end_p, > as Richard pointed out). I didn't want to use it because it > treats any array at the end of a struct as a flexible array > member, but simple tests show that that's what -Wstringop- > overflow does now, and it wasn't my intention to tighten up > the checking under this change. It surprises me that no tests > exposed this. Let me relax the check and think about proposing > to tighten it up separately. Done in the attached patch. (I opened bug 81849 for the enhancement to have -Wstringop-overflow diagnose overflows when writing to member arrays bigger than 1 element even if they're last). (I've left the handling for zero size in place because GCC allows global arrays to be declared to have zero elements.) > >>> @@ -3883,6 +3920,30 @@ expand_builtin_strncat (tree exp, rtx) >>> return NULL_RTX; >>> } >>> >>> +/* Helper to check the sizes of sequences and the destination of calls >>> + to __builtin_strncpy (DST, SRC, LEN) and __builtin___strncpy_chk. >>> + Returns true on success (no overflow warning), false otherwise. */ >>> + >>> +static bool >>> +check_strncpy_sizes (tree exp, tree dst, tree src, tree len) >>> +{ >>> + tree dstsize = compute_objsize (dst, warn_stringop_overflow - 1); >>> + >>> + if (!check_sizes (OPT_Wstringop_overflow_, >>> + exp, len, /*maxlen=*/NULL_TREE, src, dstsize)) >>> + return false; >>> + >>> + if (!dstsize || TREE_CODE (len) != INTEGER_CST) >>> + return true; >>> + >>> + if (tree_int_cst_lt (dstsize, len)) >>> + warning_at (EXPR_LOCATION (exp), OPT_Wstringop_truncation, >>> + "%K%qD specified bound %E exceeds destination size %E", >>> + exp, get_callee_fndecl (exp), len, dstsize); >>> + >>> + return true; >> So in the case where you issue the warning, what should the return value >> be? According to the comment it should be false. It looks like you got >> the wrong return value for the tree_int_cst_lt (dstsize, len) test. > > Corrected. The return value is unused by the only caller so > there is no test to exercise it. Done in the attached patch. >>> +/* A helper of handle_builtin_stxncpy. 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[LEN - 1] = '\0'. If a) holds >>> + and b) does not, warn. Otherwise, do nothing. Return true if >>> + diagnostic has been issued. >>> + >>> + The purpose is to diagnose calls to strncpy and stpncpy that do >>> + not nul-terminate the copy while allowing for the idiom where >>> + such a call is immediately followed by setting the last element >>> + to nul, as in: >>> + char a[32]; >>> + strncpy (a, s, sizeof a); >>> + a[sizeof a - 1] = '\0'; >>> +*/ >> So using gsi_next to find the next statement could make the heuristic >> fail to find the a[sizeof a - 1] = '\0'; statement when debugging is >> enabled. >> >> gsi_next_nondebug would be better as it would skip over any debug insns. > > Thanks. I'll have to remember this. I went with this simple approach for now since it worked for GDB. If it turns out that there are important instances of this idiom that rely on intervening statements the warning can be relaxed. >> What might be even better would be to use the immediate uses of the >> memory tag. For your case there should be only one immediate use and it >> should point to the statement which NUL terminates the destination. Or >> maybe that would be worse in that you only want to allow this exception >> when the statements are consecutive. >>> /* Handle a memcpy-like ({mem{,p}cpy,__mem{,p}cpy_chk}) call. >>> If strlen of the second argument is known and length of the third >>> argument >>> is that plus one, strlen of the first argument is the same after >>> this >>> @@ -2512,6 +2789,19 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi) >> You still need to rename strlen_optimize_stmt since after your changes >> it does both optimizations and warnings. > > I'm not sure I understand why. It's a pre-existing function that > just dispatches to the built-in handlers. We don't rename function > callers each time we improve error/warning detection in some > function they call (case in point: all the expanders in builtins.c) > Why do it here? And what would be a suitable name? All that comes > to my mind is awkward variations on strlen_optimize_stmt_and_warn. I've left the function name as is. If you feel strongly that it needs to be renamed let me know. Martin --------------186A909103B9426AFE6DC16F Content-Type: text/x-patch; name="gcc-81117-3.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-81117-3.diff" Content-length: 72718 PR c/81117 - Improve buffer overflow checking in strncpy gcc/ChangeLog: PR c/81117 * builtins.c (compute_objsize): Handle arrays that compute_builtin_object_size likes to fail for. Make extern. * builtins.h (compute_objsize): Declare. (check_strncpy_sizes): New function. (expand_builtin_strncpy): Call check_strncpy_sizes. * gimple-fold.c (gimple_fold_builtin_strncpy): Implement -Wstringop-truncation. (gimple_fold_builtin_strncat): Same. * gimple.c (gimple_build_call_from_tree): Set call location. * tree-ssa-strlen.c (strlen_to_stridx): New global variable. (maybe_diag_bound_equal_length, is_strlen_related_p): New functions. (handle_builtin_stxncpy, handle_builtin_strncat): Same. (handle_builtin_strlen): Use strlen_to_stridx. (strlen_optimize_stmt): Handle flavors of strncat, strncpy, and stpncpy. Use strlen_to_stridx. (pass_strlen::execute): Release strlen_to_stridx. * doc/invoke.texi (-Wsizeof-pointer-memaccess): Document enhancement. (-Wstringop-truncation): Document new option. gcc/c-family/ChangeLog: PR c/81117 * c-warn.c (sizeof_pointer_memaccess_warning): Handle arrays. * c.opt (-Wstriingop-truncation): New option. gcc/testsuite/ChangeLog: PR c/81117 * c-c++-common/Wsizeof-pointer-memaccess3.c: New test. * c-c++-common/Wstringop-overflow.c: Same. * c-c++-common/Wstringop-truncation.c: Same. * c-c++-common/Wstringop-truncation-2.c: Same. * c-c++-common/Wsizeof-pointer-memaccess2.c: Adjust. * c-c++-common/attr-nonstring-2.c: New test. * g++.dg/torture/Wsizeof-pointer-memaccess1.C: Adjust. * g++.dg/torture/Wsizeof-pointer-memaccess2.C: Same. * gcc.dg/torture/pr63554.c: Same. * gcc.dg/Walloca-1.c: Disable macro tracking. Index: gcc/builtins.c =================================================================== --- gcc/builtins.c (revision 251100) +++ gcc/builtins.c (working copy) @@ -3222,18 +3222,56 @@ check_sizes (int opt, tree exp, tree size, tree ma } /* Helper to compute the size of the object referenced by the DEST - expression which must of of pointer type, using Object Size type + expression which must have pointer type, using Object Size type OSTYPE (only the least significant 2 bits are used). Return the size of the object if successful or NULL when the size cannot be determined. */ -static inline tree +tree compute_objsize (tree dest, int ostype) { unsigned HOST_WIDE_INT size; - if (compute_builtin_object_size (dest, ostype & 3, &size)) + + /* Only the two least significant bits are meaningful. */ + ostype &= 3; + + if (compute_builtin_object_size (dest, ostype, &size)) return build_int_cst (sizetype, size); + /* Unless computing the largest size (for memcpy and other raw memory + functions), try to determine the size of the object from its type. */ + if (!ostype) + return NULL_TREE; + + if (TREE_CODE (dest) == SSA_NAME) + { + gimple *stmt = SSA_NAME_DEF_STMT (dest); + if (!is_gimple_assign (stmt)) + return NULL_TREE; + + dest = gimple_assign_rhs1 (stmt); + } + + if (TREE_CODE (dest) != ADDR_EXPR) + return NULL_TREE; + + tree type = TREE_TYPE (dest); + if (TREE_CODE (type) == POINTER_TYPE) + type = TREE_TYPE (type); + + type = TYPE_MAIN_VARIANT (type); + + if (TREE_CODE (type) == ARRAY_TYPE + && !array_at_struct_end_p (dest)) + { + /* Return the constant size unless it's zero (that's a zero-length + array likely at the end of a struct). */ + tree size = TYPE_SIZE_UNIT (type); + if (size && TREE_CODE (size) == INTEGER_CST + && !integer_zerop (size)) + return size; + } + return NULL_TREE; } @@ -3885,6 +3923,32 @@ expand_builtin_strncat (tree exp, rtx) return NULL_RTX; } +/* Helper to check the sizes of sequences and the destination of calls + to __builtin_strncpy (DST, SRC, LEN) and __builtin___strncpy_chk. + Returns true on success (no overflow warning), false otherwise. */ + +static bool +check_strncpy_sizes (tree exp, tree dst, tree src, tree len) +{ + tree dstsize = compute_objsize (dst, warn_stringop_overflow - 1); + + if (!check_sizes (OPT_Wstringop_overflow_, + exp, len, /*maxlen=*/NULL_TREE, src, dstsize)) + return false; + + if (!dstsize || TREE_CODE (len) != INTEGER_CST) + return true; + + if (!tree_int_cst_lt (dstsize, len)) + return true; + + warning_at (EXPR_LOCATION (exp), OPT_Wstringop_truncation, + "%K%qD specified bound %E exceeds destination size %E", + exp, get_callee_fndecl (exp), len, dstsize); + + return false; +} + /* Expand expression EXP, which is a call to the strncpy builtin. Return NULL_RTX if we failed the caller should emit a normal call. */ @@ -3903,17 +3967,8 @@ expand_builtin_strncpy (tree exp, rtx target) /* The length of the source sequence. */ tree slen = c_strlen (src, 1); - if (warn_stringop_overflow) - { - tree destsize = compute_objsize (dest, - warn_stringop_overflow - 1); + check_strncpy_sizes (exp, dest, src, len); - /* The number of bytes to write is LEN but check_sizes will also - check SLEN if LEN's value isn't known. */ - check_sizes (OPT_Wstringop_overflow_, - exp, len, /*maxlen=*/NULL_TREE, src, destsize); - } - /* We must be passed a constant len and src parameter. */ if (!tree_fits_uhwi_p (len) || !slen || !tree_fits_uhwi_p (slen)) return NULL_RTX; Index: gcc/builtins.h =================================================================== --- gcc/builtins.h (revision 251100) +++ gcc/builtins.h (working copy) @@ -89,6 +89,7 @@ extern tree fold_call_stmt (gcall *, bool); extern void set_builtin_user_assembler_name (tree decl, const char *asmspec); extern bool is_simple_builtin (tree); extern bool is_inexpensive_builtin (tree); +extern tree compute_objsize (tree, int); extern bool readonly_data_expr (tree exp); extern bool init_target_chars (void); Index: gcc/c-family/c-warn.c =================================================================== --- gcc/c-family/c-warn.c (revision 251100) +++ gcc/c-family/c-warn.c (working copy) @@ -628,7 +628,8 @@ sizeof_pointer_memaccess_warning (location_t *size || vec_safe_length (params) <= 1) return; - switch (DECL_FUNCTION_CODE (callee)) + enum built_in_function fncode = DECL_FUNCTION_CODE (callee); + switch (fncode) { case BUILT_IN_STRNCMP: case BUILT_IN_STRNCASECMP: @@ -710,9 +711,28 @@ sizeof_pointer_memaccess_warning (location_t *size type = TYPE_P (sizeof_arg[idx]) ? sizeof_arg[idx] : TREE_TYPE (sizeof_arg[idx]); + if (!POINTER_TYPE_P (type)) - return; + { + /* The argument type may be an array. Diagnose bounded string + copy functions that specify the bound in terms of the source + argument rather than the destination. */ + if (strop && !cmp && fncode != BUILT_IN_STRNDUP && src) + { + tem = tree_strip_nop_conversions (src); + if (TREE_CODE (tem) == ADDR_EXPR) + tem = TREE_OPERAND (tem, 0); + if (operand_equal_p (tem, sizeof_arg[idx], OEP_ADDRESS_OF)) + warning_at (sizeof_arg_loc[idx], OPT_Wsizeof_pointer_memaccess, + "argument to % in %qD call is the same " + "expression as the source; did you mean to use " + "the size of the destination?", + callee); + } + return; + } + if (dest && (tem = tree_strip_nop_conversions (dest)) && POINTER_TYPE_P (TREE_TYPE (tem)) Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 251100) +++ gcc/c-family/c.opt (working copy) @@ -736,6 +736,10 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger V Under the control of Object Size type, warn about buffer overflow in string manipulation functions like memcpy and strcpy. +Wstringop-truncation +C ObjC C++ ObjC++ Var(warn_stringop_truncation) Warning Init (1) LangEnabledBy(C ObjC C++ ObjC++, Wall) +Warn about truncation in string manipulation functions like strncat and strncpy. + Wsuggest-attribute=format C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning Warn about functions which might be candidates for format attributes. Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 251100) +++ gcc/doc/invoke.texi (working copy) @@ -312,7 +312,7 @@ Objective-C and Objective-C++ Dialects}. -Wsizeof-pointer-memaccess -Wsizeof-array-argument @gol -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol -Wstrict-aliasing=n -Wstrict-overflow -Wstrict-overflow=@var{n} @gol --Wstringop-overflow=@var{n} @gol +-Wstringop-overflow=@var{n} -Wstringop-truncation @gol -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol -Wsuggest-final-types @gol -Wsuggest-final-methods -Wsuggest-override @gol -Wmissing-format-attribute -Wsubobject-linkage @gol @@ -5171,6 +5171,56 @@ whether to issue a warning. Similarly to @option{ setting of the option may result in warnings for benign code. @end table +@item -Wstringop-truncation +@opindex Wstringop-truncation +@opindex Wno-stringop-truncation +Warn for calls to bounded string manipulation functions such as @code{strncat}, +@code{strncpy}, and @code{stpncpy} that may either truncate the copied string +or leave the destination unchanged. + +In the following example, the call to @code{strncat} specifies a bound that +is less than the length of the source string. As a result, the copy of +the source will be truncated and so the call is diagnosed. To avoid the +warning use @code{bufsize - strlen (buf) - 1)} as the bound. + +@smallexample +void append (char *buf, size_t bufsize) +@{ + strncat (buf, ".txt", 3); +@} +@end smallexample + +As another example, the following call to @code{strncpy} results in copying +to @code{d} just the characters preceding the terminating NUL, without +appending the NUL to the end. Assuming the result of @code{strncpy} is +necessarily a NUL-terminated string is a common mistake, and so the call +is diagnosed. To avoid the warning when the result is not expected to be +NUL-terminated, call @code{memcpy} instead. + +@smallexample +void copy (char *d, const char *s) +@{ + strncpy (d, s, strlen (s)); +@} +@end smallexample + +In the following example, the call to @code{strncpy} specifies the size +of the destination buffer as the bound. If the length of the source +string is equal to or greater than this size the result of the copy will +not be NUL-terminated. Therefore, the call is also diagnosed. To avoid +the warning, specify @code{sizeof buf - 1} as the bound and set the last +element of the buffer to @code{NUL}. + +@smallexample +void copy (const char *s) +@{ + char buf[80]; + strncpy (buf, s, sizeof buf); + @dots{} +@} +@end smallexample + + @item -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @opindex Wsuggest-attribute= @opindex Wno-suggest-attribute= @@ -6155,12 +6205,27 @@ not an array, but a pointer. This warning is enab @opindex Wsizeof-pointer-memaccess @opindex Wno-sizeof-pointer-memaccess Warn for suspicious length parameters to certain string and memory built-in -functions if the argument uses @code{sizeof}. This warning warns e.g.@: -about @code{memset (ptr, 0, sizeof (ptr));} if @code{ptr} is not an array, -but a pointer, and suggests a possible fix, or about -@code{memcpy (&foo, ptr, sizeof (&foo));}. This warning is enabled by -@option{-Wall}. +functions if the argument uses @code{sizeof}. This warning triggers for +example for @code{memset (ptr, 0, sizeof (ptr));} if @code{ptr} is not +an array, but a pointer, and suggests a possible fix, or about +@code{memcpy (&foo, ptr, sizeof (&foo));}. @option{-Wsizeof-pointer-memaccess} +also warns about calls to bounded string copy functions like @code{strncat} +or @code{strncpy} that specify as the bound a @code{sizeof} expression of +the source array. For example, in the following function the call to +@code{strncat} specifies the size of the source string as the bound. That +is almost certainly a mistake and so the call is diagnosed. +@smallexample +void make_file (const char *name) +@{ + char path[PATH_MAX]; + strncpy (path, name, sizeof path - 1); + strncat (path, ".text", sizeof ".text"); + @dots{} +@} +@end smallexample +The @option{-Wsizeof-pointer-memaccess} option is enabled by @option{-Wall}. + @item -Wsizeof-array-argument @opindex Wsizeof-array-argument @opindex Wno-sizeof-array-argument Index: gcc/gimple-fold.c =================================================================== --- gcc/gimple-fold.c (revision 251100) +++ gcc/gimple-fold.c (working copy) @@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see #include "gimple-iterator.h" #include "tree-into-ssa.h" #include "tree-dfa.h" +#include "tree-object-size.h" #include "tree-ssa.h" #include "tree-ssa-propagate.h" #include "ipa-utils.h" @@ -59,6 +60,8 @@ along with GCC; see the file COPYING3. If not see #include "stringpool.h" #include "attribs.h" #include "asan.h" +#include "diagnostic-core.h" +#include "intl.h" /* Return true when DECL can be referenced from current unit. FROM_DECL (if non-null) specify constructor of variable DECL was taken from. @@ -1524,12 +1527,28 @@ static bool gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, tree dest, tree src, tree len) { - location_t loc = gimple_location (gsi_stmt (*gsi)); - tree fn; + gimple *stmt = gsi_stmt (*gsi); + location_t loc = gimple_location (stmt); /* If the LEN parameter is zero, return DEST. */ if (integer_zerop (len)) { + tree fndecl = gimple_call_fndecl (stmt); + gcall *call = as_a (stmt); + + /* Warn about the lack of nul termination: the result is not + a (nul-terminated) string. */ + tree slen = get_maxval_strlen (src, 0); + if (slen && !integer_zerop (slen)) + warning_at (loc, OPT_Wstringop_truncation, + "%G%qD destination unchanged after copying no bytes " + "from a string of length %E", + call, fndecl, slen); + else + warning_at (loc, OPT_Wstringop_truncation, + "%G%qD destination unchanged after copying no bytes", + call, fndecl); + replace_call_with_value (gsi, dest); return true; } @@ -1544,16 +1563,66 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator if (!slen || TREE_CODE (slen) != INTEGER_CST) return false; - slen = size_binop_loc (loc, PLUS_EXPR, slen, ssize_int (1)); + /* The size of the source string including the terminating nul. */ + tree ssize = size_binop_loc (loc, PLUS_EXPR, slen, ssize_int (1)); /* We do not support simplification of this case, though we do support it when expanding trees into RTL. */ /* FIXME: generate a call to __builtin_memset. */ - if (tree_int_cst_lt (slen, len)) + if (tree_int_cst_lt (ssize, len)) return false; + if (tree_int_cst_lt (len, slen)) + { + tree fndecl = gimple_call_fndecl (stmt); + gcall *call = as_a (stmt); + + warning_at (loc, OPT_Wstringop_truncation, + (tree_int_cst_equal (size_one_node, len) + ? G_("%G%qD output truncated copying %E byte " + "from a string of length %E") + : G_("%G%qD output truncated copying %E bytes " + "from a string of length %E")), + call, fndecl, len, slen); + } + else if (tree_int_cst_equal (len, slen)) + { + tree decl = dest; + if (TREE_CODE (decl) == SSA_NAME) + { + gimple *def_stmt = SSA_NAME_DEF_STMT (decl); + if (is_gimple_assign (def_stmt)) + { + tree_code code = gimple_assign_rhs_code (def_stmt); + if (code == ADDR_EXPR || code == VAR_DECL) + decl = gimple_assign_rhs1 (def_stmt); + } + } + + if (TREE_CODE (decl) == ADDR_EXPR) + decl = TREE_OPERAND (decl, 0); + + if (TREE_CODE (decl) == COMPONENT_REF) + decl = TREE_OPERAND (decl, 1); + + tree fndecl = gimple_call_fndecl (stmt); + gcall *call = as_a (stmt); + + if (!DECL_P (decl) + || !lookup_attribute ("nonstring", DECL_ATTRIBUTES (decl))) + warning_at (loc, OPT_Wstringop_truncation, + (tree_int_cst_equal (size_one_node, len) + ? G_("%G%qD output truncated before terminating nul " + "copying %E byte from a string of the same " + "length") + : G_("%G%qD output truncated before terminating nul " + "copying %E bytes from a string of the same " + "length")), + call, fndecl, len); + } + /* OK transform into builtin memcpy. */ - fn = builtin_decl_implicit (BUILT_IN_MEMCPY); + tree fn = builtin_decl_implicit (BUILT_IN_MEMCPY); if (!fn) return false; @@ -1562,6 +1631,7 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator NULL_TREE, true, GSI_SAME_STMT); gimple *repl = gimple_build_call (fn, 3, dest, src, len); replace_call_with_call_and_fold (gsi, repl); + return true; } @@ -1858,21 +1928,77 @@ gimple_fold_builtin_strncat (gimple_stmt_iterator return true; } - /* If the requested len is greater than or equal to the string - length, call strcat. */ - if (TREE_CODE (len) == INTEGER_CST && p - && compare_tree_int (len, strlen (p)) >= 0) + if (TREE_CODE (len) == INTEGER_CST && p) { - tree fn = builtin_decl_implicit (BUILT_IN_STRCAT); + unsigned srclen = strlen (p); - /* If the replacement _DECL isn't initialized, don't do the - transformation. */ - if (!fn) - return false; + int cmpsrc = compare_tree_int (len, srclen); - gcall *repl = gimple_build_call (fn, 2, dst, src); - replace_call_with_call_and_fold (gsi, repl); - return true; + unsigned HOST_WIDE_INT dstsize; + + bool nowarn = gimple_no_warning_p (stmt); + + if (!nowarn && compute_builtin_object_size (dst, 1, &dstsize)) + { + 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_("%G%qD specified bound %E equals " + "destination size") + : G_("%G%qD specified bound %E exceeds " + "destination size %wu"), + stmt, fndecl, len, dstsize); + if (nowarn) + gimple_set_no_warning (stmt, true); + } + } + + if (!nowarn && cmpsrc <= 0) + { + tree fndecl = gimple_call_fndecl (stmt); + + /* To avoid certain truncation the specified bound should also + not be equal to (or less than) the length of the source. */ + location_t loc = gimple_location (stmt); + bool warn; + if (cmpsrc == 0) + warn = warning_at (loc, OPT_Wstringop_overflow_, + "%G%qD specified bound %E equals source length", + stmt, fndecl, len); + else + warn = warning_at (loc, OPT_Wstringop_truncation, + "%G%qD specified bound %E is less than source " + "length %u", + stmt, fndecl, len, srclen); + if (warn) + gimple_set_no_warning (stmt, true); + } + + /* If the requested len is greater than or equal to the string + length, call strcat. */ + if (cmpsrc >= 0) + { + tree fn = builtin_decl_implicit (BUILT_IN_STRCAT); + + /* If the replacement _DECL isn't initialized, don't do the + transformation. */ + if (!fn) + return false; + + gcall *repl = gimple_build_call (fn, 2, dst, src); + replace_call_with_call_and_fold (gsi, repl); + return true; + } } return false; Index: gcc/gimple.c =================================================================== --- gcc/gimple.c (revision 251100) +++ gcc/gimple.c (working copy) @@ -361,6 +361,7 @@ gimple_build_call_from_tree (tree t) gimple_call_set_arg (call, i, CALL_EXPR_ARG (t, i)); gimple_set_block (call, TREE_BLOCK (t)); + gimple_set_location (call, EXPR_LOCATION (t)); /* Carry all the CALL_EXPR flags to the new GIMPLE_CALL. */ gimple_call_set_chain (call, CALL_EXPR_STATIC_CHAIN (t)); Index: gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c =================================================================== --- gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c (revision 251100) +++ gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c (working copy) @@ -1,7 +1,7 @@ /* Test -Wsizeof-pointer-memaccess warnings. */ /* { dg-do compile } */ -/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -ftrack-macro-expansion=0" } */ -/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -Wno-c++-compat -ftrack-macro-expansion=0" {target c} } */ +/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -Wno-stringop-truncation -ftrack-macro-expansion=0" } */ +/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -Wno-stringop-truncation -Wno-c++-compat -ftrack-macro-expansion=0" {target c} } */ /* { dg-require-effective-target alloca } */ #define bos(ptr) __builtin_object_size (ptr, 1) @@ -473,12 +473,15 @@ f4 (char *x, char **y, int z, char w[64]) strncat (w, s2, sizeof (w)); /* { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } */ stpncpy (w, s1, sizeof (w)); /* { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } */ - /* These are correct, no warning. */ + /* These are pointless when the destination is large enough, and + cause overflow otherwise. If the copies are guaranteed to be + safe the calls might as well be replaced by strcat(), strcpy(), + or memcpy(). */ const char s3[] = "foobarbaz"; const char s4[] = "abcde12345678"; - strncpy (x, s3, sizeof (s3)); - strncat (x, s4, sizeof (s4)); - stpncpy (x, s3, sizeof (s3)); + strncpy (x, s3, sizeof (s3)); /* { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } */ + strncat (x, s4, sizeof (s4)); /* { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } */ + stpncpy (x, s3, sizeof (s3)); /* { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } */ } /* { dg-prune-output "\[\n\r\]*writing\[\n\r\]*" } */ Index: gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c =================================================================== --- gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c (nonexistent) +++ gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c (working copy) @@ -0,0 +1,132 @@ +/* Test -Wsizeof-pointer-memaccess warnings. */ +/* { dg-do compile } */ +/* { dg-options "-Wsizeof-pointer-memaccess -Wno-stringop-overflow -Wno-stringop-truncation -ftrack-macro-expansion=0" } */ + +#define bos(ptr) __builtin_object_size (ptr, 1) +#define bos0(ptr) __builtin_object_size (ptr, 0) + +#define memset(dst, val, sz) \ + (FUNC (memset, dst, val, sz, bos (dst)), sink ((dst))) + +#define memcpy(dst, src, sz) \ + (FUNC (memcpy, dst, src, sz, bos (dst)), sink ((dst))) + +#define memmove(dst, src, sz) \ + (FUNC (memmove, dst, src, sz, bos (dst)), sink ((dst))) + +#define mempcpy(dst, src, sz) \ + (FUNC (mempcpy, dst, src, sz, bos (dst)), sink ((dst))) + +#define strncpy(dst, src, sz) \ + (FUNC (strncpy, dst, src, sz, bos (dst)), sink (dst)) + +#define strncat(dst, src, sz) \ + (FUNC (strncat, dst, src, sz, bos (dst)), sink (dst)) + +#define stpncpy(dst, src, sz) \ + (FUNC (stpncpy, dst, src, sz, bos (dst)), sink (dst)) + +void sink (void*); + +#define S10 "123456789" +extern char a10[10]; + +void test_string_literal (char *dst) +{ +#define FUNC(f, d, s, n, x) __builtin_ ## f (d, s, n) + + /* It's common to call memcpy and other raw memory functions with + size drerived from the source argument. Verify that no warning + is ussued for such calls. */ + memcpy (dst, S10, sizeof S10); + mempcpy (dst, S10, sizeof S10); + memmove (dst, S10, sizeof S10); + + memset (dst, 0, sizeof S10); + + stpncpy (dst, S10, sizeof S10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncpy (dst, S10, sizeof S10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncat (dst, S10, sizeof S10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + /* Unlike in the cases above, even though the calls below are likely + wrong, it's not easy to detect that the expression (sizeof X - 1) + involves sizeof of the source, so no warning is issued here, as + helpful as one might be. Whether -Wstringop-truncation is issued + is tested elsewhere. */ + stpncpy (dst, S10, sizeof S10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncpy (dst, S10, sizeof S10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncat (dst, S10, sizeof S10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ +} + + +void test_char_array (char *dst) +{ + memcpy (dst, a10, sizeof a10); + mempcpy (dst, a10, sizeof a10); + memmove (dst, a10, sizeof a10); + + memset (dst, 0, sizeof a10); + + stpncpy (dst, a10, sizeof a10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncpy (dst, a10, sizeof a10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncat (dst, a10, sizeof a10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + stpncpy (dst, a10, sizeof a10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncpy (dst, a10, sizeof a10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncat (dst, a10, sizeof a10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ +} + + +#undef FUNC +#define FUNC(f, d, s, n, os) __builtin___ ## f ## _chk (d, s, n, os) + +void test_char_array_chk (char *dst) +{ + memcpy (dst, S10, sizeof S10); + mempcpy (dst, S10, sizeof S10); + memmove (dst, S10, sizeof S10); + + memset (dst, 0, sizeof S10); + + stpncpy (dst, S10, sizeof S10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncpy (dst, S10, sizeof S10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncat (dst, S10, sizeof S10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + stpncpy (dst, S10, sizeof S10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncpy (dst, S10, sizeof S10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncat (dst, S10, sizeof S10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ +} + + +void test_string_literal_chk (char *dst) +{ + memcpy (dst, a10, sizeof a10); + mempcpy (dst, a10, sizeof a10); + memmove (dst, a10, sizeof a10); + + memset (dst, 0, sizeof a10); + + stpncpy (dst, a10, sizeof a10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncpy (dst, a10, sizeof a10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + strncat (dst, a10, sizeof a10); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" } */ + + stpncpy (dst, a10, sizeof a10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncpy (dst, a10, sizeof a10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ + + strncat (dst, a10, sizeof a10 - 1); /* { dg-warning "\\\[-Wsizeof-pointer-memaccess]" "" { xfail *-*-* } } */ +} Index: gcc/testsuite/c-c++-common/Wstringop-overflow.c =================================================================== --- gcc/testsuite/c-c++-common/Wstringop-overflow.c (nonexistent) +++ gcc/testsuite/c-c++-common/Wstringop-overflow.c (working copy) @@ -0,0 +1,158 @@ +/* PR middle-end/81117 - Improve buffer overflow checking in strncpy + { dg-do compile } + { dg-options "-O2 -Wstringop-overflow -Wno-stringop-truncation -ftrack-macro-expansion=0" } */ + +typedef __SIZE_TYPE__ size_t; + +#if __cplusplus +extern "C" { +#endif + +size_t strlen (const char*); +char* strncat (char*, const char*, size_t); +char* strncpy (char*, const char*, size_t); +#if __cplusplus +} +#endif + +const char ar[] = "123"; + +void test_strncat (char **d, const char* s, int i) +{ + /* Use a fresh pointer for each test to prevent the optimizer from + eliminating redundant writes into the same destination. Avoid + calling functions like sink() on the result that would have to + be assumed to change the source string by the alias oracle. */ +#define T(d, s, len) strncat (*d++, (s), (len)) + + T (d, "", 0); + T (d, "", 1); + T (d, "", 2); + T (d, "", 3); + T (d, "123", 0); + /* The following two calls truncate the copy and are diagnosed + by -Wstringop-truncation but there is evidence of overflow so + they're not diagnosed by -Wstringop-overflow. */ + T (d, "123", 1); + T (d, "123", 2); + + T (d, "123", 3); /* { dg-warning ".strncat\[^\n\r\]* specified bound 3 equals source length" } */ + T (d, "123", 4); + T (d, "123", 9); + + T (d, s, strlen (s)); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + T (d, s, strlen (s) + 1); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + /* The following could also be diagnosed by -Wstringop-truncation + (with some effort to distinguish the pattern from others like + the one above. */ + T (d, s, strlen (s) - 1); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + T (d, s, strlen (s) - i); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + + /* The following is dubious but not necessarily a smoking gun. */ + T (d, s, strlen (s) - strlen (s)); + + { + signed char n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + } + + { + short n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + } + + { + int n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + } + + { + unsigned n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + } + + { + size_t n; + n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + } + + { + size_t n; + n = strlen (s) - 1; /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-message ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + } + + { + /* This doesn't overflow so iit should not be diagnosed. */ + size_t n = strlen (s) - strlen (s); + T (d, s, n); + } + + { + size_t n = i < strlen (s) ? i : strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-message ".strncat\[^\n\r\]* specified bound depends on the length of the source argument" } */ + } +} + + +void test_strncpy (char **d, const char* s, int i) +{ +#undef T +#define T(d, s, len) strncpy (*d++, (s), (len)) + + T (d, "", 0); + T (d, "", 1); + T (d, "", 2); + T (d, "", 3); + T (d, "123", 0); + T (d, "123", 1); + T (d, "123", 2); + T (d, "123", 3); + T (d, "123", 4); + T (d, "123", 9); + + T (d, "123", sizeof "123"); + T (d, ar, sizeof ar); + + T (d, s, strlen (s)); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ + + { + int n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ + } + + { + unsigned n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ + } + + { + size_t n; + n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ + } + + { + size_t n; + n = strlen (s) - 1; /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */ + } + + { + /* This is diagnosed by -Wstringop-truncation. Verify that it isn't + also diagnosed by -Wstringop-overflow. */ + size_t n = strlen (s) - strlen (s); + T (d, s, n); + } + + { + /* This use of strncpy is certainly dubious and it could well be + diagnosed by -Wstringop-truncation but it isn't. That it is + diagnosed with -Wstringop-overflow is more by accident than + by design. -Wstringop-overflow considers any dependency of + the bound on strlen(s) a potential bug. */ + size_t n = i < strlen (s) ? i : strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-message ".strncpy\[^\n\r]* specified bound depends on the length of the source argument" } */ + } +} Index: gcc/testsuite/c-c++-common/Wstringop-truncation.c =================================================================== --- gcc/testsuite/c-c++-common/Wstringop-truncation.c (nonexistent) +++ gcc/testsuite/c-c++-common/Wstringop-truncation.c (working copy) @@ -0,0 +1,378 @@ +/* PR middle-end/81117 - Improve buffer overflow checking in strncpy + { dg-do compile } + { dg-options "-O2 -Wstringop-truncation -Wno-stringop-overflow -ftrack-macro-expansion=0" } */ + + +typedef __SIZE_TYPE__ size_t; + +#if __cplusplus +extern "C" { +#endif + +size_t strlen (const char*); +char* strncat (char*, const char*, size_t); +char* strncpy (char*, const char*, size_t); + +#if __cplusplus +} +#endif + +void sink (void*); + +#define S4 "123" +const char a4[] = "123"; + + +typedef struct Dest +{ + char a5[5]; + char b7[7]; + char c3ns[3] __attribute__ ((nonstring)); +} Dest; + +char dst7[7]; +char dst2_5[2][5]; + +/* Verify strncat warnings for arrays of known bounds. */ + +void test_strncat_array (Dest *pd) +{ +#undef T +#define T(d, s, len) (strncat ((d), (s), (len)), sink (d)) + + T (dst7, S4, 2); /* { dg-warning "specified bound 2 is less than source length 3" } */ + + T (dst7, a4, 1); /* { dg-warning "specified bound 1 is less than source length 3" } */ + + /* There is no truncation here but possible overflow so these + are diagnosed by -Wstringop-overflow: + T (dst7, S4, 3); + T (dst7, a4, 3); + */ + + T (pd->a5, S4, 2); /* { dg-warning "specified bound 2 is less than source length" } */ + T (pd->a5, S4, 1); /* { dg-warning "specified bound 1 is less than source length 3" } */ +} + +/* Verify strncat warnings for arrays of unknown bounds. */ + +void test_strncat_vla (char *d, unsigned n) +{ + T (d, S4, 2); /* { dg-warning "specified bound 2 is less than source length 3" } */ + T (d, S4, 4); + + T (d, a4, 2); /* { dg-warning "specified bound 2 is less than source length 3" } */ + + /* There is no truncation here but possible overflow so these + are diagnosed by -Wstringop-overflow: + T (d, S4, 3); + T (d, a4, 3); + */ + T (d, a4, 4); + + char vla[n]; + + T (vla, S4, 2); /* { dg-warning "specified bound 2 is less than source length 3" } */ + + T (vla, S4, 4); + T (vla, S4, n); + + T (vla, a4, 2); /* { dg-warning "specified bound 2 is less than source length 3" } */ + + T (vla, a4, 4); + T (vla, a4, n); + + T (d, vla, 1); + T (d, vla, 3); + T (d, vla, 4); + T (d, vla, n); + + /* There is no truncation here but possible overflow so these + are diagnosed by -Wstringop-overflow: + T (vla, S4, 3); + T (vla, a4, 3); + */ +} + +/* Verify strncpy warnings for pointers to unknown strings. */ + +void test_strncpy_ptr (char *d, const char* s, int i) +{ +#undef T +#define T(d, s, len) (strncpy ((d), (s), (len)), sink (d)) + + /* Strncpy doesn't nul-terminate so the following is diagnosed. */ + T (d, "", 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + + /* This is safe. */ + T (d, "", 1); + T (d, "", 2); + + /* Truncation. */ + T (d, "123", 3); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + T (d, "123", 4); + T (d, "123", 9); + + T (d, S4, sizeof S4); /* Covered by -Wsizeof-pointer-memaccess. */ + T (d, S4, sizeof S4 - 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + + T (d, a4, sizeof a4); /* Covered by -Wsizeof-pointer-memaccess. */ + T (d, a4, sizeof a4 - 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + T (d, a4, sizeof a4 - 3); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 3" } */ + T (d, a4, sizeof a4 - 4); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes from a string of length 3" } */ + + T (d, S4, strlen (S4)); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + /* Likely buggy but no truncation. Diagnosed by -Wstringop-overflow. */ + T (d, a4, strlen (a4) + 1); + T (d, S4, strlen (S4) + i); + + T (d, a4, strlen (a4)); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + /* As above, buggy but no evidence of truncation. */ + T (d, S4, strlen (S4) + 1); + + { + signed char n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */ + } + + { + short n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */ + } + + { + int n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */ + } + + { + unsigned n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */ + } + + { + size_t n; + n = strlen (s); /* { dg-message "length computed here" } */ + T (d, s, n); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */ + } + + { + size_t n; + char *dp2 = d + 1; + n = strlen (s); /* { dg-message "length computed here" } */ + T (dp2, s, n); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */ + } + + { + /* The following is likely buggy but there's no apparent truncation + so it's not diagnosed by -Wstringop-truncation. Instead, it is + diagnosed by -Wstringop-overflow (tested elsewhere). */ + int n; + n = strlen (s) - 1; + T (d, s, n); + } + + { + /* Same as above. */ + size_t n; + n = strlen (s) - 1; + T (d, s, n); + } + + { + size_t n = strlen (s) - strlen (s); + T (d, s, n); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + } + + { + /* This use of strncpy is dubious but it's probably not worth + worrying about (truncation may not actually take place when + i is the result). It is diagnosed with -Wstringop-overflow + (although more by accident than by design). + + size_t n = i < strlen (s) ? i : strlen (s); + T (d, s, n); + */ + } +} + + +/* Verify strncpy warnings for arrays of known bounds. */ + +void test_strncpy_array (Dest *pd, const char* s) +{ +#undef T +#define T(d, s, len) (strncpy ((d), (s), (len)), sink (d)) + + T (dst7, s, 7); /* { dg-warning "specified bound 7 equals destination size" } */ + T (dst7, s, sizeof dst7); /* { dg-warning "specified bound 7 equals destination size" } */ + + T (dst2_5[0], s, sizeof dst2_5[0]); /* { dg-warning "specified bound 5 equals destination size" "bug 77293" { xfail *-*-* } } */ + T (dst2_5[1], s, sizeof dst2_5[1]); /* { dg-warning "specified bound 5 equals destination size" } */ + + /* Verify that copies that nul-terminate are not diagnosed. */ + T (dst7, "", sizeof dst7); + T (dst7 + 6, "", sizeof dst7 - 6); + T (dst7, "1", sizeof dst7); + T (dst7 + 1, "1", sizeof dst7 - 1); + T (dst7, "123456", sizeof dst7); + T (dst7 + 1, "12345", sizeof dst7 - 1); + + /* No nul-termination here. */ + T (dst7 + 2, "12345", sizeof dst7 - 2); /* { dg-warning "output truncated before terminating nul copying 5 bytes from a string of the same length" } */ + + /* Because strnlen appends as many NULs as necessary to write the specified + number of byts the following doesn't (necessarily) truncate but rather + overflow, and so is diagnosed by -Wstringop-overflow. */ + T (dst7, s, 8); + + T (dst7 + 1, s, 6); /* { dg-warning "specified bound 6 equals destination size" } */ + T (dst7 + 6, s, 1); /* { dg-warning "specified bound 1 equals destination size" } */ + + T (pd->a5, s, 5); /* { dg-warning "specified bound 5 equals destination size" } */ + T (pd->a5, s, sizeof pd->a5); /* { dg-warning "specified bound 5 equals destination size" } */ + + /* Verify that a copy that nul-terminates is not diagnosed. */ + T (pd->a5, "1234", sizeof pd->a5); + + /* Same above, diagnosed by -Wstringop-overflow. */ + T (pd->a5, s, 6); + + /* Exercise destination with attribute "nonstring". */ + T (pd->c3ns, "", 3); + T (pd->c3ns, "", 1); + /* Truncation is still diagnosed -- using strncpy in this case is + pointless and should be replaced with memcpy. */ + T (pd->c3ns, "12", 1); /* { dg-warning "output truncated copying 1 byte from a string of length 2" } */ + T (pd->c3ns, "12", 2); + T (pd->c3ns, "12", 3); + T (pd->c3ns, "123", 3); + T (pd->c3ns, s, 3); + T (pd->c3ns, s, sizeof pd->c3ns); + + /* Verify that the idiom of calling strncpy with a bound equal to + the size of the destination (and thus potentially without NUL- + terminating it) immediately followed by setting the last element + of the array to NUL is not diagnosed. */ + { + /* This might be better written using memcpy() but it's safe so + it probably shouldn't be diagnosed. It currently triggers + a warning because of bug 81704. */ + strncpy (dst7, "0123456", sizeof dst7); /* { dg-bogus "truncated" "bug 81704" { xfail *-*-* } } */ + dst7[sizeof dst7 - 1] = '\0'; + sink (dst7); + } + + { + const char a[] = "0123456789"; + strncpy (dst7, a, sizeof dst7); + dst7[sizeof dst7 - 1] = '\0'; + sink (dst7); + } + + { + strncpy (dst7, s, sizeof dst7); + dst7[sizeof dst7 - 1] = '\0'; + sink (dst7); + } + + { + strncpy (pd->a5, "01234", sizeof pd->a5); /* { dg-bogus "truncated" "bug 81704" { xfail *-*-* } } */ + pd->a5[sizeof pd->a5 - 1] = '\0'; + sink (pd); + } + + { + strncpy (pd->a5, s, sizeof pd->a5); + pd->a5[sizeof pd->a5 - 1] = '\0'; + sink (pd); + } + + { + unsigned n = 7; + char *p = (char*)__builtin_malloc (n); + strncpy (p, s, n); + p[n - 1] = '\0'; + sink (p); + } + + { + /* This should be diagnosed because the NUL-termination doesn't + immediately follow the strncpy call (sink may expect pd->a5 + to be NUL-terminated). */ + strncpy (pd->a5, s, sizeof pd->a5); /* { dg-warning "specified bound 5 equals destination size" } */ + sink (pd); + pd->a5[sizeof pd->a5] = '\0'; + sink (pd); + } +} + +typedef struct Flex +{ + size_t n; + char a0[0]; + char ax[]; +} Flex; + +extern char array[]; + +/* Verify that no warning is issued for array of unknown bound, flexible + array members, or zero-length arrays, except when the source is definitely + truncated. */ + +void test_strncpy_flexarray (Flex *pf, const char* s) +{ +#undef T +#define T(d, s, len) (strncpy ((d), (s), (len)), sink (d)) + + T (array, "12345", 7); + T (array, "12345", 123); + + T (array, s, 7); + T (array, s, 123); + + T (pf->a0, s, 1); + T (pf->a0, s, 1234); + + T (pf->a0, "", 1); + T (pf->a0, "12345", 5); /* { dg-warning "output truncated before terminating nul copying 5 bytes from a string of the same length" } */ + T (pf->a0, "12345", 1234); + + T (pf->ax, s, 5); + T (pf->ax, s, 12345); + + T (pf->ax, "1234", 5); + T (pf->ax, "12345", 5); /* { dg-warning "output truncated before terminating nul copying 5 bytes from a string of the same length" } */ + T (pf->ax, "12345", 12345); +} + +/* Verify warnings for dynamically allocated objects. */ + +void test_strncpy_alloc (const char* s) +{ + size_t n = 7; + char *d = (char *)__builtin_malloc (n); + + T (d, s, n); /* { dg-warning "specified bound 7 equals destination size" "bug 79016" { xfail *-*-* } } */ + + Dest *pd = (Dest *)__builtin_malloc (sizeof *pd * n); + T (pd->a5, s, 5); /* { dg-warning "specified bound 5 equals destination size" } */ + T (pd->a5, s, sizeof pd->a5); /* { dg-warning "specified bound 5 equals destination size" } */ +} + +/* Verify warnings for VLAs. */ + +void test_strncpy_vla (unsigned n, const char* s) +{ + char vla[n]; + T (vla, s, 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + + T (vla, s, 1); + T (vla, s, 2); + T (vla, s, n); + + T (vla, "", 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + T (vla, "", 1); + T (vla, S4, 3); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + T (vla, S4, n); +} Index: gcc/testsuite/c-c++-common/Wstringop-truncation-2.c =================================================================== --- gcc/testsuite/c-c++-common/Wstringop-truncation-2.c (nonexistent) +++ gcc/testsuite/c-c++-common/Wstringop-truncation-2.c (working copy) @@ -0,0 +1,22 @@ +/* PR middle-end/81117 - Improve buffer overflow checking in strncpy + { dg-do compile } + { dg-options "-O2 -Wstringop-truncation -g" } */ + +char dst[4]; + +int test_debug_stmt (const char *src) +{ + int ret; + + __builtin_strncpy (dst, src, sizeof dst); + + /* The following assignment is preserved only in the form of a debug + statement but not actual executable code. */ + ret = 123; + + /* Verify that an intervening debug statement doesn't cause the warning + to trigger. */ + dst[sizeof dst - 1] = '\0'; + + return ret; +} Index: gcc/testsuite/c-c++-common/attr-nonstring-1.c =================================================================== --- gcc/testsuite/c-c++-common/attr-nonstring-1.c (nonexistent) +++ gcc/testsuite/c-c++-common/attr-nonstring-1.c (working copy) @@ -0,0 +1,60 @@ +/* Test to exercise attribute "nonstring" syntax. + { dg-do compile } + { dg-options "-Wattributes" } */ + +#define ATTR(list) __attribute__ (list) +#define NONSTR ATTR ((nonstring)) + +/* Verify it's accepted on char arrays. */ +extern NONSTR char nsx_1[]; +extern char NONSTR nsx_2[]; +extern char nsx_3[] NONSTR; + +extern NONSTR char ns1[1]; +extern char NONSTR ns3[3]; +extern char ns5[5] NONSTR; + +/* Verify it's accepted on char pointers. */ +extern NONSTR char* pns_1; +extern char NONSTR* pns_2; +extern char* NONSTR pns_3; + +struct S +{ +/* Verify it's accepted on char member pointers. */ + NONSTR char* mpns_1; + char NONSTR* mpns_2; + char* NONSTR mpns_3; + +/* Verify it's accepted on char member arrays. */ + NONSTR char mns1[1]; + char NONSTR mns3[3]; + char mns5[5] NONSTR; + +/* Verify it's accepted on char flexible array members. */ + char mnsx[] NONSTR; +}; + +/* Verify it's rejected on non-array and non-pointer objects. */ +extern NONSTR char c1; /* { dg-warning ".nonstring. attribute ignored on objects of type .char." } */ + +extern NONSTR int i1; /* { dg-warning ".nonstring. attribute ignored on objects of type .int." } */ + +extern NONSTR int ia1[]; /* { dg-warning ".nonstring. attribute ignored on objects of type .int *\\\[\\\]." } */ + +extern NONSTR int* pi1; /* { dg-warning ".nonstring. attribute ignored on objects of type .int *\\*." } */ + +extern NONSTR +void f (void); /* { dg-warning ".nonstring. attribute does not apply to functions" } */ + +struct NONSTR +NonStrType { int i; }; /* { dg-warning ".nonstring. attribute does not apply to types" } */ + +typedef char NONSTR nschar_t; /* { dg-warning ".nonstring. attribute does not apply to types" } */ + +void func (NONSTR char *pns1, char NONSTR *pns2, char* NONSTR pns3) +{ + (void)pns1; + (void)pns2; + (void)pns3; +} Index: gcc/testsuite/c-c++-common/attr-nonstring-2.c =================================================================== --- gcc/testsuite/c-c++-common/attr-nonstring-2.c (nonexistent) +++ gcc/testsuite/c-c++-common/attr-nonstring-2.c (working copy) @@ -0,0 +1,110 @@ +/* Test to exercise attribute "nonstring". + { dg-do compile } + { dg-options "-O2 -Wattributes -Wstringop-truncation -ftrack-macro-expansion=0" } */ + +#define ATTR(list) __attribute__ (list) +#define NONSTR ATTR ((nonstring)) +#define strncpy(d, s, n) (__builtin_strncpy ((d), (s), (n)), sink (d)) + +void sink (void*); + +/* Global string with an unknown bound. */ +extern char gsx[]; + +/* Global string with an known bound. */ +extern char gs3[3]; + +/* Global non-strings with an unknown bound. */ +extern NONSTR char gax_1[]; +extern char NONSTR gax_2[]; +extern char gax_3[] NONSTR; + +/* Global non-strings with a known bound. */ +NONSTR char gns3[3]; +char NONSTR gns4[4]; +char gns5[5] NONSTR; + +/* Global string pointer. */ +extern char *ps_1; + +/* Global non-string pointers. */ +extern NONSTR char *pns_1; +extern char* NONSTR pns_2; +extern char *pns_3 NONSTR; + +struct MemArrays +{ + NONSTR char ma3[3]; + char NONSTR ma4[4]; + char ma5[5] NONSTR; + char max[] NONSTR; +}; + + +void test_array (const char *s, unsigned n) +{ + const char s7[] = "1234567"; + + strncpy (gs3, "", 0); /* { dg-warning "destination unchanged after copying no bytes" } */ + strncpy (gs3, "a", 1); /* { dg-warning "output truncated before terminating nul copying 1 byte from a string of the same length" } */ + strncpy (gs3, "a", 2); + strncpy (gs3, "a", 3); + strncpy (gs3, "ab", 3); + strncpy (gs3, "abc", 3); /* { dg-warning "output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + + strncpy (gax_3, s7, 3); /* { dg-warning "output truncated copying 3 bytes from a string of length 7" } */ + + strncpy (gax_1, "a", 1); + strncpy (gax_2, "ab", 2); + strncpy (gax_3, "abc", 3); + strncpy (gax_3, s7, 3); /* { dg-warning "output truncated copying 3 bytes from a string of length 7" } */ + + strncpy (gax_1, s, 1); + strncpy (gax_2, s, 1); + strncpy (gax_3, s, 1); + + strncpy (gax_1, s, n); + strncpy (gax_2, s, n); + strncpy (gax_3, s, n); +} + + +void test_pointer (const char *s, unsigned n) +{ + const char s7[] = "1234567"; + + strncpy (pns_1, "a", 1); + strncpy (pns_2, "ab", 2); + strncpy (pns_3, "abc", 3); + strncpy (pns_3, s7, 3); /* { dg-warning "output truncated copying 3 bytes from a string of length 7" } */ + + strncpy (pns_1, s, 1); + strncpy (pns_2, s, 1); + strncpy (pns_3, s, 1); + + strncpy (pns_1, s, n); + strncpy (pns_2, s, n); + strncpy (pns_3, s, n); +} + + +void test_member_array (struct MemArrays *p, const char *s, unsigned n) +{ + const char s7[] = "1234567"; + + strncpy (p->ma3, "a", 1); + strncpy (p->ma4, "ab", 2); + strncpy (p->ma5, "abc", 3); + strncpy (p->max, "abcd", 4); + strncpy (p->max, s7, 5); /* { dg-warning "output truncated copying 5 bytes from a string of length 7" } */ + + strncpy (p->ma3, s, 1); + strncpy (p->ma4, s, 1); + strncpy (p->ma5, s, 1); + strncpy (p->max, s, 1); + + strncpy (p->ma3, s7, n); + strncpy (p->ma4, s7, n); + strncpy (p->ma5, s7, n); + strncpy (p->max, s7, n); +} Index: gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C =================================================================== --- gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C (revision 251100) +++ gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C (working copy) @@ -1,6 +1,6 @@ // Test -Wsizeof-pointer-memaccess warnings. // { dg-do compile } -// { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow" } +// { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow -Wno-stringop-truncation" } // Test just twice, once with -O0 non-fortified, once with -O2 fortified. // { dg-skip-if "" { *-*-* } { "*" } { "-O0" "-O2" } } // { dg-skip-if "" { *-*-* } { "-flto" } { "" } } @@ -698,12 +698,17 @@ f4 (char *x, char **y, int z, char w[64]) strncat (w, s2, sizeof (w)); // { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } stpncpy (w, s1, sizeof (w)); // { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } - // These are correct, no warning. const char s3[] = "foobarbaz"; const char s4[] = "abcde12345678"; - strncpy (x, s3, sizeof (s3)); - strncat (x, s4, sizeof (s4)); - stpncpy (x, s3, sizeof (s3)); + + // These are pointless when the destination is large enough, and + // cause overflow otherwise. They might as well be replaced by + // strcpy() or memcpy(). + strncpy (x, s3, sizeof (s3)); // { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } + strncat (x, s4, sizeof (s4)); // { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } + stpncpy (x, s3, sizeof (s3)); // { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } + + // These are safe, no warning. y[1] = strndup (s3, sizeof (s3)); z += strncmp (s3, s4, sizeof (s3)); z += strncmp (s3, s4, sizeof (s4)); Index: gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C =================================================================== --- gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C (revision 251100) +++ gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C (working copy) @@ -1,6 +1,6 @@ // Test -Wsizeof-pointer-memaccess warnings. // { dg-do compile } -// { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow" } +// { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow -Wno-stringop-truncation" } // Test just twice, once with -O0 non-fortified, once with -O2 fortified, // suppressing buffer overflow warnings. // { dg-skip-if "" { *-*-* } { "*" } { "-O0" "-O2" } } @@ -703,12 +703,13 @@ f4 (char *x, char **y, int z, char w[64]) strncat (w, s2, sizeof (w)); // { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } stpncpy (w, s1, sizeof (w)); // { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } - // These are correct, no warning. const char s3[] = "foobarbaz"; const char s4[] = "abcde12345678"; - strncpy (x, s3, sizeof (s3)); - strncat (x, s4, sizeof (s4)); - stpncpy (x, s3, sizeof (s3)); + strncpy (x, s3, sizeof (s3)); // { dg-warning "call is the same expression as the source; did you mean to use the size of the destination" } + strncat (x, s4, sizeof (s4)); // { dg-warning "call is the same expression as the source; did you mean to use the size of the destination" } + stpncpy (x, s3, sizeof (s3)); // { dg-warning "call is the same expression as the source; did you mean to use the size of the destination" } + + // These are safe, no warning. y[1] = strndup (s3, sizeof (s3)); z += strncmp (s3, s4, sizeof (s3)); z += strncmp (s3, s4, sizeof (s4)); Index: gcc/testsuite/gcc.dg/Walloca-1.c =================================================================== --- gcc/testsuite/gcc.dg/Walloca-1.c (revision 251100) +++ gcc/testsuite/gcc.dg/Walloca-1.c (working copy) @@ -1,6 +1,6 @@ /* { dg-do compile } */ /* { dg-require-effective-target alloca } */ -/* { dg-options "-Walloca-larger-than=2000 -O2" } */ +/* { dg-options "-Walloca-larger-than=2000 -O2 -ftrack-macro-expansion=0" } */ #define alloca __builtin_alloca Index: gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c =================================================================== --- gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c (revision 251100) +++ gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c (working copy) @@ -1,6 +1,6 @@ /* Test -Wsizeof-pointer-memaccess warnings. */ /* { dg-do compile } */ -/* { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow" } */ +/* { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow -Wno-stringop-truncation" } */ /* Test just twice, once with -O0 non-fortified, once with -O2 fortified. */ /* { dg-skip-if "" { *-*-* } { "*" } { "-O0" "-O2" } } */ /* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */ @@ -704,12 +704,17 @@ f4 (char *x, char **y, int z, char w[64]) strncat (w, s2, sizeof (w)); /* { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } */ stpncpy (w, s1, sizeof (w)); /* { dg-warning "call is the same expression as the destination; did you mean to provide an explicit length" } */ - /* These are correct, no warning. */ + /* These are pointless when the destination is large enough, and + cause overflow otherwise. If the copies are guaranteed to be + safe the calls might as well be replaced by strcat(), strcpy(), + or memcpy(). */ const char s3[] = "foobarbaz"; const char s4[] = "abcde12345678"; - strncpy (x, s3, sizeof (s3)); - strncat (x, s4, sizeof (s4)); - stpncpy (x, s3, sizeof (s3)); + strncpy (x, s3, sizeof (s3)); /* { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } */ + strncat (x, s4, sizeof (s4)); /* { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } */ + stpncpy (x, s3, sizeof (s3)); /* { dg-warning "call is the same expression as the source; did you mean to use the size of the destination?" } */ + + /* These are correct, no warning. */ y[1] = strndup (s3, sizeof (s3)); z += strncmp (s3, s4, sizeof (s3)); z += strncmp (s3, s4, sizeof (s4)); Index: gcc/testsuite/gcc.dg/torture/pr63554.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr63554.c (revision 251100) +++ gcc/testsuite/gcc.dg/torture/pr63554.c (working copy) @@ -1,4 +1,5 @@ -/* { dg-do compile } */ +/* PR c/63554 - ice in "execute_todo, at passes.c:1797" with -O3 + { dg-do compile } */ char *a; void @@ -7,3 +8,5 @@ nssutil_ReadSecmodDB (void) long b = __builtin_object_size (0, 0); a = __builtin___strncat_chk (a, " ", 1, b); } + +/* { dg-prune-output "\\\[-Wstringop-overflow=]" } */ Index: gcc/tree-ssa-strlen.c =================================================================== --- gcc/tree-ssa-strlen.c (revision 251100) +++ gcc/tree-ssa-strlen.c (working copy) @@ -40,11 +40,16 @@ along with GCC; see the file COPYING3. If not see #include "expr.h" #include "tree-dfa.h" #include "domwalk.h" +#include "tree-ssa-alias.h" #include "tree-ssa-propagate.h" #include "params.h" #include "ipa-chkp.h" #include "tree-hash-traits.h" #include "builtins.h" +#include "diagnostic-core.h" +#include "diagnostic.h" +#include "intl.h" +#include "attribs.h" /* A vector indexed by SSA_NAME_VERSION. 0 means unknown, positive value is an index into strinfo vector, negative value stands for @@ -146,6 +151,9 @@ struct decl_stridxlist_map mappings. */ static hash_map *decl_to_stridxlist_htab; +typedef std::pair stridx_strlenloc; +static hash_map strlen_to_stridx; + /* Obstack for struct stridxlist and struct decl_stridxlist_map. */ static struct obstack stridx_obstack; @@ -1197,6 +1205,9 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi) si->nonzero_chars = lhs; gcc_assert (si->full_string_p); } + + location_t loc = gimple_location (stmt); + strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc)); return; } } @@ -1240,6 +1251,9 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi) strinfo *si = new_strinfo (src, idx, lhs, true); set_strinfo (idx, si); find_equal_ptrs (src, idx); + + location_t loc = gimple_location (stmt); + strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc)); } } @@ -1606,6 +1620,270 @@ handle_builtin_strcpy (enum built_in_function bcod fprintf (dump_file, "not possible.\n"); } +/* Return true if LEN depends on a call to strlen(SRC) in an interesting + way. LEN can either be an integer expression, or a pointer (to char). + When it is the latter (such as in recursive calls to self) is is + assumed to be the argument in some call to strlen() whose relationship + to SRC is being ascertained. */ + +static bool +is_strlen_related_p (tree src, tree len) +{ + if (TREE_CODE (TREE_TYPE (len)) == POINTER_TYPE + && operand_equal_p (src, len, 0)) + return true; + + if (TREE_CODE (len) != SSA_NAME) + return false; + + gimple *def_stmt = SSA_NAME_DEF_STMT (len); + if (!def_stmt) + return false; + + if (is_gimple_call (def_stmt)) + { + tree func = gimple_call_fndecl (def_stmt); + if (!valid_builtin_call (def_stmt) + || DECL_FUNCTION_CODE (func) != BUILT_IN_STRLEN) + return false; + + tree arg = gimple_call_arg (def_stmt, 0); + return is_strlen_related_p (src, arg); + } + + if (!is_gimple_assign (def_stmt)) + return false; + + tree_code code = gimple_assign_rhs_code (def_stmt); + tree rhs1 = gimple_assign_rhs1 (def_stmt); + tree rhstype = TREE_TYPE (rhs1); + + if ((TREE_CODE (rhstype) == POINTER_TYPE && code == POINTER_PLUS_EXPR) + || (INTEGRAL_TYPE_P (rhstype) + && (code == BIT_AND_EXPR + || code == NOP_EXPR))) + { + /* Pointer plus (an integer) and integer cast or truncation are + considered among the (potentially) related expressions to strlen. + Others are not. */ + return is_strlen_related_p (src, rhs1); + } + + return false; +} + +/* A helper of handle_builtin_stxncpy. 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[LEN - 1] = '\0'. If a) holds + and b) does not, warn. Otherwise, do nothing. Return true if + diagnostic has been issued. + + The purpose is to diagnose calls to strncpy and stpncpy that do + not nul-terminate the copy while allowing for the idiom where + such a call is immediately followed by setting the last element + to nul, as in: + char a[32]; + strncpy (a, s, sizeof a); + a[sizeof a - 1] = '\0'; +*/ + +static bool +maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree len) +{ + if (!warn_stringop_truncation) + return false; + + gimple *stmt = gsi_stmt (gsi); + + if (TREE_CODE (len) != INTEGER_CST) + return false; + + /* Negative value is the constant string length. */ + int sidx = get_stridx (src); + if (sidx < 0 && compare_tree_int (len, ~sidx) > 0) + return false; + + tree dst = gimple_call_arg (stmt, 0); + + /* See if the destination is declared with attribute "nonstring" + and if so, avoid the truncation warning. */ + if (TREE_CODE (dst) == SSA_NAME) + { + if (SSA_NAME_IS_DEFAULT_DEF (dst)) + dst = SSA_NAME_VAR (dst); + else + { + gimple *def = SSA_NAME_DEF_STMT (dst); + + if (is_gimple_assign (def) + && gimple_assign_rhs_code (def) == ADDR_EXPR) + dst = gimple_assign_rhs1 (def); + } + } + + tree dstdecl = dst; + if (TREE_CODE (dstdecl) == ADDR_EXPR) + dstdecl = TREE_OPERAND (dstdecl, 0); + + { + tree d = dstdecl; + if (TREE_CODE (d) == COMPONENT_REF) + d = TREE_OPERAND (d, 1); + + if (DECL_P (d) && lookup_attribute ("nonstring", DECL_ATTRIBUTES (d))) + return false; + } + + /* Look for dst[i] = '\0'; after the stxncpy() call and if found + avoid the truncation warning. */ + gsi_next_nondebug (&gsi); + gimple *next_stmt = gsi_stmt (gsi); + + if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt)) + { + HOST_WIDE_INT off; + dstdecl = get_addr_base_and_unit_offset (dstdecl, &off); + + tree lhs = gimple_assign_lhs (next_stmt); + tree lhsbase = get_addr_base_and_unit_offset (lhs, &off); + if (operand_equal_p (dstdecl, lhsbase, 0)) + return false; + } + + strinfo *sisrc = sidx > 0 ? get_strinfo (sidx) : NULL; + + if (sisrc && sisrc->nonzero_chars + && tree_fits_uhwi_p (sisrc->nonzero_chars)) + { + /* If the source is shorter than the specified bound the copy + is nul-terminated. */ + if (tree_int_cst_lt (sisrc->nonzero_chars, len)) + return false; + + /* If the source is longer than the specified bound the copy + is truncated. */ + if (tree_int_cst_lt (len, sisrc->nonzero_chars)) + { + location_t callloc = gimple_location (stmt); + tree func = gimple_call_fndecl (stmt); + + warning_at (callloc, OPT_Wstringop_truncation, + integer_onep (len) + ? G_("%qD output truncated copying %E byte from " + "a string of length %E") + : G_("%qD output truncated copying %E bytes from " + "a string of length %E"), + func, len, sisrc->nonzero_chars); + return true; + } + + /* Otherwise the source length matches the bound. */ + } + else + { + /* The source length is uknown. Try to determine the destination + size and see if it matches the specified bound. If not, bail. + Otherwise go on to see if it should be diagnosed for possible + truncation. */ + tree objsize = compute_objsize (dst, 1); + if (!objsize) + return false; + + if (!tree_int_cst_equal (objsize, len)) + return false; + } + + location_t callloc = gimple_location (stmt); + tree func = gimple_call_fndecl (stmt); + + bool warned = warning_at (callloc, OPT_Wstringop_truncation, + "%qD specified bound %E equals destination size", + func, len); + + gimple_set_no_warning (stmt, true); + + return warned; +} + +/* Check the size argument to the built-in forms of stpncpy and strncpy + to see if it's derived from calling strlen() on the source argument + and if so, issue a warning. */ + +static void +handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi) +{ + gimple *stmt = gsi_stmt (*gsi); + + bool with_bounds = gimple_call_with_bounds_p (stmt); + + tree src = gimple_call_arg (stmt, with_bounds ? 2 : 1); + tree len = gimple_call_arg (stmt, with_bounds ? 3 : 2); + + /* If the length argument was computed from strlen(S) for some string + S retrieve the strinfo index for the string (PSS->FIRST) alonng with + the location of the strlen() call (PSS->SECOND). */ + stridx_strlenloc *pss = strlen_to_stridx.get (len); + if (!pss || pss->first <= 0) + { + maybe_diag_stxncpy_trunc (*gsi, src, len); + return; + } + + int sidx = get_stridx (src); + strinfo *sisrc = sidx > 0 ? get_strinfo (sidx) : NULL; + + /* Strncpy() et al. cannot modify the source string. Prevent the rest + of the pass from invalidating the strinfo data. */ + if (sisrc) + sisrc->dont_invalidate = true; + + /* Retrieve the strinfo data for the string S that LEN was computed + from as some function F of strlen (S) (i.e., LEN need not be equal + to strlen(S)). */ + strinfo *silen = get_strinfo (pss->first); + + location_t callloc = gimple_location (stmt); + + tree func = gimple_call_fndecl (stmt); + + bool warned = false; + + /* When -Wstringop-truncation is set, try to determine truncation + before diagnosing possible overflow. Truncation is implied by + the LEN argument being equal to strlen(SRC), regardless of + 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 (warn_stringop_truncation + && sisrc == silen + && is_strlen_related_p (src, len)) + warned = warning_at (callloc, OPT_Wstringop_truncation, + "%qD output truncated before terminating nul " + "copying as many bytes from a string as its length", + func); + else if (silen && is_strlen_related_p (src, silen->ptr)) + warned = warning_at (callloc, OPT_Wstringop_overflow_, + "%qD specified bound depends on the length " + "of the source argument", func); + if (warned) + { + location_t strlenloc = pss->second; + if (strlenloc != UNKNOWN_LOCATION && strlenloc != callloc) + inform (strlenloc, "length computed here"); + } +} + +/* Check the size argument to the built-in forms of strncat to see if + it's derived from calling strlen() on the source argument and if so, + issue a warning. */ + +static void +handle_builtin_strncat (built_in_function bcode, gimple_stmt_iterator *gsi) +{ + /* Same as stxncpy(). */ + handle_builtin_stxncpy (bcode, gsi); +} + /* Handle a memcpy-like ({mem{,p}cpy,__mem{,p}cpy_chk}) call. If strlen of the second argument is known and length of the third argument is that plus one, strlen of the first argument is the same after this @@ -2512,6 +2790,19 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi) case BUILT_IN_STPCPY_CHK_CHKP: handle_builtin_strcpy (DECL_FUNCTION_CODE (callee), gsi); break; + + case BUILT_IN_STRNCAT: + case BUILT_IN_STRNCAT_CHK: + handle_builtin_strncat (DECL_FUNCTION_CODE (callee), gsi); + break; + + case BUILT_IN_STPNCPY: + case BUILT_IN_STPNCPY_CHK: + case BUILT_IN_STRNCPY: + case BUILT_IN_STRNCPY_CHK: + handle_builtin_stxncpy (DECL_FUNCTION_CODE (callee), gsi); + break; + case BUILT_IN_MEMCPY: case BUILT_IN_MEMCPY_CHK: case BUILT_IN_MEMPCPY: @@ -2575,6 +2866,10 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi) else if (code == EQ_EXPR || code == NE_EXPR) fold_strstr_to_strncmp (gimple_assign_rhs1 (stmt), gimple_assign_rhs2 (stmt), stmt); + + tree rhs1 = gimple_assign_rhs1 (stmt); + if (stridx_strlenloc *ps = strlen_to_stridx.get (rhs1)) + strlen_to_stridx.put (lhs, *ps); } else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs)) { @@ -2826,6 +3121,8 @@ pass_strlen::execute (function *fun) laststmt.len = NULL_TREE; laststmt.stridx = 0; + strlen_to_stridx.empty (); + return 0; } --------------186A909103B9426AFE6DC16F--