[3/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

Message ID	d1fd572a-6bff-d95b-1b9e-23527e8ea14b@gmail.com
State	New
Headers	show Return-Path: <gcc-patches-return-459908-incoming=patchwork.ozlabs.org@gcc.gnu.org> DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=xG6bcvPifUw8jvRv8 hqpdnRYEaynpoccZBOO6NC8sijeczvCl+ugFzYTuo4MHurhlYoV9/d87/k2YUWzU +ibYA48kDTBWYGw5xLcM8rZY7yRwD6ulIqeUsTeHyHaNImBnMDZ81lQfUhJP5s1p 46ycfdWucuDvn6PmEV03ENIo1g= Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk Sender: gcc-patches-owner@gcc.gnu.org Subject: [PATCH 3/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117) To: Jeff Law <law@redhat.com>, Gcc Patch List <gcc-patches@gcc.gnu.org> References: <13944863-99a8-4144-1703-c6e1a2f36425@gmail.com> <0bbc91cd-fcdb-be61-e1d0-4b230f23b1a9@redhat.com> <4f4fbd4c-cb46-b80d-5749-ebb6bb050bc4@gmail.com> <a0069a6d-d4a8-67df-e2f3-902dc7704ab2@redhat.com> From: Martin Sebor <msebor@gmail.com> Message-ID: <d1fd572a-6bff-d95b-1b9e-23527e8ea14b@gmail.com> Date: Sun, 6 Aug 2017 14:07:17 -0600 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: <a0069a6d-d4a8-67df-e2f3-902dc7704ab2@redhat.com> Content-Type: multipart/mixed; boundary="------------E61B867F81D0F42446AF5412"

On 11/03/2017 09:49 PM, Jeff Law wrote: > On 10/20/2017 06:18 PM, Martin Sebor wrote: > >>>>> 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. >> >> You said "maybe that would be worse" so I hadn't implemented it. >> I went ahead and coded it up but with more testing I don't think >> it has the desired result. See below. > I probably should have said that while it may be better at rooting out > false positives, the ability to find things that are not consecutive > could perhaps be seen as a negative in that it might fail to warn on > code that while not technically incorrect is likely dodgy. That's my feeling as well. >>>> I'll have to try this to better understand how it might work. >>> It's actually quite simple. >>> >>> Rather than looking at the next statement in the chain via >>> gsi_next_nondebug you follow the def->use chain for the memory tag >>> associated with the string copy statement. >>> >>> /* Get the memory tag that is defined by this statement. */ >>> defvar = gimple_vdef (gsi_stmt (gsi)); >>> >>> imm_use_iterator iter; >>> gimple *use_stmt; >>> >>> if (num_imm_uses (defvar) == 1) >>> { >>> imm_use_terator iter; >>> gimple *use_stmt; >>> >>> /* Iterate over the immediate uses of the memory tag. */ >>> FOR_EACH_IMM_USE_STMT (use_stmt, ui, defvar) >>> { >>> Check if STMT is dst[i] = '\0' >>> } >>> } >>> >>> >>> >>> The check that there is a single immediate use is designed to make sure >>> you get a warning for this scenario: >> >> Thanks for the outline of the solution. I managed to get it to >> work with only a few minor changes(*) but... >> >>> strxncpy >>> read the destination >>> terminate the destination >>> >>> Which I think you'd want to consider non-terminated because of the read >>> of the destination prior to termination. >>> >>> But avoids warnings for >>> >>> strxncpy >>> stuff that doesn't read the destination >>> terminate the destintion >> >> ...while it works fine for the basic cases it has the downside >> of missing more subtle problems like this one: >> >> char a[8]; >> >> void f (void) { puts (a); } >> >> void g (const char *s) >> { >> strncpy (a, s); >> >> f (); // assumes a is a string >> >> a[sizeof a - 1] = '\0'; >> } > Don't you have a VDEF at the call to f()? Wouldn't that be the only > immediate use? If you hit a VDEF without finding the termination, then > you warn. > > Or is it the case that the call gets inlined and we know enough about > puts that we get a VUSE rather than a VDEF? There is a VDEF in both cases. The point of the puts was just to illustrate that f() could read the array even though it's not obvious from the call. > In which case we have a VUSE at the puts and a VDEF at the array > assignment in the immediate use list. > > I think this highlights that I over-simplified my pseudo code. If you > find a VUSE in the list, then you have to warn. IF you find a VDEF that > does not terminate, then you have to warn. I don't think it's that simple. Suppose we have: strncpy (a, s, sizeof a); unsigned n = strlen (s); a[sizeof a - 1] = 0; return n; The next statement after the strncpy is a VUSE but warning would be a false positive because the next statement appends the NUL to a. So I'd have to iterate over all subsequent statements and inspect each to see if it happens to either use or set a. I'm sure it's possible to get it to work but I don't think it's worth the effort or the complexity or the compile time costs. I haven't come across any code that does this yet. > It's probably academic at this point -- it's probably more useful to > understand the immediate uses and the vuse/vdef work for the future. I > can live with a statement walking implementation. Since we both feel that the practice to encourage is to have the nul termination follow immediately the function call I agree that this is academic. I do appreciate the suggestion though. >>>> + 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); >>>> + } >>> >>> This seems wrong as-written -- you have no idea what the RHS code is. >>> You're just blindly taking the first operand, then digging into its >>> type. It probably works in practice, but it would seem better to verify >>> that gimple_assign_rhs_code is a conversion first (CONVERT_EXPR_CODE_P). >>> If it's not a CONVERT_EXPR_CODE_P, return NULL_TREE. >> >> The code doesn't match CONVERT_EXPR_P(). It's expected to match >> ADDR_EXPR and that's also what it tests for on the line just below >> the hunk you pasted above. Here it is: > [ ... [ >> >> 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; >> >> So unless I'm missing something this does what you're looking >> for.But you still making assumptions that you don't know are valid. > > You walk to the defining statement of the given SSA_NAME. You verify it > is an assignment. You're OK up to this point. > > You then proceed to look at rhs1. Instead you should look at > gimple_assign_rhs_code and verify that is an ADDR_EXPR. I see what you mean. The code also (optimistically) handles things like char a[10]; extern unsigned offset; strcpy (a + offset, "123456"); i.e., POINTER_PLUS_EXPR with a non-constant offset. I've added a check for these two codes and excluded everything else (I'm not sure what else could reasonably come up here so I have no tests for this). > For your particular example I don't think it matters, but I suspect it'd > matter for something if the defining statement was something like a > POINTER_PLUS_EXPR where the first operand might be an ADDR_EXPR. Attached is another update to the patch with the above. While going through it again I noticed a couple of opportunities to handle ranges so I added those as well. I have retested the patch with GDB and Glibc. They both require minor changes and I will submit patches with those once this work is checked in. Is this good enough to commit now? Martin PR c/81117 - Improve buffer overflow checking in strncpy ada/ChangeLog: PR c/81117 * adadecode.c (__gnat_decode): Use memcpy instead of strncpy. * argv.c (__gnat_fill_env): Same. fortran/ChangeLog: PR c/81117 * decl.c (build_sym): Use memcpy instead of 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-common.c (catenate_strings): Use memcpy instead of strncpy. * c-warn.c (sizeof_pointer_memaccess_warning): Handle arrays. * c.opt (-Wstringop-truncation): New option. gcc/objc/ChangeLog: PR c/81117 * objc-encoding.c (encode_type): Use memcpy instead of strncpy. 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/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. diff --git a/gcc/ada/adadecode.c b/gcc/ada/adadecode.c index 8c9c7ab..0cbef81 100644 --- a/gcc/ada/adadecode.c +++ b/gcc/ada/adadecode.c @@ -330,7 +330,7 @@ __gnat_decode (const char *coded_name, char *ada_name, int verbose) } /* Write symbol in the space. */ - strncpy (optoken, trans_table[k][1], oplen); + memcpy (optoken, trans_table[k][1], oplen); } else k++; diff --git a/gcc/ada/argv.c b/gcc/ada/argv.c index 430404e..aee0f88 100644 --- a/gcc/ada/argv.c +++ b/gcc/ada/argv.c @@ -92,7 +92,7 @@ void __gnat_fill_arg (char *a, int i) { if (gnat_argv != NULL) - strncpy (a, gnat_argv[i], strlen(gnat_argv[i])); + memcpy (a, gnat_argv[i], strlen (gnat_argv[i])); } int @@ -118,7 +118,7 @@ void __gnat_fill_env (char *a, int i) { if (gnat_envp != NULL) - strncpy (a, gnat_envp[i], strlen (gnat_envp[i])); + memcpy (a, gnat_envp[i], strlen (gnat_envp[i])); } #ifdef __cplusplus diff --git a/gcc/builtins.c b/gcc/builtins.c index a677705..ce53073 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -3260,18 +3260,60 @@ check_sizes (int opt, tree exp, tree size, tree maxlen, tree src, tree objsize) } /* 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; + + tree_code code = gimple_assign_rhs_code (stmt); + if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR) + 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; } @@ -3923,6 +3965,22 @@ 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, CNT) 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 cnt) +{ + tree dstsize = compute_objsize (dst, warn_stringop_overflow - 1); + + if (!check_sizes (OPT_Wstringop_overflow_, + exp, cnt, /*maxlen=*/NULL_TREE, src, dstsize)) + return false; + + return true; +} + /* Expand expression EXP, which is a call to the strncpy builtin. Return NULL_RTX if we failed the caller should emit a normal call. */ @@ -3941,16 +3999,7 @@ 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); - - /* 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); - } + check_strncpy_sizes (exp, dest, src, len); /* We must be passed a constant len and src parameter. */ if (!tree_fits_uhwi_p (len) || !slen || !tree_fits_uhwi_p (slen)) diff --git a/gcc/builtins.h b/gcc/builtins.h index 4ae7056..cf3fc17 100644 --- a/gcc/builtins.h +++ b/gcc/builtins.h @@ -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); diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 24077c7..a76fae7 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -5890,10 +5890,10 @@ check_builtin_function_arguments (location_t loc, vec<location_t> arg_loc, static char * catenate_strings (const char *lhs, const char *rhs_start, int rhs_size) { - const int lhs_size = strlen (lhs); + const size_t lhs_size = strlen (lhs); char *result = XNEWVEC (char, lhs_size + rhs_size); - strncpy (result, lhs, lhs_size); - strncpy (result + lhs_size, rhs_start, rhs_size); + memcpy (result, lhs, lhs_size); + memcpy (result + lhs_size, rhs_start, rhs_size); return result; } diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index 09ef685..6cfded9 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -693,7 +693,8 @@ sizeof_pointer_memaccess_warning (location_t *sizeof_arg_loc, tree callee, || 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: @@ -775,8 +776,27 @@ sizeof_pointer_memaccess_warning (location_t *sizeof_arg_loc, tree callee, 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 %<sizeof%> 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)) diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 9ab31f0..479ae63 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -744,6 +744,10 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Ini 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. diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index e897d93..4427328 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -314,7 +314,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{|}malloc@r{]} @gol -Wsuggest-final-types @gol -Wsuggest-final-methods -Wsuggest-override @gol -Wmissing-format-attribute -Wsubobject-linkage @gol @@ -5214,6 +5214,55 @@ whether to issue a warning. Similarly to @option{-Wstringop-overflow=3} this 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{|}cold@r{|}malloc@r{]} @opindex Wsuggest-attribute= @opindex Wno-suggest-attribute= @@ -6230,11 +6279,26 @@ not an array, but a pointer. This warning is enabled by @option{-Wall}. @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 diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index 1a2d8f0..11264e7 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -1427,11 +1427,9 @@ build_sym (const char *name, gfc_charlen *cl, bool cl_deferred, { char u_name[GFC_MAX_SYMBOL_LEN + 1]; gfc_symtree *st; - int nlen; - nlen = strlen(name); - gcc_assert (nlen <= GFC_MAX_SYMBOL_LEN); - strncpy (u_name, name, nlen + 1); + gcc_assert (strlen(name) <= GFC_MAX_SYMBOL_LEN); + strcpy (u_name, name); u_name[0] = upper; st = gfc_find_symtree (gfc_current_ns->sym_root, u_name); diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 85fd397..c65b418 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -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. @@ -1553,12 +1556,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 <gcall *> (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; } @@ -1573,16 +1592,66 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, 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 <gcall *> (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 <gcall *> (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; @@ -1591,6 +1660,7 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, 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; } @@ -1887,24 +1957,71 @@ gimple_fold_builtin_strncat (gimple_stmt_iterator *gsi) 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) + return false; + + unsigned srclen = strlen (p); + + int cmpsrc = compare_tree_int (len, srclen); + + /* Return early if the requested len is less than the string length. + Warnings will be issued elsewhere later. */ + if (cmpsrc < 0) + return false; + + unsigned HOST_WIDE_INT dstsize; + + bool nowarn = gimple_no_warning_p (stmt); + + if (!nowarn && compute_builtin_object_size (dst, 1, &dstsize)) { - tree fn = builtin_decl_implicit (BUILT_IN_STRCAT); + int cmpdst = compare_tree_int (len, dstsize); - /* If the replacement _DECL isn't initialized, don't do the - transformation. */ - if (!fn) - return false; + 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); + } + } - gcall *repl = gimple_build_call (fn, 2, dst, src); - replace_call_with_call_and_fold (gsi, repl); - return 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); + if (warning_at (loc, OPT_Wstringop_overflow_, + "%G%qD specified bound %E equals source length", + stmt, fndecl, len)) + gimple_set_no_warning (stmt, true); } - return false; + tree fn = builtin_decl_implicit (BUILT_IN_STRCAT); + + /* If the replacement _DECL isn't initialized, don't do the + transformation. */ + if (!fn) + return false; + + /* Otherwise, emit a call to strcat. */ + gcall *repl = gimple_build_call (fn, 2, dst, src); + replace_call_with_call_and_fold (gsi, repl); + return true; } /* Fold a call to the __strncat_chk builtin with arguments DEST, SRC, diff --git a/gcc/gimple.c b/gcc/gimple.c index 37f2248..c986a73 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -361,6 +361,7 @@ gimple_build_call_from_tree (tree t, tree fnptrtype) 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)); diff --git a/gcc/objc/objc-encoding.c b/gcc/objc/objc-encoding.c index 9f46d57..f9d8d47 100644 --- a/gcc/objc/objc-encoding.c +++ b/gcc/objc/objc-encoding.c @@ -734,7 +734,7 @@ encode_type (tree type, int curtype, int format) /* Rewrite "in const" from "nr" to "rn". */ if (curtype >= 1 && !strncmp (enc - 1, "nr", 2)) - strncpy (enc - 1, "rn", 2); + memcpy (enc - 1, "rn", 2); } } } diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c index 895a50e..f7bfa35 100644 --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c +++ b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c @@ -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\]*" } */ diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c new file mode 100644 index 0000000..97598c4 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c @@ -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 *-*-* } } */ +} diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow.c b/gcc/testsuite/c-c++-common/Wstringop-overflow.c new file mode 100644 index 0000000..53f5166 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wstringop-overflow.c @@ -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" } */ + } +} diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c new file mode 100644 index 0000000..c536a13 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c @@ -0,0 +1,448 @@ +/* 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 + +extern size_t unsigned_value (void) +{ + extern volatile size_t unsigned_value_source; + return unsigned_value_source; +} + +size_t unsigned_range (size_t min, size_t max) +{ + size_t val = unsigned_value (); + return val < min || max < val ? min : val; +} + +#define UR(min, max) unsigned_range (min, max) + +void sink (void*); + +#define S4 "123" +const char a4[] = "123"; + +#define CHOOSE(a, b) (unsigned_value () & 1 ? a : b) + + +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) +{ +#define CAT(d, s, len) (strncat ((d), (s), (len)), sink (d)) + + CAT (dst7, S4, 2); /* { dg-warning "output truncated copying 2 bytes from a string of length 3" } */ + + CAT (dst7, a4, 1); /* { dg-warning "output truncated copying 1 byte from a string of length 3" } */ + + /* There is no truncation here but possible overflow so these + are diagnosed by -Wstringop-overflow: + CAT (dst7, S4, 3); + CAT (dst7, a4, 3); + */ + + CAT (pd->a5, S4, 2); /* { dg-warning "output truncated copying 2 bytes from a string of length 3" } */ + CAT (pd->a5, S4, 1); /* { dg-warning "output truncated copying 1 byte from a string of length 3" } */ +} + +/* Verify strncat warnings for arrays of known bounds and a non-const + character count in some range. */ + +void test_strncat_array_range (Dest *pd) +{ + CAT (dst7, S4, UR (0, 1)); /* { dg-warning "output truncated copying between 0 and 1 bytes from a string of length 3" } */ + CAT (dst7, S4, UR (0, 2)); /* { dg-warning "output truncated copying between 0 and 2 bytes from a string of length 3" } */ + CAT (dst7, S4, UR (1, 3)); /* { dg-warning "output truncated copying between 1 and 3 bytes from a string of length 3" } */ + CAT (dst7, S4, UR (2, 4)); /* { dg-warning "output may be truncated copying between 2 and 4 bytes from a string of length 3" } */ + + CAT (dst7, S4, UR (0, 7)); + CAT (dst7, S4, UR (1, 7)); + CAT (dst7, S4, UR (6, 7)); + + CAT (dst7, S4, UR (0, 99)); + + CAT (dst7, S4, UR (0, 99)); +} + +/* Verify strncat warnings for arrays of unknown bounds. */ + +void test_strncat_vla (char *d, unsigned n) +{ + CAT (d, S4, 2); /* { dg-warning "output truncated copying 2 bytes from a string of length 3" } */ + CAT (d, S4, 4); + + CAT (d, a4, 2); /* { dg-warning "output truncated copying 2 bytes from a string of length 3" } */ + + /* There is no truncation here but possible overflow so these + are diagnosed by -Wstringop-overflow: + CAT (d, S4, 3); + CAT (d, a4, 3); + */ + CAT (d, a4, 4); + + char vla[n]; + + CAT (vla, S4, 2); /* { dg-warning "output truncated copying 2 bytes from a string of length 3" } */ + + CAT (vla, S4, 4); + CAT (vla, S4, n); + + CAT (vla, a4, 2); /* { dg-warning "output truncated copying 2 bytes from a string of length 3" } */ + + CAT (vla, a4, 4); + CAT (vla, a4, n); + + CAT (d, vla, 1); + CAT (d, vla, 3); + CAT (d, vla, 4); + CAT (d, vla, n); + + /* There is no truncation here but possible overflow so these + are diagnosed by -Wstringop-overflow: + CAT (vla, S4, 3); + CAT (vla, a4, 3); + */ +} + +/* Verify strncpy warnings with at least one pointer to an object + or string of unknown size (destination) or length (source). */ + +void test_strncpy_ptr (char *d, const char* s, const char *t, int i) +{ +#define CPY(d, s, len) (strncpy ((d), (s), (len)), sink (d)) + + /* Strncpy doesn't nul-terminate so the following is diagnosed. */ + CPY (d, "", 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + CPY (d, s, 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + + /* This is safe. */ + CPY (d, "", 1); + CPY (d, "", 2); + + /* This could be safe. */ + CPY (d, s, 1); + CPY (d, s, 2); + + /* Truncation. */ + CPY (d, "123", 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 3" } */ + CPY (d, "123", 2); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 2 bytes from a string of length 3" } */ + CPY (d, "123", 3); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + CPY (d, "123", 4); + CPY (d, "123", 9); + + CPY (d, S4, sizeof S4); /* Covered by -Wsizeof-pointer-memaccess. */ + CPY (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" } */ + + CPY (d, a4, sizeof a4); /* Covered by -Wsizeof-pointer-memaccess. */ + CPY (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" } */ + CPY (d, a4, sizeof a4 - 3); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 3" } */ + CPY (d, a4, sizeof a4 - 4); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes from a string of length 3" } */ + + CPY (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. */ + CPY (d, a4, strlen (a4) + 1); + CPY (d, S4, strlen (S4) + i); + + CPY (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. */ + CPY (d, S4, strlen (S4) + 1); + + CPY (d, CHOOSE ("", "1"), 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + CPY (d, CHOOSE ("1", "12"), 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + + CPY (d, CHOOSE ("", "1"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output may be truncated copying 1 byte from a string of length 1" } */ + CPY (d, CHOOSE ("1", ""), 1); /* { dg-warning ".strncpy\[^\n\r\]* output may be truncated copying 1 byte from a string of length 1" } */ + CPY (d, CHOOSE (s, "1"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output may be truncated copying 1 byte from a string of length 1" } */ + CPY (d, CHOOSE (s, t), 1); + + CPY (d, CHOOSE ("", "1"), 2); + CPY (d, CHOOSE ("1", ""), 2); + CPY (d, CHOOSE ("1", "2"), 2); + CPY (d, CHOOSE ("1", s), 2); + CPY (d, CHOOSE (s, "1"), 2); + CPY (d, CHOOSE (s, t), 2); + + CPY (d, CHOOSE ("", "123"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output may be truncated copying 1 byte from a string of length 3" } */ + CPY (d, CHOOSE ("1", "123"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 1" } */ + CPY (d, CHOOSE ("12", "123"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */ + CPY (d, CHOOSE ("123", "12"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */ + + { + signed char n = strlen (s); /* { dg-message "length computed here" } */ + CPY (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" } */ + CPY (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" } */ + CPY (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" } */ + CPY (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" } */ + CPY (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" } */ + CPY (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; + CPY (d, s, n); + } + + { + /* Same as above. */ + size_t n; + n = strlen (s) - 1; + CPY (d, s, n); + } + + { + size_t n = strlen (s) - strlen (s); + CPY (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); + CPY (d, s, n); + */ + } +} + + +/* Verify strncpy warnings for arrays of known bounds. */ + +void test_strncpy_array (Dest *pd, int i, const char* s) +{ +#undef CPY +#define CPY(d, s, len) (strncpy ((d), (s), (len)), sink (d)) + + CPY (dst7, s, 7); /* { dg-warning "specified bound 7 equals destination size" } */ + CPY (dst7, s, sizeof dst7); /* { dg-warning "specified bound 7 equals destination size" } */ + + CPY (dst2_5[0], s, sizeof dst2_5[0]); /* { dg-warning "specified bound 5 equals destination size" "bug 77293" { xfail *-*-* } } */ + CPY (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. */ + CPY (dst7, "", sizeof dst7); + CPY (dst7 + 6, "", sizeof dst7 - 6); + CPY (dst7, "1", sizeof dst7); + CPY (dst7 + 1, "1", sizeof dst7 - 1); + CPY (dst7, "123456", sizeof dst7); + CPY (dst7 + 1, "12345", sizeof dst7 - 1); + + CPY (dst7 + i, s, 6); + CPY (dst7 + i, s, 7); /* { dg-warning "specified bound 7 equals destination size" } */ + /* The following two calls are diagnosed by -Wstringop-overflow. */ + CPY (dst7 + i, s, 8); + CPY (dst7 + i, s, UR (8, 9)); + + /* No nul-termination here. */ + CPY (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. */ + CPY (dst7, s, 8); + + CPY (dst7 + 1, s, 6); /* { dg-warning "specified bound 6 equals destination size" } */ + CPY (dst7 + 6, s, 1); /* { dg-warning "specified bound 1 equals destination size" } */ + + CPY (pd->a5, s, 5); /* { dg-warning "specified bound 5 equals destination size" } */ + CPY (pd->a5, s, sizeof pd->a5); /* { dg-warning "specified bound 5 equals destination size" } */ + + /* The following is not yet handled. */ + CPY (pd->a5 + i, s, sizeof pd->a5); /* { dg-warning "specified bound 5 equals destination size" "member array" { xfail *-*-* } } */ + + /* Verify that a copy that nul-terminates is not diagnosed. */ + CPY (pd->a5, "1234", sizeof pd->a5); + + /* Same above, diagnosed by -Wstringop-overflow. */ + CPY (pd->a5, s, 6); + + /* Exercise destination with attribute "nonstring". */ + CPY (pd->c3ns, "", 3); + CPY (pd->c3ns, "", 1); + /* Truncation is still diagnosed -- using strncpy in this case is + pointless and should be replaced with memcpy. */ + CPY (pd->c3ns, "12", 1); /* { dg-warning "output truncated copying 1 byte from a string of length 2" } */ + CPY (pd->c3ns, "12", 2); + CPY (pd->c3ns, "12", 3); + CPY (pd->c3ns, "123", 3); + CPY (pd->c3ns, s, 3); + CPY (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 CPY +#define CPY(d, s, len) (strncpy ((d), (s), (len)), sink (d)) + + CPY (array, "12345", 7); + CPY (array, "12345", 123); + + CPY (array, s, 7); + CPY (array, s, 123); + + CPY (pf->a0, s, 1); + CPY (pf->a0, s, 1234); + + CPY (pf->a0, "", 1); + CPY (pf->a0, "12345", 5); /* { dg-warning "output truncated before terminating nul copying 5 bytes from a string of the same length" } */ + CPY (pf->a0, "12345", 1234); + + CPY (pf->ax, s, 5); + CPY (pf->ax, s, 12345); + + CPY (pf->ax, "1234", 5); + CPY (pf->ax, "12345", 5); /* { dg-warning "output truncated before terminating nul copying 5 bytes from a string of the same length" } */ + CPY (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); + + CPY (d, s, n); /* { dg-warning "specified bound 7 equals destination size" "bug 79016" { xfail *-*-* } } */ + + Dest *pd = (Dest *)__builtin_malloc (sizeof *pd * n); + CPY (pd->a5, s, 5); /* { dg-warning "specified bound 5 equals destination size" } */ + CPY (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]; + CPY (vla, s, 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + + CPY (vla, s, 1); + CPY (vla, s, 2); + CPY (vla, s, n); + + CPY (vla, "", 0); /* { dg-warning ".strncpy\[^\n\r\]* destination unchanged after copying no bytes" } */ + CPY (vla, "", 1); + CPY (vla, S4, 3); /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 3 bytes from a string of the same length" } */ + CPY (vla, S4, n); +} diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-1.c b/gcc/testsuite/c-c++-common/attr-nonstring-1.c new file mode 100644 index 0000000..10a6688 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-nonstring-1.c @@ -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; +} diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-2.c b/gcc/testsuite/c-c++-common/attr-nonstring-2.c new file mode 100644 index 0000000..6e273e7 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-nonstring-2.c @@ -0,0 +1,123 @@ +/* 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" } */ + + /* It might perhaps be helpful to diagnose certain truncation even + for non-strings. Then again, since the destination has been + explicitly annotated as non-string, it might be viewed as a false + positive. A valid use case seen in Glibc goes something like this: + + #if FOO + # define S "1234" + #else + # define S "12345678" + #endif + + strncpy (d, S, 8); + */ + strncpy (gax_3, s7, 3); + + strncpy (gax_1, "a", 1); + strncpy (gax_2, "ab", 2); + strncpy (gax_3, "abc", 3); + strncpy (gax_3, s7, 3); + + 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); + + 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); +} diff --git a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C index c72532b..5bc5c4c 100644 --- a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C +++ b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C @@ -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)); diff --git a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C index a216f47..f2c864b 100644 --- a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C +++ b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C @@ -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)); diff --git a/gcc/testsuite/gcc.dg/Walloca-1.c b/gcc/testsuite/gcc.dg/Walloca-1.c index ad39373..85e9160 100644 --- a/gcc/testsuite/gcc.dg/Walloca-1.c +++ b/gcc/testsuite/gcc.dg/Walloca-1.c @@ -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 diff --git a/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c b/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c index f9bc57c..cd9dc72 100644 --- a/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c +++ b/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c @@ -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)); diff --git a/gcc/testsuite/gcc.dg/torture/pr63554.c b/gcc/testsuite/gcc.dg/torture/pr63554.c index fa06c5a..9162266 100644 --- a/gcc/testsuite/gcc.dg/torture/pr63554.c +++ b/gcc/testsuite/gcc.dg/torture/pr63554.c @@ -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=]" } */ diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index 4ec0dac..25fdcd0 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -40,12 +40,17 @@ 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 "target.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 @@ -147,6 +152,9 @@ struct decl_stridxlist_map mappings. */ static hash_map<tree_decl_hash, stridxlist> *decl_to_stridxlist_htab; +typedef std::pair<int, location_t> stridx_strlenloc; +static hash_map<tree, stridx_strlenloc> strlen_to_stridx; + /* Obstack for struct stridxlist and struct decl_stridxlist_map. */ static struct obstack stridx_obstack; @@ -1198,6 +1206,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; } } @@ -1241,6 +1252,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)); } } @@ -1607,6 +1621,368 @@ handle_builtin_strcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi) 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[CNT - 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 cnt) +{ + if (!warn_stringop_truncation) + return false; + + gimple *stmt = gsi_stmt (gsi); + + wide_int cntrange[2]; + + if (TREE_CODE (cnt) == INTEGER_CST) + cntrange[0] = cntrange[1] = wi::to_wide (cnt); + else if (TREE_CODE (cnt) == SSA_NAME) + { + enum value_range_type rng = get_range_info (cnt, cntrange, cntrange + 1); + if (rng == VR_RANGE) + ; + else if (rng == VR_ANTI_RANGE) + { + wide_int maxobjsize = wi::to_wide (TYPE_MAX_VALUE (ptrdiff_type_node)); + + if (wi::ltu_p (cntrange[1], maxobjsize)) + { + cntrange[0] = cntrange[1] + 1; + cntrange[1] = maxobjsize; + } + else + { + cntrange[1] = cntrange[0] - 1; + cntrange[0] = wi::zero (TYPE_PRECISION (TREE_TYPE (cnt))); + } + } + else + return false; + } + else + return false; + + /* Negative value is the constant string length. If it's less than + the lower bound there is no truncation. */ + int sidx = get_stridx (src); + if (sidx < 0 && wi::gtu_p (cntrange[0], ~sidx)) + 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 (&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 (lhsbase && operand_equal_p (dstdecl, lhsbase, 0)) + return false; + } + + int prec = TYPE_PRECISION (TREE_TYPE (cnt)); + wide_int lenrange[2]; + if (strinfo *sisrc = sidx > 0 ? get_strinfo (sidx) : NULL) + { + lenrange[0] = (sisrc->nonzero_chars + && TREE_CODE (sisrc->nonzero_chars) == INTEGER_CST + ? wi::to_wide (sisrc->nonzero_chars) + : wi::zero (prec)); + lenrange[1] = lenrange[0]; + } + else if (sidx < 0) + lenrange[0] = lenrange[1] = wi::shwi (~sidx, prec); + else + { + tree range[2]; + get_range_strlen (src, range); + if (range[0]) + { + lenrange[0] = wi::to_wide (range[0], prec); + lenrange[1] = wi::to_wide (range[1], prec); + } + else + { + lenrange[0] = wi::shwi (0, prec); + lenrange[1] = wi::shwi (-1, prec); + } + } + + location_t callloc = gimple_location (stmt); + tree func = gimple_call_fndecl (stmt); + + if (lenrange[0] != 0 || !wi::neg_p (lenrange[1])) + { + /* If the longest source string is shorter than the lower bound + of the specified count the copy is definitely nul-terminated. */ + if (wi::ltu_p (lenrange[1], cntrange[0])) + return false; + + if (wi::neg_p (lenrange[1])) + { + /* The length of one of the strings is unknown but at least + one has non-zero length and that length is stored in + LENRANGE[1]. Swap the bounds to force a "may be truncated" + warning below. */ + lenrange[1] = lenrange[0]; + lenrange[0] = wi::shwi (0, prec); + } + + if (wi::geu_p (lenrange[0], cntrange[1])) + { + /* The shortest string is longer than the upper bound of + the count so the truncation is certain. */ + if (cntrange[0] == cntrange[1]) + return warning_at (callloc, OPT_Wstringop_truncation, + integer_onep (cnt) + ? G_("%qD output truncated copying %E byte " + "from a string of length %wu") + : G_("%qD output truncated copying %E bytes " + "from a string of length %wu"), + func, cnt, lenrange[0].to_uhwi ()); + + return warning_at (callloc, OPT_Wstringop_truncation, + "%qD output truncated copying between %wu " + "and %wu bytes from a string of length %wu", + func, cntrange[0].to_uhwi (), + cntrange[1].to_uhwi (), lenrange[0].to_uhwi ()); + } + else if (wi::geu_p (lenrange[1], cntrange[1])) + { + /* The longest string is longer than the upper bound of + the count so the truncation is possible. */ + if (cntrange[0] == cntrange[1]) + return warning_at (callloc, OPT_Wstringop_truncation, + integer_onep (cnt) + ? G_("%qD output may be truncated copying %E " + "byte from a string of length %wu") + : G_("%qD output may be truncated copying %E " + "bytes from a string of length %wu"), + func, cnt, lenrange[1].to_uhwi ()); + + return warning_at (callloc, OPT_Wstringop_truncation, + "%qD output may be truncated copying between %wu " + "and %wu bytes from a string of length %wu", + func, cntrange[0].to_uhwi (), + cntrange[1].to_uhwi (), lenrange[1].to_uhwi ()); + } + + if (cntrange[0] != cntrange[1] + && wi::leu_p (cntrange[0], lenrange[0]) + && wi::leu_p (cntrange[1], lenrange[0] + 1)) + { + /* If the source (including the terminating nul) is longer than + the lower bound of the specified count but shorter than the + upper bound the copy may (but need not) be truncated. */ + return warning_at (callloc, OPT_Wstringop_truncation, + "%qD output may be truncated copying between %wu " + "and %wu bytes from a string of length %wu", + func, cntrange[0].to_uhwi (), + cntrange[1].to_uhwi (), lenrange[0].to_uhwi ()); + } + } + + if (tree dstsize = compute_objsize (dst, 1)) + { + /* 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. */ + if (!dstsize) + return false; + + if (wi::to_wide (dstsize) != cntrange[1]) + return false; + + if (cntrange[0] == cntrange[1]) + return warning_at (callloc, OPT_Wstringop_truncation, + "%qD specified bound %E equals destination size", + func, cnt); + } + + return false; +} + +/* 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) + { + if (maybe_diag_stxncpy_trunc (*gsi, src, len)) + gimple_set_no_warning (stmt, true); + + 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 @@ -2513,6 +2889,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: @@ -2576,6 +2965,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)) { @@ -2827,6 +3220,8 @@ pass_strlen::execute (function *fun) laststmt.len = NULL_TREE; laststmt.stridx = 0; + strlen_to_stridx.empty (); + return 0; }

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/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. diff --git a/gcc/builtins.c b/gcc/builtins.c index 016f68d..1aa9e22 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -3220,18 +3220,55 @@ check_sizes (int opt, tree exp, tree size, tree maxlen, tree src, tree objsize) } /* 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) + { + /* 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; } @@ -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; +} + /* Expand expression EXP, which is a call to the strncpy builtin. Return NULL_RTX if we failed the caller should emit a normal call. */ @@ -3901,16 +3962,7 @@ 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); - - /* 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); - } + check_strncpy_sizes (exp, dest, src, len); /* We must be passed a constant len and src parameter. */ if (!tree_fits_uhwi_p (len) || !slen || !tree_fits_uhwi_p (slen)) diff --git a/gcc/builtins.h b/gcc/builtins.h index f590f61..4d93eaf 100644 --- a/gcc/builtins.h +++ b/gcc/builtins.h @@ -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); diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index e970ab2..b8eb616 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -626,7 +626,8 @@ sizeof_pointer_memaccess_warning (location_t *sizeof_arg_loc, tree callee, || 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: @@ -708,8 +709,27 @@ sizeof_pointer_memaccess_warning (location_t *sizeof_arg_loc, tree callee, 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 %<sizeof%> 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)) diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index e0ad3ab..5caaf43 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -736,6 +736,10 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Ini 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. diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 5ae9dc4..7370881a 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -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 @@ -5168,6 +5168,56 @@ whether to issue a warning. Similarly to @option{-Wstringop-overflow=3} this 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= @@ -6152,11 +6202,26 @@ not an array, but a pointer. This warning is enabled by @option{-Wall}. @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 diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index d82d060..2be2503 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -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" @@ -57,6 +58,8 @@ along with GCC; see the file COPYING3. If not see #include "tree-cfg.h" #include "fold-const-call.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. @@ -1522,12 +1525,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 <gcall *> (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; } @@ -1542,16 +1561,66 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, 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 <gcall *> (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 <gcall *> (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; @@ -1560,6 +1629,7 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, 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; } @@ -1856,21 +1926,77 @@ gimple_fold_builtin_strncat (gimple_stmt_iterator *gsi) 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; diff --git a/gcc/gimple.c b/gcc/gimple.c index 3a32b53..e000cb5 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -359,6 +359,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)); diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c index 895a50e..f7bfa35 100644 --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c +++ b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c @@ -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\]*" } */ diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c new file mode 100644 index 0000000..97598c4 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c @@ -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 *-*-* } } */ +} diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow.c b/gcc/testsuite/c-c++-common/Wstringop-overflow.c new file mode 100644 index 0000000..53f5166 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wstringop-overflow.c @@ -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" } */ + } +} diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c new file mode 100644 index 0000000..8c6904f --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c @@ -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); +} diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-2.c b/gcc/testsuite/c-c++-common/attr-nonstring-2.c new file mode 100644 index 0000000..9698609 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-nonstring-2.c @@ -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); +} diff --git a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C index c72532b..5bc5c4c 100644 --- a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C +++ b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess1.C @@ -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)); diff --git a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C index a216f47..f2c864b 100644 --- a/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C +++ b/gcc/testsuite/g++.dg/torture/Wsizeof-pointer-memaccess2.C @@ -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)); diff --git a/gcc/testsuite/gcc.dg/Walloca-1.c b/gcc/testsuite/gcc.dg/Walloca-1.c index ad39373..85e9160 100644 --- a/gcc/testsuite/gcc.dg/Walloca-1.c +++ b/gcc/testsuite/gcc.dg/Walloca-1.c @@ -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 diff --git a/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c b/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c index f9bc57c..cd9dc72 100644 --- a/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c +++ b/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c @@ -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)); diff --git a/gcc/testsuite/gcc.dg/torture/pr63554.c b/gcc/testsuite/gcc.dg/torture/pr63554.c index fa06c5a..9162266 100644 --- a/gcc/testsuite/gcc.dg/torture/pr63554.c +++ b/gcc/testsuite/gcc.dg/torture/pr63554.c @@ -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=]" } */ diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index b0563fe..ac6503f 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -40,11 +40,15 @@ 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" /* A vector indexed by SSA_NAME_VERSION. 0 means unknown, positive value is an index into strinfo vector, negative value stands for @@ -146,6 +150,9 @@ struct decl_stridxlist_map mappings. */ static hash_map<tree_decl_hash, stridxlist> *decl_to_stridxlist_htab; +typedef std::pair<int, location_t> stridx_strlenloc; +static hash_map<tree, stridx_strlenloc> strlen_to_stridx; + /* Obstack for struct stridxlist and struct decl_stridxlist_map. */ static struct obstack stridx_obstack; @@ -1197,6 +1204,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 +1250,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 +1619,270 @@ handle_builtin_strcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi) 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 (&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 +2789,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 +2865,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 +3120,8 @@ pass_strlen::execute (function *fun) laststmt.len = NULL_TREE; laststmt.stridx = 0; + strlen_to_stridx.empty (); + return 0; }

[3/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

Commit Message

Comments

Patch