From: Martin Sebor Date: Fri, 10 Nov 2017 16:35:26 +0000 (+0000) Subject: PR c/81117 - Improve buffer overflow checking in strncpy X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=025d57f037ad13eb479818b677ef4be4d97b639c;p=gcc.git 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/ada/ChangeLog: PR c/81117 * ada/adadecode.c (__gnat_decode): Use memcpy instead of strncpy. * ada/argv.c (__gnat_fill_arg, __gnat_fill_env): Same. 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/fortran/ChangeLog: PR c/81117 * gcc/fortran/decl.c (build_sym): Use strcpy instead of strncpy. 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. From-SVN: r254630 --- diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 0b547335bee..7e4093a41e7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,26 @@ +2017-11-10 Martin Sebor + + 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. + 2017-11-10 Martin Liska PR gcov-profile/82702 diff --git a/gcc/ada/ChangeLog b/gcc/ada/ChangeLog index 35062ddc50f..7806ad55ed0 100644 --- a/gcc/ada/ChangeLog +++ b/gcc/ada/ChangeLog @@ -1,3 +1,9 @@ +2017-11-10 Martin Sebor + + PR c/81117 + * ada/adadecode.c (__gnat_decode): Use memcpy instead of strncpy. + * ada/argv.c (__gnat_fill_arg, __gnat_fill_env): Same. + 2017-11-10 Eric Botcazou * gcc-interface/utils.c (convert) : Add comment and do diff --git a/gcc/ada/adadecode.c b/gcc/ada/adadecode.c index 8c9c7ab7a88..0cbef8123f9 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 430404e3aa4..aee0f886443 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 a677705acad..650de0d9aca 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 4ae70566f59..cf3fc17ac08 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/ChangeLog b/gcc/c-family/ChangeLog index 373db2d1994..7e47cb8cf49 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,10 @@ +2017-11-10 Martin Sebor + + 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. + 2017-11-06 Martin Liska PR middle-end/82404 diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 24077c72ae9..a76fae7f8fc 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 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 09ef6856cf9..6cfded97e24 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 % 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 9ab31f0e153..479ae63bb0e 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 e897d93070a..44273284483 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/ChangeLog b/gcc/fortran/ChangeLog index fb067aa3d6f..6578f7d4197 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,8 @@ +2017-11-10 Martin Sebor + + PR c/81117 + * gcc/fortran/decl.c (build_sym): Use strcpy instead of strncpy. + 2017-11-10 Paul Thomas PR fortran/82934 diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index 1a2d8f004ca..11264e757e6 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 85fd3971946..adb6f3baf93 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 (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 (stmt); + + warning_at (loc, OPT_Wstringop_truncation, + (tree_int_cst_equal (size_one_node, len) + ? G_("%G%qD output truncated copying %E byte " + "from a string of length %E") + : G_("%G%qD output truncated copying %E bytes " + "from a string of length %E")), + call, fndecl, len, slen); + } + else if (tree_int_cst_equal (len, slen)) + { + tree decl = dest; + if (TREE_CODE (decl) == SSA_NAME) + { + gimple *def_stmt = SSA_NAME_DEF_STMT (decl); + if (is_gimple_assign (def_stmt)) + { + tree_code code = gimple_assign_rhs_code (def_stmt); + if (code == ADDR_EXPR || code == VAR_DECL) + decl = gimple_assign_rhs1 (def_stmt); + } + } + + if (TREE_CODE (decl) == ADDR_EXPR) + decl = TREE_OPERAND (decl, 0); + + if (TREE_CODE (decl) == COMPONENT_REF) + decl = TREE_OPERAND (decl, 1); + + tree fndecl = gimple_call_fndecl (stmt); + gcall *call = as_a (stmt); + + if (!DECL_P (decl) + || !lookup_attribute ("nonstring", DECL_ATTRIBUTES (decl))) + warning_at (loc, OPT_Wstringop_truncation, + (tree_int_cst_equal (size_one_node, len) + ? G_("%G%qD output truncated before terminating nul " + "copying %E byte from a string of the same " + "length") + : G_("%G%qD output truncated before terminating nul " + "copying %E bytes from a string of the same " + "length")), + call, fndecl, len); + } + /* OK transform into builtin memcpy. */ - fn = builtin_decl_implicit (BUILT_IN_MEMCPY); + tree fn = builtin_decl_implicit (BUILT_IN_MEMCPY); if (!fn) return false; @@ -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 37f2248f224..c986a732004 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/ChangeLog b/gcc/objc/ChangeLog index 7d865928999..f87294be8f5 100644 --- a/gcc/objc/ChangeLog +++ b/gcc/objc/ChangeLog @@ -1,3 +1,8 @@ +2017-11-10 Martin Sebor + + PR c/81117 + * objc-encoding.c (encode_type): Use memcpy instead of strncpy. + 2017-10-31 David Malcolm * objc-gnu-runtime-abi-01.c (objc_gnu_runtime_abi_01_init): Use diff --git a/gcc/objc/objc-encoding.c b/gcc/objc/objc-encoding.c index 9f46d57ac8c..f9d8d477d76 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/ChangeLog b/gcc/testsuite/ChangeLog index 72dea0a92a0..948c8b148a4 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,17 @@ +2017-11-10 Martin Sebor + + 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. + * gcc/testsuite/gcc.dg/builtin-stpncpy.c: Adjust. + * g++.dg/torture/Wsizeof-pointer-memaccess1.C: Same. + * g++.dg/torture/Wsizeof-pointer-memaccess2.C: Same. + * gcc.dg/torture/pr63554.c: Same. + * gcc.dg/Walloca-1.c: Disable macro tracking. + 2017-11-10 Jakub Jelinek PR tree-optimization/82929 diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c index 895a50e2677..f7bfa35913c 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 00000000000..97598c42346 --- /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 00000000000..53f5166f30a --- /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 00000000000..c536a13b78e --- /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 00000000000..10a66887fa2 --- /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 00000000000..6e273e785a0 --- /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 c72532be4f4..5bc5c4ca859 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 a216f470333..f2c864b0b24 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 ad39373fb9f..85e9160e845 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/builtin-stpncpy.c b/gcc/testsuite/gcc.dg/builtin-stpncpy.c index e4290d5635c..920079892dd 100644 --- a/gcc/testsuite/gcc.dg/builtin-stpncpy.c +++ b/gcc/testsuite/gcc.dg/builtin-stpncpy.c @@ -1,6 +1,6 @@ /* PR tree-optimization/80669 - Bad -Wstringop-overflow warnings for stpncpy { dg-do compile } - { dg-options "-O2 -Wall" } */ + { dg-options "-O2 -Wall -Wno-stringop-truncation" } */ #define SIZE_MAX __SIZE_MAX__ @@ -18,7 +18,9 @@ size_t range (size_t min, size_t max) return val < min || max < val ? min : val; } -/* Verify that no warning is issued for stpncpy with constant size. */ +/* Verify that no -Wstringop-overflow warning is issued for stpncpy + with constant size. (Some tests cause -Wstringop-truncation and + that's expected). */ void test_cst (char *d) { __builtin_stpncpy (d, "123", 0); @@ -37,7 +39,8 @@ void test_cst (char *d) } -/* Verify that no warning is issued for stpncpy with size in some range. */ +/* Verify that no -Wstringop-overflow warning is issued for stpncpy + with size in some range. */ void test_rng (char *d) { #define R(min, max) range (min, max) diff --git a/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c b/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c index f9bc57c4e86..cd9dc72decb 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 fa06c5a55d1..9162266da2c 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 4ec0dacf38a..2efa18275fd 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 *decl_to_stridxlist_htab; +typedef std::pair stridx_strlenloc; +static hash_map strlen_to_stridx; + /* Obstack for struct stridxlist and struct decl_stridxlist_map. */ static struct obstack stridx_obstack; @@ -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; }