On 4/1/22 15:14, Marek Polacek wrote:
Attribute format takes three arguments: archetype, string-index, and
first-to-check.  The last two specify the position in the function
parameter list.  r63030 clarified that "Since non-static C++ methods have
an implicit this argument, the arguments of such methods should be counted
from two, not one, when giving values for string-index and first-to-check."
Therefore one has to write

   struct D {
     D(const char *, ...) __attribute__((format(printf, 2, 3)));
   };

However -- and this is the problem in this PR -- ctors with virtual
bases also get two additional parameters: the in-charge parameter and
the VTT parameter (added in maybe_retrofit_in_chrg).  In fact we'll end up
with two clones of the ctor: an in-charge and a not-in-charge version (see
build_cdtor_clones).  That means that the argument position the user
specified in the attribute argument will refer to different arguments,
depending on which constructor we're currently dealing with.  This can
cause a range of problems: wrong errors, confusing warnings, or crashes.

This patch corrects that; for C we don't have to do anything, and in C++
we can use num_artificial_parms_for.  It would be wrong to rewrite the
attributes the user supplied, so I've added an extra parameter called
adjust_pos.

Attribute format_arg is not affected, because it requires that the
function returns "const char *" which will never be the case for cdtors.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

        PR c++/101833
        PR c++/47634

gcc/c-family/ChangeLog:

        * c-attribs.cc (positional_argument): Add new argument adjust_pos,
        use it.
        * c-common.cc (check_function_arguments): Pass fndecl to
        check_function_format.
        * c-common.h (check_function_format): Adjust declaration.
        (maybe_adjust_arg_pos_for_attribute): Add.
        (positional_argument): Adjust declaration.
        * c-format.cc (decode_format_attr): Add fndecl argument.  Pass it to
        maybe_adjust_arg_pos_for_attribute.  Adjust calls to get_constant.

I wonder about, instead of adding another parameter, allowing the current fntype parameter to be the fndecl when we have one.

And then that gets passed down into positional_argument, so we can call maybe_adjust_arg_pos_for_attribute there, and adjust the return value appropriately so we don't need the extra adjustment in get_constant?

        (handle_format_arg_attribute): Pass 0 to get_constant.
        (get_constant): Add new argument adjust_pos, use it.
        (check_function_format): Add fndecl argument.  Pass it to
        decode_format_attr.
        (handle_format_attribute): Get the fndecl from node[2].  Pass it to
        decode_format_attr.

gcc/c/ChangeLog:

        * c-objc-common.cc (maybe_adjust_arg_pos_for_attribute): New.

gcc/cp/ChangeLog:

        * tree.cc (maybe_adjust_arg_pos_for_attribute): New.

gcc/testsuite/ChangeLog:

        * g++.dg/ext/attr-format-arg1.C: New test.
        * g++.dg/ext/attr-format1.C: New test.
        * g++.dg/ext/attr-format2.C: New test.
        * g++.dg/ext/attr-format3.C: New test.
---
  gcc/c-family/c-attribs.cc                   | 14 ++++---
  gcc/c-family/c-common.cc                    |  4 +-
  gcc/c-family/c-common.h                     |  5 ++-
  gcc/c-family/c-format.cc                    | 46 +++++++++++++--------
  gcc/c/c-objc-common.cc                      |  9 ++++
  gcc/cp/tree.cc                              | 19 +++++++++
  gcc/testsuite/g++.dg/ext/attr-format-arg1.C | 26 ++++++++++++
  gcc/testsuite/g++.dg/ext/attr-format1.C     | 32 ++++++++++++++
  gcc/testsuite/g++.dg/ext/attr-format2.C     | 38 +++++++++++++++++
  gcc/testsuite/g++.dg/ext/attr-format3.C     | 15 +++++++
  10 files changed, 182 insertions(+), 26 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format-arg1.C
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format1.C
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format2.C
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format3.C

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 111a33f405a..6e17847ec9e 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -599,12 +599,15 @@ attribute_takes_identifier_p (const_tree attr_id)
     matching all C integral types except bool.  If successful, return
     POS after default conversions, if any.  Otherwise, issue appropriate
     warnings and return null.  A non-zero 1-based ARGNO should be passed
