Re: Patch: format checking const arrays

["Joseph S. Myers" <jsm28 at cam dot ac dot uk>]

On Thu, 19 Oct 2000, Mark Mitchell wrote:

> The point is that there are several things going on at once:
> 
>   - You want to add some new functionality.
> 
>   - We don't want to duplicate code that is already in the compiler.
> 
>   - We don't want to peturb things unduly, especially given that
>     we are trying to stabilize the tree as we get closer to branching
>     for the release.  That means we need to keep the behavior as
>     is.
> 
>   - We want to be able to easily attack the problem in the future,
>     rather than spread it out in various places.
> 
> That means that the right thing to do is add the wrapper function.

Here is a patch version which adds a wrapper function
decl_constant_value_for_broken_optimization.  Note: the comment added
that the function is broken and should be removed before GCC 3.1 is in
the good tradition of the many similar comments in the compiler saying
that this or that should be looked at / fixed before 2.5 or some such
long ago version.  (I'm half minded to open priority "high" PRs for
every such comment referencing old versions on the basis that if
something was supposed to be fixed for a long ago version it ought at
least to be looked at, or non-current comments removed or updated,
before GCC 3.0.)

Bootstrapped with no regressions on i686-pc-linux-gnu.  OK to commit?

gcc/ChangeLog:
2000-10-20  Joseph S. Myers  <jsm28@cam.ac.uk>

	* c-common.c (check_format_info_recurse): Extract string constant
	initializers from non-volatile constant arrays and check them as
	formats.
	* c-typeck.c (decl_constant_value): Don't check pedantic or check
	for DECL_MODE (decl) != BLKmode.
	(decl_constant_value_for_broken_optimization): New function which
	includes these checks.
	(default_conversion, convert_for_assignment, digest_init): Use
	decl_constant_value_for_broken_optimization instead of
	decl_constant_value.

gcc/testsuite/ChangeLog:
2000-10-20  Joseph S. Myers  <jsm28@cam.ac.uk>

	* gcc.dg/format-array-1.c: New test.

