public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] enhance overflow and truncation detection in strncpy and strncat (PR 81117)
@ 2017-07-08 20:45 Martin Sebor
  2017-07-18  2:51 ` [PING] " Martin Sebor
  2017-07-31 17:29 ` Jeff Law
  0 siblings, 2 replies; 39+ messages in thread
From: Martin Sebor @ 2017-07-08 20:45 UTC (permalink / raw)
  To: Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 2869 bytes --]

PR 81117 asks for improved detection of common misuses(*) of
strncpy and strncat.  The attached patch is my solution.  It
consists of three related sets of changes:

1) Adds a new option, -Wstringop-truncation, that diagnoses calls
to strncpy, and stpncpy (and also strncat) that truncate the copy.
This helps highlight the common but incorrect assumption that
the first two functions NUL-terminate the copy (see, for example,
CWE-170)  For strncat, it helps detect cases of inadvertent
truncation of the source string by passing in a bound that's
less than or equal to its length.

2) Enhances -Wstringon-overflow to diagnose calls of the form
strncpy(D, S, N) where the bound N depends on a call to strlen(S).
This misuse is common in legacy code when, often in response to
the adoption of a secure coding initiative, while replacing uses
of strcpy with strncpy, the engineer either makes a mistake, or
doesn't have a good enough understanding of how the function works,
or does only the bare minimum to satisfy the requirement to avoid
using strcpy without actually improving anything.

3) Enhances -Wsizeof-pointer-memaccess to also warn about uses of
the functions to copy an array to a destination of an unknown size
that specify the size of the array as the bound.  Given the
pervasive [mis]use of strncpy to bound the copy to the size of
the destination, instances like this suggest a bug: a possible
buffer overflow due to an excessive bound (see, for example,
CWE-806).  In cases when the call is safe, it's equivalent to
the corresponding call to memcpy which is clearer and can be
more efficient.

Martin

PS By coincidence rather than by intent, the strncat warnings
are a superset of Clang's -Wstrncat-size.  AFAICS, Clang only
warns when the destination is an array of known size and
doesn't have a corresponding warning for strncpy.

[*] Here's some background into these misuses.

The purpose of the historical strncpy function introduced in V7
UNIX was to completely fill an array of chars with data, either
by copying an initial portion of a source string, or by clearing
it.  I.e., its purpose wasn't to create NUL-terminated strings.
An example of its use was to fill the directory entry d_name
array (dirent::d_name) with the name of a file.

The original purpose of the strncat function, on the other hand,
was to append a not necessarily NUL-terminated array of chars
to a string to form a NUL-terminated concatenation of the two.
An example use case is appending a directory entry (struct
dirent::d_name) that need not be NUL-terminated, to form
a pathname which does.

Largely due to a misunderstanding of the functions' purpose they
have become commonly used (and misused) to make "safe," bounded
string copies by safeguarding against accidentally overflowing
the destination.  This has led to great many bugs and security 
vulnerabilities.


