[PATCH] Compile time buffer overflow checks for strncat

[Jakub Jelinek]

Hi!

This patch adds compile time buffer overflow -D_FORTIFY_SOURCE
checks for strncat.  While strcat and most of the other string functions
were already reporting obvious buffer overflow at compile time in addition
to runtime, strncat wasn't.  E.g.
char p[2]; *p = 0; strcat (p, "ab"); would be reported already at compile
time, while
char p[2]; *p = 0; strcat (p, "abcde", 2); which does exactly the same
thing would only die at runtime.  If the second argument has known string
length, then "will always overflow" warning is emitted if the overflow
is proven and a new warning, "might overflow" warning has been added, when
the 3rd strncat argument is larger or equal to the size of the buffer,
but src has unknown string length.  If the string happens to be shorter,
then it won't overflow, still this is something that is very desirable
to warn about, it doesn't make any sense to put there a length limit
that will result in buffer overflow if the source is large enough.

Unfortunately, during testing I found that while convert_to_gimple_builtin
annotates the new statements with locus, set_rhs does not, so if
fold_builtin transforms one call into a different one, which is still valid
gimple RHS, the locus is lost.  So, the tree-ssa-propagate.c bit was
added to fix this up.

Ok for trunk?

2006-09-18  Jakub Jelinek  <jakub@redhat.com>

	* tree-ssa-propagate.c (set_rhs): Copy EXPR_LOCATION if
	needed.

	* builtins.c (expand_builtin, maybe_emit_chk_warning): Handle
	BUILT_IN_STRNCAT_CHK.

	* gcc.dg/builtin-strncat-chk-1.c: New test.

--- gcc/tree-ssa-propagate.c.jj	2006-04-06 11:33:59.000000000 +0200
+++ gcc/tree-ssa-propagate.c	2006-09-18 14:49:57.000000000 +0200
@@ -591,6 +591,13 @@ set_rhs (tree *stmt_p, tree expr)
   else if (code == COMPOUND_EXPR)
     return false;
 
+  if (EXPR_HAS_LOCATION (stmt)
+      && EXPR_P (expr)
+      && ! EXPR_HAS_LOCATION (expr)
+      && TREE_SIDE_EFFECTS (expr)
+      && TREE_CODE (expr) != LABEL_EXPR)
+    SET_EXPR_LOCATION (expr, EXPR_LOCATION (stmt));
+
   switch (TREE_CODE (stmt))
     {
     case RETURN_EXPR:
--- gcc/builtins.c.jj	2006-09-02 08:54:22.000000000 +0200
+++ gcc/builtins.c	2006-09-18 16:54:57.000000000 +0200
@@ -6437,6 +6437,7 @@ expand_builtin (tree exp, rtx target, rt
     case BUILT_IN_STPCPY_CHK:
     case BUILT_IN_STRNCPY_CHK:
     case BUILT_IN_STRCAT_CHK:
+    case BUILT_IN_STRNCAT_CHK:
     case BUILT_IN_SNPRINTF_CHK:
     case BUILT_IN_VSNPRINTF_CHK:
       maybe_emit_chk_warning (exp, fcode);
@@ -10128,6 +10129,11 @@ maybe_emit_chk_warning (tree exp, enum b
       arg_mask = 6;
       is_strlen = 1;
       break;
+    case BUILT_IN_STRNCAT_CHK:
+    /* For __strncat_chk the warning will be emitted only if overflowing
+       by at least strlen (dest) + 1 bytes.  */
+      arg_mask = 12;
+      break;
     case BUILT_IN_STRNCPY_CHK:
       arg_mask = 12;
       break;
@@ -10165,6 +10171,22 @@ maybe_emit_chk_warning (tree exp, enum b
       if (! len || ! host_integerp (len, 1) || tree_int_cst_lt (len, size))
 	return;
     }
+  else if (fcode == BUILT_IN_STRNCAT_CHK)
+    {
+      tree src = TREE_VALUE (TREE_CHAIN (arglist));
+      if (! src || ! host_integerp (len, 1) || tree_int_cst_lt (len, size))
+	return;
+      src = c_strlen (src, 1);
+      if (! src || ! host_integerp (src, 1))
+	{
+	  locus = EXPR_LOCATION (exp);
+	  warning (0, "%Hcall to %D might overflow destination buffer",
+		   &locus, get_callee_fndecl (exp));
+	  return;
+	}
+      else if (tree_int_cst_lt (src, size))
+	return;
+    }
   else if (! host_integerp (len, 1) || ! tree_int_cst_lt (size, len))
     return;
 
--- gcc/testsuite/gcc.dg/builtin-strncat-chk-1.c.jj	2006-09-18 13:07:54.000000000 +0200
+++ gcc/testsuite/gcc.dg/builtin-strncat-chk-1.c	2006-09-18 16:55:09.000000000 +0200
@@ -0,0 +1,38 @@
+/* Test whether buffer overflow warnings for __strncat_chk builtin
+   are emitted properly.  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -std=gnu99" } */
+
+extern void abort (void);
+
+#include "../gcc.c-torture/execute/builtins/chk.h"
+
+char buf1[20];
+char *q;
+
+void
+test (int arg, ...)
+{
+  char *p = &buf1[10];
+
+  *p = 0;
+  strncat (p, "abcdefg", 9);
+  *p = 0;
+  strncat (p, "abcdefghi", 9);
+  *p = 0;
+  strncat (p, "abcdefghij", 9);
+  *p = 0;
+  strncat (p, "abcdefghi", 10);
+  *p = 0;
+  strncat (p, "abcdefghij", 10); /* { dg-warning "will always overflow" } */
+  *p = 0;
+  strncat (p, "abcdefgh", 11);
+  *p = 0;
+  strncat (p, "abcdefghijkl", 11); /* { dg-warning "will always overflow" } */
+  *p = 0;
+  strncat (p, q, 9);
+  *p = 0;
+  strncat (p, q, 10); /* { dg-warning "might overflow" } */
+  *p = 0;
+  strncat (p, q, 11); /* { dg-warning "might overflow" } */
+}

	Jakub