Hi,

as analyzed by Jakub in the audit trail, after changes made by Mark back in 2005, constant_value_1 doesn't return aggregate constants for fear of inadvertent copies (in general their addresses must be the same everywhere). However, for the purpose of format checking in check_format_arg it is safe to do that, and necessary, thus Jakub suggested adding a parameter to decl_constant_value controlling that behavior. Which is what I tried to do with the below, tested x86_64-linux.

Ok for mainline?

Thanks,
Paolo.

/////////////////////
/c-family
2011-10-05  Paolo Carlini  <paolo.carl...@oracle.com>

        PR c++/38980
        * c-common.h (decl_constant_value): Add bool parameter.
        * c-common.c (decl_constant_value_for_optimization): Adjust call.
        * c-format.c (check_format_arg): Likewise.

2011-10-05  Paolo Carlini  <paolo.carl...@oracle.com>

        PR c++/38980
        * c-typeck.c (decl_constant_value): Adjust definition.

/cp
2011-10-05  Paolo Carlini  <paolo.carl...@oracle.com>

        PR c++/38980
        * typeck.c (decay_conversion): Adjust decl_constant_value call.
        * call.c (convert_like_real): Likewise.
        * init.c (constant_value_1): Add bool parameter.
        (integral_constant_value): Adjust call.
        (decl_constant_value): Adjust definition.

/testsuite
2011-10-05  Paolo Carlini  <paolo.carl...@oracle.com>

        PR c++/38980
        * g++.dg/warn/format5.C: New.