[-- Attachment #2: gcc-81117.diff --]
[-- Type: text/x-patch, Size: 52667 bytes --]

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.
	(check_strncpy_sizes): New function.
	(expand_builtin_strncpy): Call check_strncpy_sizes.
	* doc/invoke.texi (-Wsizeof-pointer-memaccess): Document enhancement.
	(-Wstringop-truncation): Document new option.
	* 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.
	(is_strlen_related_p): New function.
	(check_builtin_strxncpy, check_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.

gcc/ada/ChangeLog:

	PR c/81117
	* adadecode.c (__gnat_decode): Replace pointless strncpy with
	memcpy.
	* argv.c (__gnat_fill_arg): Same.

gcc/c-family/ChangeLog:

	PR c/81117
	* c-common.c (resort_sorted_fields): Replace pointless strncpy
	with memcpy.
	* c-warn.c (sizeof_pointer_memaccess_warning): Handle arrays.
	* c.opt (-Wstriingop-truncation): New option.

gcc/fortran/ChangeLog:

	PR c/81117
	* decl.c (build_sym): Replace pointless strncpy with strcpy.

gcc/objc/ChangeLog:

	PR c/81117
	* objc-encoding.c (encode_type): Replace pointless strncpy with
	memcpy.

gcc/testsuite/ChangeLog:

	PR c/81117
	* c-c++-common/Wsizeof-pointer-memaccess3.c: New test.
	* c-c++-common/Wstringop-overflow.c: New test.
	* c-c++-common/Wstringop-truncation.c: New test.
	* g++.dg/torture/Wsizeof-pointer-memaccess1.C: Adjust.
	* g++.dg/torture/Wsizeof-pointer-memaccess2.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 608993a..706f051 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -3300,13 +3300,43 @@ check_sizes (int opt, tree exp, tree size, tree maxlen, tree src, tree objsize)
    the size of the object if successful or NULL when the size cannot
    be determined.  */
 
-static inline tree
+static 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 TYPE_SIZE_UNIT (type);
+
   return NULL_TREE;
 }
 
@@ -3949,6 +3979,36 @@ 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;
+
+  int cmp = tree_int_cst_compare (dstsize, len);
+
+  if (cmp == 0)
+    warning_at (EXPR_LOCATION (exp), OPT_Wstringop_truncation,
+		"%K%qD specified bound %E equals destination size",
+		exp, get_callee_fndecl (exp), len);
+  else if (cmp < 0)
+    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.  */
 
@@ -3967,16 +4027,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/c-family/c-common.c b/gcc/c-family/c-common.c
index b4217f3..5fc4d24 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5942,10 +5942,10 @@ resort_sorted_fields (void *obj,
 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 b9378c2..4b04d81 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 05766c4..4b9c975 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -732,10 +732,15 @@ Warn about buffer overflow in string manipulation functions like memcpy
 and strcpy.
 
 Wstringop-overflow=
-C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Init(2) Warning LangEnabledBy(C ObjC C++ ObjC++, Wall) IntegerRange(0, 4)
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Init(2) Warning LangEnabledBy(C ObjC C++ ObjC++, Wall, 2, 0) IntegerRange(0, 4)
 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 buffer overflow in string manipulation functions like memcpy
+and strcpy.
+
 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 d0b9050..e4208d9 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
@@ -5170,6 +5170,57 @@ 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-overflow
+@opindex Wno-stringop-overflow
+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 the length
+of the source string as the bound.  If the destination contains a non-empty
+string the copy of the source will be truncated.  Therefore, the call is
+diagnosed.  To avoid the warning use @code{strlen (d) - 4)} as the bound;
+
+@smallexample
+void append (char *d)
+@{
+  strncat (d, ".txt", 4);
+@}
+@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, if the goal is to avoid overflowing
+the destination, provide the size of the destination buffer if it's known
+and append the terminating NUL, or otherwise 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 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=
@@ -6154,11 +6205,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
+
+Th @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 bd31070..ee91504 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -1417,11 +1417,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 d94dc9c..5d021f9 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,25 @@ 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))
     {
+      /* 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,
+		    "%qD destination unchanged after copying no bytes "
+		    "from a string of length %E",
+		    gimple_call_fndecl (stmt), slen);
+      else
+	warning_at (loc, OPT_Wstringop_truncation,
+		    "%qD destination unchanged after copying no bytes",
+		    gimple_call_fndecl (stmt));
+
       replace_call_with_value (gsi, dest);
       return true;
     }
@@ -1542,16 +1558,36 @@ 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))
+    warning_at (loc, OPT_Wstringop_truncation,
+		(tree_int_cst_equal (size_one_node, 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")),
+		gimple_call_fndecl (stmt), len, slen);
+  else if (tree_int_cst_equal (len, slen))
+    warning_at (loc, OPT_Wstringop_truncation,
+		(tree_int_cst_equal (size_one_node, len)
+		 ? G_("%qD output truncated before terminating nul "
+		      "copying %E byte from a string of the same "
+		      "length")
+		 : G_("%qD output truncated before terminating nul "
+		      "copying %E bytes from a string of the same "
+		      "length")),
+		gimple_call_fndecl (stmt), 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 +1596,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 +1893,68 @@ 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)
+	    {
+	      /* 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_("%qD specified bound %E equals "
+					"destination size")
+				   : G_("%qD specified bound %E exceeds "
+					"destination size %wu"),
+				   gimple_call_fndecl (stmt), len, dstsize);
+	      if (nowarn)	
+		gimple_set_no_warning (stmt, true);
+	    }
+	}
+
+      if (!nowarn && cmpsrc <= 0)
+	{
+	  /* 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_truncation,
+			  cmpsrc == 0
+			  ? G_("%qD specified bound %E equals source length")
+			  : G_("%qD specified bound %E is less than source "
+			       "length %u"),
+			  gimple_call_fndecl (stmt), len, srclen))
+	    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 488f8c8..f2dcece 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/objc/objc-encoding.c b/gcc/objc/objc-encoding.c
index 2a2dfa5..e5d4f38 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-memaccess3.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess3.c
new file mode 100644
index 0000000..cf0ca67
--- /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-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..4c3bd73
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-overflow.c
@@ -0,0 +1,157 @@
+/* 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 three 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);
+  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..240f6aa
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
@@ -0,0 +1,212 @@
+/* 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 (char*);
+
+#define S4 "123"
+const char a4[] = "123";
+
+
+typedef struct Dest
+{
+  char a[5];
+} Dest;
+
+char dest[7];
+
+/* 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 (dest, S4, 2);                /* { dg-warning "specified bound 2 is less than source length 3" } */
+  T (dest, S4, 3);                /* { dg-warning "specified bound 3 equals source length" } */
+
+  T (dest, a4, 1);                /* { dg-warning "specified bound 1 is less than source length 3" } */
+  T (dest, a4, 3);                /* { dg-warning "specified bound 3 equals source length" } */
+
+  T (pd->a, S4, 2);               /* { dg-warning "specified bound 2 is less than source length" } */
+  T (pd->a, S4, 1);               /* { dg-warning "specified bound 1 is less than source length 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, S4, strlen (S4) + 1);
+  T (d, S4, strlen (S4) + i);
+
+  T (d, S4, 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 (a4) + 1);
+  T (d, S4, strlen (a4) + i);
+
+  {
+    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)
+{
+  T (dest, s, 7);                 /* { dg-warning "specified bound 7 equals destination size" } */
+  T (dest, s, sizeof dest);       /* { dg-warning "specified bound 7 equals destination size" } */
+
+  /* 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 (dest, s, 8);
+
+  T (dest + 1, s, 6);             /* { dg-warning "specified bound 6 equals destination size" } */
+  T (dest + 6, s, 1);             /* { dg-warning "specified bound 1 equals destination size" } */
+
+  T (pd->a, s, 5);                /* { dg-warning "specified bound 5 equals destination size" } */
+  T (pd->a, s, sizeof pd->a);     /* { dg-warning "specified bound 5 equals destination size" } */
+
+  /* Same asbove, diagnosed by -Wstringop-overflow.  */
+  T (pd->a, s, 6);
+}
+
+typedef struct Flex
+{
+  size_t n;
+  char a0[0];
+  char a[];
+} Flex;
+
+extern char array[];
+
+/* Verify that no warning is issued for array of unknown bound, flexible
+   array members, or zero-length arrays.  */
+
+void test_strncpy_flexarray (Flex *pd, const char* s)
+{
+  T (array, s, 7);
+  T (array, s, 123);
+
+  T (pd->a0, s, 1);
+  T (pd->a0, s, 1234);
+
+  T (pd->a, s, 5);
+  T (pd->a, s, 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->a, s, 5);                /* { dg-warning "specified bound 5 equals destination size" } */
+  T (pd->a, s, sizeof pd->a);     /* { dg-warning "specified bound 5 equals destination size" } */
+}
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/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index b0563fe..1f4b0f3 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -40,11 +40,14 @@ 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"
 
 /* A vector indexed by SSA_NAME_VERSION.  0 means unknown, positive value
    is an index into strinfo vector, negative value stands for
@@ -146,6 +149,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 +1203,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 +1249,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 +1618,133 @@ 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 (potentiall) related expressions to strlen.
+	 Others are not.  */
+      return is_strlen_related_p (src, rhs1);
+    }
+
+  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
+check_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)
+    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
+check_builtin_strncat (built_in_function bcode, gimple_stmt_iterator *gsi)
+{
+  /* Same as stxncpy().  */
+  check_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 +2651,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:
+	    check_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:
+	    check_builtin_stxncpy (DECL_FUNCTION_CODE (callee), gsi);
+	    break;
+
 	  case BUILT_IN_MEMCPY:
 	  case BUILT_IN_MEMCPY_CHK:
 	  case BUILT_IN_MEMPCPY:
@@ -2575,6 +2727,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 +2982,8 @@ pass_strlen::execute (function *fun)
   laststmt.len = NULL_TREE;
   laststmt.stridx = 0;
 
+  strlen_to_stridx.empty ();
+
   return 0;
 }
 