-   in by callers only for attributes with more than one argument.  */
+   in by callers only for attributes with more than one argument.
+   ADJUST_POS is used and non-zero in C++ when the function type has
+   invisible parameters generated by the compiler, such as the in-charge
+   or VTT parameters.  */
tree
  positional_argument (const_tree fntype, const_tree atname, tree pos,
                     tree_code code, int argno /* = 0 */,
-                    int flags /* = posargflags () */)
+                    int flags /* = posargflags () */, int adjust_pos /* = 0 */)
  {
    if (pos && TREE_CODE (pos) != IDENTIFIER_NODE
        && TREE_CODE (pos) != FUNCTION_DECL)
@@ -690,7 +693,7 @@ positional_argument (const_tree fntype, const_tree atname, 
tree pos,
    if (!nargs
        || !tree_fits_uhwi_p (pos)
        || ((flags & POSARG_ELLIPSIS) == 0
-         && !IN_RANGE (tree_to_uhwi (pos), 1, nargs)))
+         && !IN_RANGE (tree_to_uhwi (pos) + adjust_pos, 1, nargs)))
      {
if (argno < 1)
@@ -707,8 +710,9 @@ positional_argument (const_tree fntype, const_tree atname, 
tree pos,
      }
/* Verify that the type of the referenced formal argument matches
-     the expected type.  */
-  unsigned HOST_WIDE_INT ipos = tree_to_uhwi (pos);
+     the expected type.   Invisible parameters may have been added by
+     the compiler, so adjust the position accordingly.  */
+  unsigned HOST_WIDE_INT ipos = tree_to_uhwi (pos) + adjust_pos;
/* Zero was handled above. */
    gcc_assert (ipos != 0);
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index d034837bb5b..6f08b55d4a7 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -6069,8 +6069,8 @@ check_function_arguments (location_t loc, const_tree 
fndecl, const_tree fntype,
    /* Check for errors in format strings.  */
if (warn_format || warn_suggest_attribute_format)
-    check_function_format (fntype, TYPE_ATTRIBUTES (fntype), nargs, argarray,
-                          arglocs);
+    check_function_format (fntype, fndecl, TYPE_ATTRIBUTES (fntype), nargs,
+                          argarray, arglocs);
if (warn_format)
      check_function_sentinel (fntype, nargs, argarray);
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 52a85bfb783..db6ff07db37 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -857,7 +857,7 @@ extern void check_function_arguments_recurse (void (*)
                                              opt_code);
  extern bool check_builtin_function_arguments (location_t, vec<location_t>,
                                              tree, tree, int, tree *);
-extern void check_function_format (const_tree, tree, int, tree *,
+extern void check_function_format (const_tree, const_tree, tree, int, tree *,
                                   vec<location_t> *);
  extern bool attribute_fallthrough_p (tree);
  extern tree handle_format_attribute (tree *, tree, tree, int, bool *);
@@ -1049,6 +1049,7 @@ extern tree finish_label_address_expr (tree, location_t);
  extern tree lookup_label (tree);
  extern tree lookup_name (tree);
  extern bool lvalue_p (const_tree);
+extern int maybe_adjust_arg_pos_for_attribute (const_tree);
extern bool vector_targets_convertible_p (const_tree t1, const_tree t2);
  extern bool vector_types_convertible_p (const_tree t1, const_tree t2, bool 
emit_lax_note);
@@ -1494,7 +1495,7 @@ enum posargflags {
  };
extern tree positional_argument (const_tree, const_tree, tree, tree_code,
-                                int = 0, int = posargflags ());
+                                int = 0, int = posargflags (), int = 0);
extern enum flt_eval_method
  excess_precision_mode_join (enum flt_eval_method, enum flt_eval_method);
diff --git a/gcc/c-family/c-format.cc b/gcc/c-family/c-format.cc
index 98f28c0dcc6..87ae7bb73b0 100644
--- a/gcc/c-family/c-format.cc
+++ b/gcc/c-family/c-format.cc
@@ -70,8 +70,8 @@ static GTY(()) tree local_gimple_ptr_node;
  static GTY(()) tree local_cgraph_node_ptr_node;
  static GTY(()) tree locus;
-static bool decode_format_attr (const_tree, tree, tree, function_format_info *,
-                               bool);
+static bool decode_format_attr (const_tree, const_tree, tree, tree,
+                               function_format_info *, bool);
  static format_type decode_format_type (const char *, bool * = NULL);
static bool check_format_string (const_tree argument,
@@ -80,7 +80,7 @@ static bool check_format_string (const_tree argument,
                                 int expected_format_type);
  static tree get_constant (const_tree fntype, const_tree atname, tree expr,
                          int argno, unsigned HOST_WIDE_INT *value,
-                         int flags, bool validated_p);
+                         int flags, bool validated_p, int adjust_pos);
  static const char *convert_format_name_to_system_name (const char *attr_name);
static int first_target_format_type;
@@ -177,7 +177,7 @@ handle_format_arg_attribute (tree *node, tree atname,
    unsigned HOST_WIDE_INT format_num = 0;
if (tree val = get_constant (type, atname, *format_num_expr, 0, &format_num,
-                              0, false))
+                              0, false, 0))
      *format_num_expr = val;
    else
      {
@@ -305,18 +305,24 @@ check_format_string (const_tree fntype, unsigned 
HOST_WIDE_INT format_num,
     If valid, store the constant's integer value in *VALUE and return
     the value.
     If VALIDATED_P is true assert the validation is successful.
-   Returns the converted constant value on success, null otherwise.  */
+   Returns the converted constant value on success, null otherwise.
+   ADJUST_POS is used and non-zero in C++ when the function type has
+   invisible parameters generated by the compiler, such as the in-charge
+   or VTT parameters.  */
static tree
  get_constant (const_tree fntype, const_tree atname, tree expr, int argno,
-             unsigned HOST_WIDE_INT *value, int flags, bool validated_p)
+             unsigned HOST_WIDE_INT *value, int flags, bool validated_p,
+             int adjust_pos)
  {
    /* Require the referenced argument to have a string type.  For targets
       like Darwin, also accept pointers to struct CFString.  */
    if (tree val = positional_argument (fntype, atname, expr, STRING_CST,
-                                     argno, flags))
+                                     argno, flags, adjust_pos))
      {
-      *value = TREE_INT_CST_LOW (val);
+      /* Invisible parameters may have been added by the compiler, so adjust
+        the position accordingly.  */
+      *value = TREE_INT_CST_LOW (val) + adjust_pos;
        return val;
      }
@@ -332,8 +338,8 @@ get_constant (const_tree fntype, const_tree atname, tree expr, int argno,
     attributes are successfully decoded, false otherwise.  */
static bool
-decode_format_attr (const_tree fntype, tree atname, tree args,
-                   function_format_info *info, bool validated_p)
+decode_format_attr (const_tree fntype, const_tree fndecl, tree atname,
+                   tree args, function_format_info *info, bool validated_p)
  {
    tree format_type_id = TREE_VALUE (args);
    /* Note that TREE_VALUE (args) is changed in place below.  Ditto
@@ -372,15 +378,19 @@ decode_format_attr (const_tree fntype, tree atname, tree 
args,
        }
      }
+ int adjust_pos = maybe_adjust_arg_pos_for_attribute (fndecl);
+
    if (tree val = get_constant (fntype, atname, *format_num_expr,
-                              2, &info->format_num, 0, validated_p))
+                              2, &info->format_num, 0, validated_p,
+                              adjust_pos))
      *format_num_expr = val;
    else
      return false;
if (tree val = get_constant (fntype, atname, *first_arg_num_expr,
                               3, &info->first_arg_num,
-                              (POSARG_ZERO | POSARG_ELLIPSIS), validated_p))
+                              (POSARG_ZERO | POSARG_ELLIPSIS), validated_p,
+                              adjust_pos))
      *first_arg_num_expr = val;
    else
      return false;
@@ -1160,8 +1170,8 @@ decode_format_type (const char *s, bool *is_raw /* = NULL 
*/)
     attribute themselves.  */
void
-check_function_format (const_tree fntype, tree attrs, int nargs,
-                      tree *argarray, vec<location_t> *arglocs)
+check_function_format (const_tree fntype, const_tree fndecl, tree attrs,
+                      int nargs, tree *argarray, vec<location_t> *arglocs)
  {
    tree a;
@@ -1174,7 +1184,7 @@ check_function_format (const_tree fntype, tree attrs, int nargs,
        {
          /* Yup; check it.  */
          function_format_info info;
-         decode_format_attr (fntype, atname, TREE_VALUE (a), &info,
+         decode_format_attr (fntype, fndecl, atname, TREE_VALUE (a), &info,
                              /*validated=*/true);
          if (warn_format)
            {
@@ -5150,10 +5160,11 @@ convert_format_name_to_system_name (const char 
*attr_name)
  /* Handle a "format" attribute; arguments as in
     struct attribute_spec.handler.  */
  tree
-handle_format_attribute (tree *node, tree atname, tree args,
+handle_format_attribute (tree node[3], tree atname, tree args,
                         int flags, bool *no_add_attrs)
  {
    const_tree type = *node;
+  const_tree fndecl = node[2];
    function_format_info info;
#ifdef TARGET_FORMAT_TYPES
@@ -5179,7 +5190,8 @@ handle_format_attribute (tree *node, tree atname, tree 
args,
    if (TREE_CODE (TREE_VALUE (args)) == IDENTIFIER_NODE)
      TREE_VALUE (args) = canonicalize_attr_name (TREE_VALUE (args));
- if (!decode_format_attr (type, atname, args, &info, /* validated_p = */false))
+  if (!decode_format_attr (type, fndecl, atname, args, &info,
+                          /* validated_p = */false))
      {
        *no_add_attrs = true;
        return NULL_TREE;
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index 97850ada2c8..70e10a98e33 100644
--- a/gcc/c/c-objc-common.cc
+++ b/gcc/c/c-objc-common.cc
@@ -394,3 +394,12 @@ c_get_alias_set (tree t)
return c_common_get_alias_set (t);
  }
+
+/* In C there are no invisible parameters like in C++ (this, in-charge, VTT,
+   etc.).  */
+
+int
+maybe_adjust_arg_pos_for_attribute (const_tree)
+{
+  return 0;
+}
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 780a8d89165..ddfeb8ce428 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -6101,6 +6101,25 @@ maybe_warn_zero_as_null_pointer_constant (tree expr, 
location_t loc)
      }
    return false;
  }
+
+/* FNDECL is a function declaration whose type may have been altered by
+   adding extra parameters such as this, in-charge, or VTT.  When this
+   takes place, the positional arguments supplied by the user (as in the
+   'format' attribute arguments) may refer to the wrong argument.  This
+   function returns an integer indicating how many arguments should be
+   skipped.  */
+
+int
+maybe_adjust_arg_pos_for_attribute (const_tree fndecl)
+{
+  if (!fndecl)
+    return 0;
+  int n = num_artificial_parms_for (fndecl);
+  /* The manual states that it's the user's responsibility to account
+     for the implicit this parameter.  */
+  return n > 0 ? n - 1 : 0;
+}
+
  
  /* Release memory we no longer need after parsing.  */
  void
diff --git a/gcc/testsuite/g++.dg/ext/attr-format-arg1.C 
b/gcc/testsuite/g++.dg/ext/attr-format-arg1.C
new file mode 100644
index 00000000000..a7ad0f9ca33
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-format-arg1.C
@@ -0,0 +1,26 @@
+// PR c++/101833
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+struct B { };
+
+struct V : virtual B {
+  const char *fmt (int, const char *) __attribute__((format_arg(3)));
+};
+
+struct D : B {
+  const char *fmt (int, const char *) __attribute__((format_arg(3)));
+};
+
+extern void fmt (const char *, ...) __attribute__((format(printf, 1, 2)));
+
+void
+g ()
+{
+  V v;
+  fmt (v.fmt (1, "%d"), 1);
+  fmt (v.fmt (1, "%d"), 1lu); // { dg-warning "expects argument of type" }
+  D d;
+  fmt (d.fmt (1, "%d"), 1);
+  fmt (d.fmt (1, "%d"), 1lu); // { dg-warning "expects argument of type" }
+}
diff --git a/gcc/testsuite/g++.dg/ext/attr-format1.C 
b/gcc/testsuite/g++.dg/ext/attr-format1.C
new file mode 100644
index 00000000000..1b8464ed6ac
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-format1.C
@@ -0,0 +1,32 @@
+// PR c++/47634
+// { dg-do compile }
+
+class Base {
+public:
+  Base() { }
+};
+
+class VDerived : public virtual Base {
+public:
+  VDerived(int x, const char * f, ...) __attribute__((format(printf, 3, 4)));
+};
+
+class Derived : public Base {
+public:
+  Derived(int x, const char * f, ...) __attribute__((format(printf, 3, 4)));
+};
+
+VDerived::VDerived(int, const char *, ...)
+{
+}
+
+Derived::Derived(int, const char *, ...)
+{
+}
+
+int
+main(int, char **)
+{
+  throw VDerived(1, "%s %d", "foo", 1);
+  throw Derived(1, "%s %d", "bar", 1);
+}
diff --git a/gcc/testsuite/g++.dg/ext/attr-format2.C 
b/gcc/testsuite/g++.dg/ext/attr-format2.C
new file mode 100644
index 00000000000..7e6eec58047
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-format2.C
@@ -0,0 +1,38 @@
+// PR c++/101833
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+struct B { };
+
+struct V : virtual B {
+  V(int, const char *, ...) __attribute__((format(printf, 3, 4)));
+};
+
+struct D : B {
+  D(int, const char *, ...) __attribute__((format(printf, 3, 4)));
+};
+
+struct D2 : B {
+  template<typename T>
+  D2(T, const char *, ...) __attribute__((format(printf, 3, 4)));
+};
+
+struct V2 : virtual B {
+  template<typename T>
+  V2(T, const char *, ...) __attribute__((format(printf, 3, 4)));
+};
+
+struct X {
+  template<typename T>
+  X(T, ...) __attribute__((format(printf, 2, 3)));
+};
+
+V v(1, "%s %d", "foo", 1);
+D d(1, "%s %d", "foo", 1);
+D2 d2(1, "%s %d", "foo", 1);
+V2 v2(1, "%s %d", "foo", 1);
+
+// Test that it actually works.
+V e1(1, "%d", 1L); // { dg-warning "expects argument of type" }
+D e2(1, "%d", 1L); // { dg-warning "expects argument of type" }
+X e3("%d", 1L); // { dg-warning "expects argument of type" }
diff --git a/gcc/testsuite/g++.dg/ext/attr-format3.C 
b/gcc/testsuite/g++.dg/ext/attr-format3.C
new file mode 100644
index 00000000000..d6c9e40756f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-format3.C
@@ -0,0 +1,15 @@
+// PR c++/101833
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+class Base {};
+
+struct VDerived : virtual Base {
+  VDerived(int, int, const char *, ...) __attribute__((format(printf, 2, 3))); // { 
dg-error ".format. attribute argument 2 value .2. refers to parameter type 
.int." }
+  VDerived(int, const char *, ...) __attribute__((format(printf, 5, 6))); // { 
dg-warning ".format. attribute argument 2 value .5. exceeds" }
+} a(1, "%s %d", "foo", 1);
+
+struct Derived : Base {
+  Derived(int, int, const char *, ...) __attribute__((format(printf, 2, 3))); // { 
dg-error ".format. attribute argument 2 value .2. refers to parameter type 
.int." }
+  Derived(int, const char *, ...) __attribute__((format(printf, 5, 6))); // { dg-warning 
".format. attribute argument 2 value .5. exceeds" }
+} b(1, "%s %d", "foo", 1);;

base-commit: 95533fe4f014c10dd18de649927668aba6117daf

Reply via email to