Index: c-family/c-format.c
===================================================================
--- c-family/c-format.c (revision 179540)
+++ c-family/c-format.c (working copy)
@@ -1529,7 +1529,7 @@ check_format_arg (void *ctx, tree format_tree,
     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
+      && (array_init = decl_constant_value (format_tree, true)) != format_tree
       && TREE_CODE (array_init) == STRING_CST)
     {
       /* Extract the string constant initializer.  Note that this may include
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c (revision 179540)
+++ c-family/c-common.c (working copy)
@@ -1443,7 +1443,7 @@ decl_constant_value_for_optimization (tree exp)
       || DECL_MODE (exp) == BLKmode)
     return exp;
 
-  ret = decl_constant_value (exp);
+  ret = decl_constant_value (exp, false);
   /* Avoid unwanted tree sharing between the initializer and current
      function's body where the tree can be modified e.g. by the
      gimplifier.  */
Index: c-family/c-common.h
===================================================================
--- c-family/c-common.h (revision 179540)
+++ c-family/c-common.h (working copy)
@@ -886,7 +886,7 @@ extern tree default_conversion (tree);
 
 extern tree common_type (tree, tree);
 
-extern tree decl_constant_value (tree);
+extern tree decl_constant_value (tree, bool);
 
 /* Handle increment and decrement of boolean types.  */
 extern tree boolean_increment (enum tree_code, tree);
Index: testsuite/g++.dg/warn/format5.C
===================================================================
--- testsuite/g++.dg/warn/format5.C     (revision 0)
+++ testsuite/g++.dg/warn/format5.C     (revision 0)
@@ -0,0 +1,12 @@
+// PR c++/38980
+// { dg-options "-Wformat" }
+
+extern "C"
+int printf(const char *format, ...) __attribute__((format(printf, 1, 2) ));
+
+const char fmt1[] = "Hello, %s";
+
+void f()
+{
+  printf(fmt1, 3); // { dg-warning "expects argument" }
+}
Index: cp/typeck.c
===================================================================
--- cp/typeck.c (revision 179540)
+++ cp/typeck.c (working copy)
@@ -1827,7 +1827,7 @@ decay_conversion (tree exp)
   /* FIXME remove? at least need to remember that this isn't really a
      constant expression if EXP isn't decl_constant_var_p, like with
      C_MAYBE_CONST_EXPR.  */
-  exp = decl_constant_value (exp);
+  exp = decl_constant_value (exp, /*return_aggregate_cst_ok_p=*/false);
   if (error_operand_p (exp))
     return error_mark_node;
 
Index: cp/init.c
===================================================================
--- cp/init.c   (revision 179540)
+++ cp/init.c   (working copy)
@@ -1794,10 +1794,11 @@ build_offset_ref (tree type, tree member, bool add
    constant initializer, return the initializer (or, its initializers,
    recursively); otherwise, return DECL.  If INTEGRAL_P, the
    initializer is only returned if DECL is an integral
-   constant-expression.  */
+   constant-expression.  If RETURN_AGGREGATE_CST_OK_P, it is ok to
+   return an aggregate constant.  */
 
 static tree
-constant_value_1 (tree decl, bool integral_p)
+constant_value_1 (tree decl, bool integral_p, bool return_aggregate_cst_ok_p)
 {
   while (TREE_CODE (decl) == CONST_DECL
         || (integral_p
@@ -1834,11 +1835,12 @@ static tree
       if (!init
          || !TREE_TYPE (init)
          || !TREE_CONSTANT (init)
-         || (!integral_p
-             /* Do not return an aggregate constant (of which
-                string literals are a special case), as we do not
-                want to make inadvertent copies of such entities,
-                and we must be sure that their addresses are the
+         || (!integral_p && !return_aggregate_cst_ok_p
+             /* Unless RETURN_AGGREGATE_CST_OK_P is true, do not
+                return an aggregate constant (of which string
+                literals are a special case), as we do not want
+                to make inadvertent copies of such entities, and
+                we must be sure that their addresses are the
                 same everywhere.  */
              && (TREE_CODE (init) == CONSTRUCTOR
                  || TREE_CODE (init) == STRING_CST)))
@@ -1856,7 +1858,8 @@ static tree
 tree
 integral_constant_value (tree decl)
 {
-  return constant_value_1 (decl, /*integral_p=*/true);
+  return constant_value_1 (decl, /*integral_p=*/true,
+                          /*return_aggregate_cst_ok_p=*/false);
 }
 
 /* A more relaxed version of integral_constant_value, used by the
@@ -1864,10 +1867,10 @@ integral_constant_value (tree decl)
    purposes.  */
 
 tree
-decl_constant_value (tree decl)
+decl_constant_value (tree decl, bool return_aggregate_cst_ok_p)
 {
-  return constant_value_1 (decl,
-                          /*integral_p=*/processing_template_decl);
+  return constant_value_1 (decl, /*integral_p=*/processing_template_decl,
+                          return_aggregate_cst_ok_p);
 }
 
 /* Common subroutines of build_new and build_vec_delete.  */
Index: cp/call.c
===================================================================
--- cp/call.c   (revision 179540)
+++ cp/call.c   (working copy)
@@ -5703,8 +5703,10 @@ convert_like_real (conversion *convs, tree expr, t
         leave it as an lvalue.  */
       if (inner >= 0)
         {   
-          expr = decl_constant_value (expr);
-          if (expr == null_node && INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P 
(totype))
+          expr = decl_constant_value (expr,
+                                     /*return_aggregate_cst_ok_p=*/false);
+          if (expr == null_node
+             && INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (totype))
             /* If __null has been converted to an integer type, we do not
                want to warn about uses of EXPR as an integer, rather than
                as a pointer.  */
Index: c-typeck.c
===================================================================
--- c-typeck.c  (revision 179540)
+++ c-typeck.c  (working copy)
@@ -1742,7 +1742,7 @@ c_size_in_bytes (const_tree type)
 /* Return either DECL or its known constant value (if it has one).  */
 
 tree
-decl_constant_value (tree decl)
+decl_constant_value (tree decl, bool return_aggregate_cst_ok_p 
ATTRIBUTE_UNUSED)
 {
   if (/* Don't change a variable array bound or initial value to a constant
         in a place where a variable is invalid.  Note that DECL_INITIAL

Reply via email to