^ permalink raw reply	[flat|nested] 39+ messages in thread
end of thread, other threads:[~2017-11-15 15:43 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-08 20:45 [PATCH] enhance overflow and truncation detection in strncpy and strncat (PR 81117) Martin Sebor
2017-07-18  2:51 ` [PING] " Martin Sebor
2017-07-25  3:10   ` [PING #2] " Martin Sebor
2017-07-31 17:29 ` Jeff Law
2017-07-31 19:42   ` Martin Sebor
2017-08-02 16:59     ` Jeff Law
2017-08-06 20:07       ` [PATCH 1/4] " Martin Sebor
2017-08-10  5:02         ` Jeff Law
2017-08-14 19:21           ` Martin Sebor
2017-08-06 20:07       ` [PATCH 3/4] " Martin Sebor
2017-08-10  7:17         ` Jeff Law
2017-08-10  7:39           ` Richard Biener
2017-08-10 20:21           ` Martin Sebor
2017-08-15  3:06             ` Martin Sebor
2017-08-23 21:11               ` [PING] " Martin Sebor
2017-08-29  5:07                 ` [PING 2] " Martin Sebor
2017-09-19 15:44                   ` [PING 3] " Martin Sebor
2017-09-26  2:27                     ` [PING 4] " Martin Sebor
2017-10-02 22:15             ` Jeff Law
2017-10-21  0:26               ` Martin Sebor
2017-11-04  3:49                 ` Jeff Law
2017-11-10  0:17                   ` Martin Sebor
2017-11-10  0:31                     ` Jeff Law
2017-11-14  9:24         ` [testsuite, committed] Require alloca for c-c++-common/Wstringop-truncation.c Tom de Vries
2017-11-15 15:30         ` [testsuite, committed] Compile strncpy-fix-1.c with -Wno-stringop-truncation Tom de Vries
2017-11-15 15:58           ` Martin Sebor
2017-08-06 20:07       ` [PATCH] enhance overflow and truncation detection in strncpy and strncat (PR 81117) Martin Sebor
2017-08-09 19:21         ` Jeff Law
2017-08-06 20:07       ` [PATCH 4/4] " Martin Sebor
2017-08-06 20:07       ` [PATCH 2/4] " Martin Sebor
2017-08-10  6:39         ` Jeff Law
2017-08-14 18:04           ` Martin Sebor
2017-08-14 18:29             ` Joseph Myers
2017-08-14 19:26               ` Martin Sebor
2017-08-14 20:41                 ` Joseph Myers
2017-08-14 20:44                   ` Martin Sebor
2017-08-15  3:03                     ` Joseph Myers
2017-11-10 23:03         ` Marc Glisse
2017-11-11 21:10           ` Martin Sebor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).