--- c-common.c.orig	Wed Oct 18 21:27:38 2000
+++ c-common.c	Wed Oct 18 21:52:49 2000
@@ -2344,6 +2344,8 @@ check_format_info_recurse (status, res, 
 {
   int format_length;
   const char *format_chars;
+  tree array_size = 0;
+  tree array_init;
 
   if (TREE_CODE (format_tree) == NOP_EXPR)
     {
@@ -2436,6 +2438,17 @@ check_format_info_recurse (status, res, 
       return;
     }
   format_tree = TREE_OPERAND (format_tree, 0);
+  if (TREE_CODE (format_tree) == VAR_DECL
+      && TREE_CODE (TREE_TYPE (format_tree)) == ARRAY_TYPE
+      && (array_init = decl_constant_value (format_tree)) != format_tree
+      && TREE_CODE (array_init) == STRING_CST)
+    {
+      /* Extract the string constant initializer.  Note that this may include
+	 a trailing NUL character that is not in the array (e.g.
+	 const char a[3] = "foo";).  */
+      array_size = DECL_SIZE_UNIT (format_tree);
+      format_tree = array_init;
+    }
   if (TREE_CODE (format_tree) != STRING_CST)
     {
       res->number_non_literal++;
@@ -2448,6 +2461,20 @@ check_format_info_recurse (status, res, 
     }
   format_chars = TREE_STRING_POINTER (format_tree);
   format_length = TREE_STRING_LENGTH (format_tree);
+  if (array_size != 0)
+    {
+      /* Variable length arrays can't be initialized.  */
+      if (TREE_CODE (array_size) != INTEGER_CST)
+	abort ();
+      if (host_integerp (array_size, 0))
+	{
+	  HOST_WIDE_INT array_size_value = TREE_INT_CST_LOW (array_size);
+	  if (array_size_value > 0
+	      && array_size_value == (int) array_size_value
+	      && format_length > array_size_value)
+	    format_length = array_size_value;
+	}
+    }
   if (format_length < 1)
     {
       res->number_unterminated++;
--- c-typeck.c.orig	Thu Oct 19 21:00:54 2000
+++ c-typeck.c	Thu Oct 19 21:30:10 2000
@@ -54,6 +54,7 @@ static tree qualify_type		PARAMS ((tree,
 static int comp_target_types		PARAMS ((tree, tree));
 static int function_types_compatible_p	PARAMS ((tree, tree));
 static int type_lists_compatible_p	PARAMS ((tree, tree));
+static tree decl_constant_value_for_broken_optimization PARAMS ((tree));
 static tree lookup_field		PARAMS ((tree, tree, tree *));
 static tree convert_arguments		PARAMS ((tree, tree, tree, tree));
 static tree pointer_int_sum		PARAMS ((enum tree_code, tree, tree));
@@ -844,7 +845,6 @@ decl_constant_value (decl)
   if (/* Don't change a variable array bound or initial value to a constant
 	 in a place where a variable is invalid.  */
       current_function_decl != 0
-      && ! pedantic
       && ! TREE_THIS_VOLATILE (decl)
       && TREE_READONLY (decl)
       && DECL_INITIAL (decl) != 0
@@ -854,12 +854,29 @@ decl_constant_value (decl)
 	 or a variable, then re-evaluating it could give different results.  */
       && TREE_CONSTANT (DECL_INITIAL (decl))
       /* Check for cases where this is sub-optimal, even though valid.  */
-      && TREE_CODE (DECL_INITIAL (decl)) != CONSTRUCTOR
-      && DECL_MODE (decl) != BLKmode)
+      && TREE_CODE (DECL_INITIAL (decl)) != CONSTRUCTOR)
     return DECL_INITIAL (decl);
   return decl;
 }
 
+/* Return either DECL or its known constant value (if it has one), but
+   return DECL if pedantic or DECL has mode BLKmode.  This is for
+   bug-compatibility with the old behavior of decl_constant_value
+   (before GCC 3.0); every use of this function is a bug and it should
+   be removed before GCC 3.1.  It is not appropriate to use pedantic
+   in a way that affects optimization, and BLKmode is probably not the
+   right test for avoiding misoptimizations either.  */
+
+static tree
+decl_constant_value_for_broken_optimization (decl)
+     tree decl;
+{
+  if (pedantic || DECL_MODE (decl) == BLKmode)
+    return decl;
+  else
+    return decl_constant_value (decl);
+}
+
 /* Perform default promotions for C data used in expressions.
    Arrays and functions are converted to pointers;
    enumeral types or short or char, to int.
@@ -881,7 +898,7 @@ default_conversion (exp)
      address of the array produces consistent results.  */
   else if (optimize && TREE_CODE (exp) == VAR_DECL && code != ARRAY_TYPE)
     {
-      exp = decl_constant_value (exp);
+      exp = decl_constant_value_for_broken_optimization (exp);
       type = TREE_TYPE (exp);
     }
 
@@ -4019,7 +4036,7 @@ convert_for_assignment (type, rhs, errty
       || TREE_CODE (TREE_TYPE (rhs)) == FUNCTION_TYPE)
     rhs = default_conversion (rhs);
   else if (optimize && TREE_CODE (rhs) == VAR_DECL)
-    rhs = decl_constant_value (rhs);
+    rhs = decl_constant_value_for_broken_optimization (rhs);
 
   rhstype = TREE_TYPE (rhs);
   coder = TREE_CODE (rhstype);
@@ -4695,7 +4712,7 @@ digest_init (type, init, require_constan
 	}
 
       if (optimize && TREE_CODE (inside_init) == VAR_DECL)
-	inside_init = decl_constant_value (inside_init);
+	inside_init = decl_constant_value_for_broken_optimization (inside_init);
 
       /* Compound expressions can only occur here if -pedantic or
 	 -pedantic-errors is specified.  In the later case, we always want
--- testsuite/gcc.dg/format-array-1.c.orig	Fri Sep 11 11:31:59 1998
+++ testsuite/gcc.dg/format-array-1.c	Tue Oct 17 18:34:24 2000
@@ -0,0 +1,39 @@
+/* Test for format checking of constant arrays.  */
+/* Origin: Joseph Myers <jsm28@cam.ac.uk> */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99 -Wformat=2" } */
+
+extern int printf (const char *, ...);
+
+const char a1[] = "foo";
+const char a2[] = "foo%d";
+const char b[3] = "foo";
+static const char c1[] = "foo";
+static const char c2[] = "foo%d";
+char d[] = "foo";
+volatile const char e[] = "foo";
+
+void
+foo (int i, long l)
+{
+  const char p1[] = "bar";
+  const char p2[] = "bar%d";
+  static const char q1[] = "bar";
+  static const char q2[] = "bar%d";
+  printf (a1);
+  printf (a2, i);
+  printf (a2, l); /* { dg-warning "format" "wrong type with array" } */
+  printf (b); /* { dg-warning "unterminated" "unterminated array" } */
+  printf (c1);
+  printf (c2, i);
+  printf (c2, l); /* { dg-warning "format" "wrong type with array" } */
+  printf (p1);
+  printf (p2, i);
+  printf (p2, l); /* { dg-warning "format" "wrong type with array" } */
+  printf (q1);
+  printf (q2, i);
+  printf (q2, l); /* { dg-warning "format" "wrong type with array" } */
+  /* Volatile or non-constant arrays must not be checked.  */
+  printf (d); /* { dg-warning "not a string literal" "non-const" } */
+  printf ((const char *)e); /* { dg-warning "not a string literal" "volatile" } */
+}

-- 
Joseph S. Myers
jsm28@cam.ac.uk