On Fri, Mar 09, 2018 at 11:28:47AM -0500, Jason Merrill wrote:
> On Thu, Mar 8, 2018 at 4:26 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> > On Thu, Mar 08, 2018 at 04:20:40PM -0500, Jason Merrill wrote:
> >> On Thu, Mar 8, 2018 at 1:08 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> >> > The reporter complains that -Wformat incorrectly reports std::string* 
> >> > passed
> >> > to "%s" format rather than std::string, which is what the user did.
> >> >
> >> > This transformation of non-trivial copy init or dtor classes in ellipsis 
> >> > is
> >> > done by convert_arg_to_ellipsis; that function does many changes and all
> >> > but this one look desirable for the 
> >> > -Wnonnull/-Wformat/-Wsentinel/-Wrestrict
> >> > warnings.  We prepare a special argument vector in any case, so this 
> >> > patch
> >> > just arranges to undo what convert_arg_to_ellipsis did for the classes.
> >> > I think -Wnonnull shouldn't care, because when passing such a class by
> >> > value, it will be non-NULL (and -Wnonnull looks only for literal NULLs
> >> > anyway), -Wrestrict only cares about named arguments and -Wsentinel only
> >> > cares about NULL arguments passed to ellipsis.
> >>
> >> I notice that convert_arg_to_ellipsis generates a pointer-typed
> >> expression, whereas convert_for_arg_passing generates a reference.
> >> Does correcting that inconsistency help?
> >
> > No (though I think it is a good idea anyway).
> 
> Perhaps then check_format_types could look through REFERENCE_TYPE,
> since there are no actual expressions of reference type.

It can be done in the caller as well like below, or in c-format.c's
          if (warn_format)
            {
              /* FIXME: Rewrite all the internal functions in this file
                 to use the ARGARRAY directly instead of constructing this
                 temporary list.  */
              tree params = NULL_TREE;
              int i;
              for (i = nargs - 1; i >= 0; i--)
                params = tree_cons (NULL_TREE, argarray[i], params);
              check_format_info (&info, params, arglocs);
            }
I'm afraid the c-format.c has so many uses of the params list that it is
hard to tweak it anywhere else.

2018-03-09  Jason Merrill  <ja...@redhat.com>
            Jakub Jelinek  <ja...@redhat.com>

        PR c++/84076
        * call.c (convert_arg_to_ellipsis): Instead of cp_build_addr_expr
        build ADDR_EXPR with REFERENCE_TYPE.
        (build_over_call): For purposes of check_function_arguments, if
        argarray[j] is ADDR_EXPR with REFERENCE_TYPE created above, use
        its operand rather than the argument itself.

        * g++.dg/warn/Wformat-2.C: New test.

--- gcc/cp/call.c.jj    2018-03-09 09:01:32.017423737 +0100
+++ gcc/cp/call.c       2018-03-09 18:12:34.973494714 +0100
@@ -7209,7 +7209,7 @@ convert_arg_to_ellipsis (tree arg, tsubs
                     "passing objects of non-trivially-copyable "
                     "type %q#T through %<...%> is conditionally supported",
                     arg_type);
-         return cp_build_addr_expr (arg, complain);
+         return build1 (ADDR_EXPR, build_reference_type (arg_type), arg);
        }
       /* Build up a real lvalue-to-rvalue conversion in case the
         copy constructor is trivial but not callable.  */
@@ -8018,7 +8018,15 @@ build_over_call (struct z_candidate *can
       tree *fargs = (!nargs ? argarray
                            : (tree *) alloca (nargs * sizeof (tree)));
       for (j = 0; j < nargs; j++)
-       fargs[j] = maybe_constant_value (argarray[j]);
+       {
+         /* For -Wformat undo the implicit passing by hidden reference
+            done by convert_arg_to_ellipsis.  */
+         if (TREE_CODE (argarray[j]) == ADDR_EXPR
+             && TREE_CODE (TREE_TYPE (argarray[j])) == REFERENCE_TYPE)
+           fargs[j] = TREE_OPERAND (argarray[j], 0);
+         else
+           fargs[j] = maybe_constant_value (argarray[j]);
+       }
 
       warned_p = check_function_arguments (input_location, fn, TREE_TYPE (fn),
                                           nargs, fargs, NULL);
--- gcc/testsuite/g++.dg/warn/Wformat-2.C.jj    2018-03-09 17:59:57.098113181 
+0100
+++ gcc/testsuite/g++.dg/warn/Wformat-2.C       2018-03-09 17:59:57.098113181 
+0100
@@ -0,0 +1,17 @@
+// PR c++/84076
+// { dg-do compile }
+// { dg-options "-Wformat" }
+
+struct S { ~S (); };
+struct T { T (); T (const T &); };
+
+void
+foo ()
+{
+  S s;
+  T t;
+  __builtin_printf ("%s\n", s);        // { dg-warning "format '%s' expects 
argument of type 'char\\*', but argument 2 has type 'S'" }
+  __builtin_printf ("%s\n", t);        // { dg-warning "format '%s' expects 
argument of type 'char\\*', but argument 2 has type 'T'" }
+  __builtin_printf ("%s\n", &s);// { dg-warning "format '%s' expects argument 
of type 'char\\*', but argument 2 has type 'S\\*'" }
+  __builtin_printf ("%s\n", &t);// { dg-warning "format '%s' expects argument 
of type 'char\\*', but argument 2 has type 'T\\*'" }
+}


        Jakub

Reply via email to