[PATCH][gcc] Allow functions without C-style ellipsis to use format attribute

[PATCH][gcc] Allow functions without C-style ellipsis to use format attribute

[Tuan Le Quang]

Hi,

Currently, format attribute can be used to do type-checking for arguments
with respect to  a format string. However, only functions with a C-style
ellipsis can use it.
Supporting this attribute for non-variadic functions(functions without a
C-style ellipsis) gives nice convenience especially when writing code in
C++, we can use it for C++ variadic template functions like this

template<Args...args>
__attribute__((format(printf, 1, 2))) void myPrint (const char * fmt,
Args...args)

This patch will introduce these changes:
1. It is no longer an error simply to have a function with the format
attribute but no C-style variadic arguments
2. Functions are subjected to warnings/errors as before, except errors
mentioned in point 1 about not being variadic. For example, when a
non-variadic function has wrong arguments, e.g
__attribute__((format(printf, 1, 1))) or when being type-checked.

Note that behaviours of C-style variadic functions do not change, errors
and warnings are given as before.

This patch does it by:
1.   Relaxing several conditions for format attribute:
     -  Will only use POSARG_ELLIPSIS flag to call `get_constant` when
getting attribute arguments of a variadic function
     -  Relax the check for the last argument of the attribute (will not
require an ellipsis argument)
     -  (Before this patch) After passing the above check, current gcc will
call `get_constant` to get the function parameter that the third attribute
argument is pointing to. If POSARG_ELLIPSIS is set, `get_constant` will
look for `...`. If not, `get_constant` will look for a C-style string. Note
that POSARG_ELLIPSIS is set automatically for getting the third attribute
argument.
        (After this patch) POSARG_ELLIPSIS is set only when the function
has C-style '...'. Now, if POSARG_ELLIPSIS is not set, `get_constant` will
not check whether the third argument of format attribute points to a
C-style string.
2.   Modifying expected outcome of a testcase in objc testsuite, where we
expect a warning instead of an error
3.   Adding 2 test files

Successully bootstrapped and regression tested on x86_64-pc-linux-gnu.

Signed-off-by: Le Quang Tuan <lequangtuan6b@gmail.com>

gcc/c-family/ChangeLog:

* c-attribs.c (positional_argument): allow third argument of format
attribute to point to parameters of any type if the function is not C-style
variadic
* c-format.c (decode_format_attr): read third argument with POSARG_ELLIPSIS
only if the function has has a variable argument
(handle_format_attribute): relax explicit checks for non-variadic functions

gcc/testsuite/ChangeLog:

* gcc.dg/format/attr-3.c: modify comment
* objc.dg/attributes/method-format-1.m: errors do not hold anymore, a
warning is given instead
* g++.dg/warn/format9.C: New test with usage of variadic templates.
* gcc.dg/format/attr-9.c: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 6bf492afcc0..7a17ce671de 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -714,6 +714,11 @@ positional_argument (const_tree fntype, const_tree
atname, tree pos,
   return NULL_TREE;
  }

+  /* For format attribute with argno >= 3, we don't expect any type
+   */
+  if (argno >= 3 && strcmp (IDENTIFIER_POINTER(atname), "format") == 0 &&
!(flags & POSARG_ELLIPSIS ) )
+    return pos;
+
       /* Where the expected code is STRING_CST accept any pointer
  expected by attribute format (this includes possibly qualified
  char pointers and, for targets like Darwin, also pointers to
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index bda3b18fcd0..453565ad7b8 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -380,9 +380,15 @@ decode_format_attr (const_tree fntype, tree atname,
tree args,
   else
     return false;

+  bool has_variable_arg = !type_argument_type(fntype,
type_num_arguments(fntype) + 1);
+  int extra_flag = 0;
+  if (has_variable_arg) {
+    extra_flag = POSARG_ELLIPSIS;
+  }
+
   if (tree val = get_constant (fntype, atname, *first_arg_num_expr,
        3, &info->first_arg_num,
-       (POSARG_ZERO | POSARG_ELLIPSIS), validated_p))
+       (POSARG_ZERO | extra_flag), validated_p))
     *first_arg_num_expr = val;
   else
     return false;
@@ -5193,11 +5199,11 @@ handle_format_attribute (tree *node, tree atname,
tree args,
   tree arg_type;

   /* Verify that first_arg_num points to the last arg,
-     the ...  */
+     if the last arg is  ... */
   FOREACH_FUNCTION_ARGS (type, arg_type, iter)
     arg_num++;

-  if (arg_num != info.first_arg_num)
+  if (arg_num != info.first_arg_num && !type_argument_type(type, arg_num))
     {
       if (!(flags & (int) ATTR_FLAG_BUILT_IN))
  error ("argument to be formatted is not %<...%>");
diff --git a/gcc/testsuite/g++.dg/warn/format9.C
b/gcc/testsuite/g++.dg/warn/format9.C
new file mode 100644
index 00000000000..39b615859fc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/format9.C
@@ -0,0 +1,16 @@
+// Test format attribute used with variadic templates
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wformat" }
+
+template<typename...Args>
+__attribute__((format(printf, 1, 2))) void fa (const char * fmt,
Args...args)
+{
+    return;
+}
+
+int main()
+{
+    fa ("%d%d", 5, 7);
+    fa ("%d%d%d", 5, 6, 7);
+    fa ("%s%d", 3, 4); // { dg-warning "format" "printf warning" }
+}
diff --git a/gcc/testsuite/gcc.dg/format/attr-3.c
b/gcc/testsuite/gcc.dg/format/attr-3.c
index 1b275c5aba8..64990c43472 100644
--- a/gcc/testsuite/gcc.dg/format/attr-3.c
+++ b/gcc/testsuite/gcc.dg/format/attr-3.c
@@ -55,7 +55,7 @@ extern void fg2 () __attribute__((format(gnu_attr_printf,
1, 1))); /* { dg-error
 extern void fg3 () __attribute__((format(gnu_attr_printf, 2, 1))); /* {
dg-error "follows" "bad number order" } */

 /* The format string argument must be a string type, and the arguments to
-   be formatted must be the "...".  */
+   be formatted must be the "..." if the function is variadic  */
 extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 2)));
/* { dg-error ".format. attribute argument 2 value .1. refers to parameter
type .int." "format int string" } */
 extern void fh1 (signed char *, ...)
__attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format.
attribute argument 2 value .1. refers to parameter type .signed char \\\*."
"signed char string" } */
 extern void fh2 (unsigned char *, ...)
__attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format.
attribute argument 2 value .1. refers to parameter type .unsigned char
\\\*." "unsigned char string" } */
diff --git a/gcc/testsuite/gcc.dg/format/attr-9.c
b/gcc/testsuite/gcc.dg/format/attr-9.c
new file mode 100644
index 00000000000..3f6788f291c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/format/attr-9.c
@@ -0,0 +1,21 @@
+/* Test for format attributes: test usage of the attribute for
non-variadic functions */
+/* Origin: Tuan Le <lequangtuan6b@gmail.com> */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99 -Wformat" } */
+
+#define DONT_GNU_PROTOTYPE
+#include "format.h"
+
+/* Usage of the attributes */
+extern void fa (const char *, int, const char *)
__attribute__((format(gnu_attr_printf, 1, 2)));
+extern void fb (const char *, int, int, int)
__attribute__((format(gnu_attr_printf, 1, 3)));
+
+/* Some bad uses */
+extern void fc (const char *, int) __attribute__((format(gnu_attr_printf,
1, 1))); /* { dg-error "format string argument follows the arguments to be
formatted" } */
+
+void
+foo (const char *s, int *p)
+{
+    fa ("%d%s", 5, "hello");
+    fa ("%d %d", 5, "hello"); /* { dg-warning "format" "attribute format
__printf__" } */
+}
diff --git a/gcc/testsuite/objc.dg/attributes/method-format-1.m
b/gcc/testsuite/objc.dg/attributes/method-format-1.m
index d3bb997b2aa..443f9c73120 100644
--- a/gcc/testsuite/objc.dg/attributes/method-format-1.m
+++ b/gcc/testsuite/objc.dg/attributes/method-format-1.m
@@ -20,8 +20,8 @@
 - (void) log2: (int)level  message: (const char *) my_format, ...
 __attribute__ ((format (printf, 2)));    /* { dg-error "wrong" } */
 + (void) debug2: (const char *) my_format, ...  __attribute__ ((format
(printf))); /* { dg-error "wrong" } */
 - (void) debug2: (const char *) my_format, ...  __attribute__ ((format
(printf))); /* { dg-error "wrong" } */
-+ (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-error "does not refer to a variable argument list" } */
-- (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-error "does not refer to a variable argument list" } */
++ (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-warning "exceeds the number of function parameters" } */
+- (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-warning "exceeds the number of function parameters" } */
 @end

 void test (LogObject *object)
-- 
2.17.1

Re: [PATCH][gcc] Allow functions without C-style ellipsis to use format attribute

[Martin Sebor]

On 6/27/21 10:24 PM, Tuan Le Quang via Gcc-patches wrote:
> Hi,
> 
> Currently, format attribute can be used to do type-checking for arguments
> with respect to  a format string. However, only functions with a C-style
> ellipsis can use it.
> Supporting this attribute for non-variadic functions(functions without a
> C-style ellipsis) gives nice convenience especially when writing code in
> C++, we can use it for C++ variadic template functions like this
> 
> template<Args...args>
> __attribute__((format(printf, 1, 2))) void myPrint (const char * fmt,
> Args...args)

The main benefit of variadic functions templates over C vararg
functions is that they make use of the type system for type safety.
I'm not sure I see applying attribute format to them as a very
compelling use case.  (I'd expect the format string in a variadic
function template to use generic conversion specifiers, say %@ or
some such, and only let the caller specify things like flags, width
and precision but not type conversion specifiers).  Is there one
where relying on the type system isn't good enough?

> This patch will introduce these changes:
> 1. It is no longer an error simply to have a function with the format
> attribute but no C-style variadic arguments

I'm a little on the fence about this.  On the one hand it seems
unexpected to apply format checking to ordinary (non-variadic)
functions.  On the other, I can't think of anything wrong with it
and it even seems like it could be useful to specify a format for
a fixed number of arguments of fixed types.  Do you have an actual
use case for it or did it just fall out of the varaidic template
implementation?

> 2. Functions are subjected to warnings/errors as before, except errors
> mentioned in point 1 about not being variadic. For example, when a
> non-variadic function has wrong arguments, e.g
> __attribute__((format(printf, 1, 1))) or when being type-checked.
> 
> Note that behaviours of C-style variadic functions do not change, errors
> and warnings are given as before.
> 
> This patch does it by:
> 1.   Relaxing several conditions for format attribute:
>       -  Will only use POSARG_ELLIPSIS flag to call `get_constant` when
> getting attribute arguments of a variadic function
>       -  Relax the check for the last argument of the attribute (will not
> require an ellipsis argument)
>       -  (Before this patch) After passing the above check, current gcc will
> call `get_constant` to get the function parameter that the third attribute
> argument is pointing to. If POSARG_ELLIPSIS is set, `get_constant` will
> look for `...`. If not, `get_constant` will look for a C-style string. Note
> that POSARG_ELLIPSIS is set automatically for getting the third attribute
> argument.
>          (After this patch) POSARG_ELLIPSIS is set only when the function
> has C-style '...'. Now, if POSARG_ELLIPSIS is not set, `get_constant` will
> not check whether the third argument of format attribute points to a
> C-style string.
> 2.   Modifying expected outcome of a testcase in objc testsuite, where we
> expect a warning instead of an error
> 3.   Adding 2 test files
> 
> Successully bootstrapped and regression tested on x86_64-pc-linux-gnu.
> 
> Signed-off-by: Le Quang Tuan <lequangtuan6b@gmail.com>
> 
> gcc/c-family/ChangeLog:
> 
> * c-attribs.c (positional_argument): allow third argument of format
> attribute to point to parameters of any type if the function is not C-style
> variadic
> * c-format.c (decode_format_attr): read third argument with POSARG_ELLIPSIS
> only if the function has has a variable argument
> (handle_format_attribute): relax explicit checks for non-variadic functions
> 
> gcc/testsuite/ChangeLog:
> 
> * gcc.dg/format/attr-3.c: modify comment
> * objc.dg/attributes/method-format-1.m: errors do not hold anymore, a
> warning is given instead
> * g++.dg/warn/format9.C: New test with usage of variadic templates.
> * gcc.dg/format/attr-9.c: New test.
> 
> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> index 6bf492afcc0..7a17ce671de 100644
> --- a/gcc/c-family/c-attribs.c
> +++ b/gcc/c-family/c-attribs.c
> @@ -714,6 +714,11 @@ positional_argument (const_tree fntype, const_tree
> atname, tree pos,
>     return NULL_TREE;
>    }
> 
> +  /* For format attribute with argno >= 3, we don't expect any type
> +   */
> +  if (argno >= 3 && strcmp (IDENTIFIER_POINTER(atname), "format") == 0 &&
> !(flags & POSARG_ELLIPSIS ) )
> +    return pos;

Hardcoding knowledge of individual attributes in this function doesn't
seem very robust.  Avoiding that is the purpose of the flags argument.
I'd suggest adding a bit to the posargflags enum.

Also, at this point, (flags & POSARG_ELLIPSIS) should be zero as
a result of the test above (not shown) so repeating the test shouldn't
be necessary.

Martin

Re: [PATCH][gcc] Allow functions without C-style ellipsis to use format attribute

[Tuan Le Quang]

Hi Martin,

Thank you for your quick response.

>  The main benefit of variadic functions templates over C vararg
>  functions is that they make use of the type system for type safety.
>  I'm not sure I see applying attribute format to them as a very
>  compelling use case.  (I'd expect the format string in a variadic
>  function template to use generic conversion specifiers, say %@ or
>  some such, and only let the caller specify things like flags, width
>  and precision but not type conversion specifiers).  Is there one
>  where relying on the type system isn't good enough?

One case we have is wrapping a C API in a C++ one. Hence, the underlying
API is C and we must specify types for format arguments. It means that
generic conversion is not possible.

> Do you have an actual
> use case for it or did it just fall out of the varaidic template
> implementation?

Yes, it falls out of the variadic template. It is the only way that helps
variadic templates use format attribute. And I also feel the same, it might
be useful sometimes.

Also, your suggestion on the code is helpful! I have drafted a new patch
here. I have bootstrapped and regression tested it on x86_64-pc-linux-gnu

Regards,
Tuan

gcc/c-family/ChangeLog:

* c-attribs.c (positional_argument): allow third argument of format
attribute to point to parameters of any type if the function is not C-style
variadic
* c-common.h (enum posargflags): add flag POSARG_ANY for the third argument
of format attribute
* c-format.c (decode_format_attr): read third argument with POSARG_ELLIPSIS
only if the function has has a variable argument
(handle_format_attribute): relax explicit checks for non-variadic functions

gcc/testsuite/ChangeLog:

* gcc.dg/format/attr-3.c: modify comment
* objc.dg/attributes/method-format-1.m: errors do not hold anymore, a
warning is given instead
* g++.dg/warn/format9.C: New test.
* gcc.dg/format/attr-9.c: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 6bf492afcc0..a46d882feba 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -714,6 +714,9 @@ positional_argument (const_tree fntype, const_tree
atname, tree pos,
   return NULL_TREE;
  }

+      if (flags & POSARG_ANY)
+    return pos;
+
       /* Where the expected code is STRING_CST accept any pointer
  expected by attribute format (this includes possibly qualified
  char pointers and, for targets like Darwin, also pointers to
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index be4b29a017b..391cfa685d4 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1462,7 +1462,10 @@ enum posargflags {
   POSARG_ZERO = 1,
   /* Consider positional attribute argument value valid if it refers
      to the ellipsis (i.e., beyond the last typed argument).  */
-  POSARG_ELLIPSIS = 2
+  POSARG_ELLIPSIS = 2,
+  /* Consider positional attribute argument value valid if it refers
+     to an argument of any type */
+  POSARG_ANY = 4
 };

 extern tree positional_argument (const_tree, const_tree, tree, tree_code,
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index bda3b18fcd0..0cef3152828 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -380,9 +380,16 @@ decode_format_attr (const_tree fntype, tree atname,
tree args,
   else
     return false;

+  bool has_variable_arg = !type_argument_type(fntype,
type_num_arguments(fntype) + 1);
+  int extra_flag = 0;
+  if (has_variable_arg)
+    extra_flag = POSARG_ELLIPSIS;
+  else
+    extra_flag = POSARG_ANY;
+
   if (tree val = get_constant (fntype, atname, *first_arg_num_expr,
        3, &info->first_arg_num,
-       (POSARG_ZERO | POSARG_ELLIPSIS), validated_p))
+       (POSARG_ZERO | extra_flag), validated_p))
     *first_arg_num_expr = val;
   else
     return false;
@@ -5193,11 +5200,11 @@ handle_format_attribute (tree *node, tree atname,
tree args,
   tree arg_type;

   /* Verify that first_arg_num points to the last arg,
-     the ...  */
+     if the last arg is  ... */
   FOREACH_FUNCTION_ARGS (type, arg_type, iter)
     arg_num++;

-  if (arg_num != info.first_arg_num)
+  if (arg_num != info.first_arg_num && !type_argument_type(type, arg_num))
     {
       if (!(flags & (int) ATTR_FLAG_BUILT_IN))
  error ("argument to be formatted is not %<...%>");
diff --git a/gcc/testsuite/g++.dg/warn/format9.C
b/gcc/testsuite/g++.dg/warn/format9.C
new file mode 100644
index 00000000000..39b615859fc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/format9.C
@@ -0,0 +1,16 @@
+// Test format attribute used with variadic templates
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wformat" }
+
+template<typename...Args>
+__attribute__((format(printf, 1, 2))) void fa (const char * fmt,
Args...args)
+{
+    return;
+}
+
+int main()
+{
+    fa ("%d%d", 5, 7);
+    fa ("%d%d%d", 5, 6, 7);
+    fa ("%s%d", 3, 4); // { dg-warning "format" "printf warning" }
+}
diff --git a/gcc/testsuite/gcc.dg/format/attr-3.c
b/gcc/testsuite/gcc.dg/format/attr-3.c
index 1b275c5aba8..64990c43472 100644
--- a/gcc/testsuite/gcc.dg/format/attr-3.c
+++ b/gcc/testsuite/gcc.dg/format/attr-3.c
@@ -55,7 +55,7 @@ extern void fg2 () __attribute__((format(gnu_attr_printf,
1, 1))); /* { dg-error
 extern void fg3 () __attribute__((format(gnu_attr_printf, 2, 1))); /* {
dg-error "follows" "bad number order" } */

 /* The format string argument must be a string type, and the arguments to
-   be formatted must be the "...".  */
+   be formatted must be the "..." if the function is variadic  */
 extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 2)));
/* { dg-error ".format. attribute argument 2 value .1. refers to parameter
type .int." "format int string" } */
 extern void fh1 (signed char *, ...)
__attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format.
attribute argument 2 value .1. refers to parameter type .signed char \\\*."
"signed char string" } */
 extern void fh2 (unsigned char *, ...)
__attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format.
attribute argument 2 value .1. refers to parameter type .unsigned char
\\\*." "unsigned char string" } */
diff --git a/gcc/testsuite/gcc.dg/format/attr-9.c
b/gcc/testsuite/gcc.dg/format/attr-9.c
new file mode 100644
index 00000000000..3f6788f291c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/format/attr-9.c
@@ -0,0 +1,21 @@
+/* Test for format attributes: test usage of the attribute for
non-variadic functions */
+/* Origin: Tuan Le <lequangtuan6b@gmail.com> */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99 -Wformat" } */
+
+#define DONT_GNU_PROTOTYPE
+#include "format.h"
+
+/* Usage of the attributes */
+extern void fa (const char *, int, const char *)
__attribute__((format(gnu_attr_printf, 1, 2)));
+extern void fb (const char *, int, int, int)
__attribute__((format(gnu_attr_printf, 1, 3)));
+
+/* Some bad uses */
+extern void fc (const char *, int) __attribute__((format(gnu_attr_printf,
1, 1))); /* { dg-error "format string argument follows the arguments to be
formatted" } */
+
+void
+foo (const char *s, int *p)
+{
+    fa ("%d%s", 5, "hello");
+    fa ("%d %d", 5, "hello"); /* { dg-warning "format" "attribute format
__printf__" } */
+}
diff --git a/gcc/testsuite/objc.dg/attributes/method-format-1.m
b/gcc/testsuite/objc.dg/attributes/method-format-1.m
index d3bb997b2aa..443f9c73120 100644
--- a/gcc/testsuite/objc.dg/attributes/method-format-1.m
+++ b/gcc/testsuite/objc.dg/attributes/method-format-1.m
@@ -20,8 +20,8 @@
 - (void) log2: (int)level  message: (const char *) my_format, ...
 __attribute__ ((format (printf, 2)));    /* { dg-error "wrong" } */
 + (void) debug2: (const char *) my_format, ...  __attribute__ ((format
(printf))); /* { dg-error "wrong" } */
 - (void) debug2: (const char *) my_format, ...  __attribute__ ((format
(printf))); /* { dg-error "wrong" } */
-+ (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-error "does not refer to a variable argument list" } */
-- (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-error "does not refer to a variable argument list" } */
++ (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-warning "exceeds the number of function parameters" } */
+- (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-warning "exceeds the number of function parameters" } */
 @end

 void test (LogObject *object)
-- 
2.17.1



Vào Th 4, 30 thg 6, 2021 vào lúc 03:56 Martin Sebor <msebor@gmail.com> đã
viết:

> On 6/27/21 10:24 PM, Tuan Le Quang via Gcc-patches wrote:
> > Hi,
> >
> > Currently, format attribute can be used to do type-checking for arguments
> > with respect to  a format string. However, only functions with a C-style
> > ellipsis can use it.
> > Supporting this attribute for non-variadic functions(functions without a
> > C-style ellipsis) gives nice convenience especially when writing code in
> > C++, we can use it for C++ variadic template functions like this
> >
> > template<Args...args>
> > __attribute__((format(printf, 1, 2))) void myPrint (const char * fmt,
> > Args...args)
>
> The main benefit of variadic functions templates over C vararg
> functions is that they make use of the type system for type safety.
> I'm not sure I see applying attribute format to them as a very
> compelling use case.  (I'd expect the format string in a variadic
> function template to use generic conversion specifiers, say %@ or
> some such, and only let the caller specify things like flags, width
> and precision but not type conversion specifiers).  Is there one
> where relying on the type system isn't good enough?
>
> > This patch will introduce these changes:
> > 1. It is no longer an error simply to have a function with the format
> > attribute but no C-style variadic arguments
>
> I'm a little on the fence about this.  On the one hand it seems
> unexpected to apply format checking to ordinary (non-variadic)
> functions.  On the other, I can't think of anything wrong with it
> and it even seems like it could be useful to specify a format for
> a fixed number of arguments of fixed types.  Do you have an actual
> use case for it or did it just fall out of the varaidic template
> implementation?
>
> > 2. Functions are subjected to warnings/errors as before, except errors
> > mentioned in point 1 about not being variadic. For example, when a
> > non-variadic function has wrong arguments, e.g
> > __attribute__((format(printf, 1, 1))) or when being type-checked.
> >
> > Note that behaviours of C-style variadic functions do not change, errors
> > and warnings are given as before.
> >
> > This patch does it by:
> > 1.   Relaxing several conditions for format attribute:
> >       -  Will only use POSARG_ELLIPSIS flag to call `get_constant` when
> > getting attribute arguments of a variadic function
> >       -  Relax the check for the last argument of the attribute (will not
> > require an ellipsis argument)
> >       -  (Before this patch) After passing the above check, current gcc
> will
> > call `get_constant` to get the function parameter that the third
> attribute
> > argument is pointing to. If POSARG_ELLIPSIS is set, `get_constant` will
> > look for `...`. If not, `get_constant` will look for a C-style string.
> Note
> > that POSARG_ELLIPSIS is set automatically for getting the third attribute
> > argument.
> >          (After this patch) POSARG_ELLIPSIS is set only when the function
> > has C-style '...'. Now, if POSARG_ELLIPSIS is not set, `get_constant`
> will
> > not check whether the third argument of format attribute points to a
> > C-style string.
> > 2.   Modifying expected outcome of a testcase in objc testsuite, where we
> > expect a warning instead of an error
> > 3.   Adding 2 test files
> >
> > Successully bootstrapped and regression tested on x86_64-pc-linux-gnu.
> >
> > Signed-off-by: Le Quang Tuan <lequangtuan6b@gmail.com>
> >
> > gcc/c-family/ChangeLog:
> >
> > * c-attribs.c (positional_argument): allow third argument of format
> > attribute to point to parameters of any type if the function is not
> C-style
> > variadic
> > * c-format.c (decode_format_attr): read third argument with
> POSARG_ELLIPSIS
> > only if the function has has a variable argument
> > (handle_format_attribute): relax explicit checks for non-variadic
> functions
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/format/attr-3.c: modify comment
> > * objc.dg/attributes/method-format-1.m: errors do not hold anymore, a
> > warning is given instead
> > * g++.dg/warn/format9.C: New test with usage of variadic templates.
> > * gcc.dg/format/attr-9.c: New test.
> >
> > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> > index 6bf492afcc0..7a17ce671de 100644
> > --- a/gcc/c-family/c-attribs.c
> > +++ b/gcc/c-family/c-attribs.c
> > @@ -714,6 +714,11 @@ positional_argument (const_tree fntype, const_tree
> > atname, tree pos,
> >     return NULL_TREE;
> >    }
> >
> > +  /* For format attribute with argno >= 3, we don't expect any type
> > +   */
> > +  if (argno >= 3 && strcmp (IDENTIFIER_POINTER(atname), "format") == 0
> &&
> > !(flags & POSARG_ELLIPSIS ) )
> > +    return pos;
>
> Hardcoding knowledge of individual attributes in this function doesn't
> seem very robust.  Avoiding that is the purpose of the flags argument.
> I'd suggest adding a bit to the posargflags enum.
>
> Also, at this point, (flags & POSARG_ELLIPSIS) should be zero as
> a result of the test above (not shown) so repeating the test shouldn't
> be necessary.
>
> Martin
>

Re: [PATCH][gcc] Allow functions without C-style ellipsis to use format attribute

[Martin Sebor]

On 7/5/21 12:38 AM, Tuan Le Quang wrote:
> Hi Martin,
> 
> Thank you for your quick response.
> 
>  >  The main benefit of variadic functions templates over C vararg
>  >  functions is that they make use of the type system for type safety.
>  >  I'm not sure I see applying attribute format to them as a very
>  >  compelling use case.  (I'd expect the format string in a variadic
>  >  function template to use generic conversion specifiers, say %@ or
>  >  some such, and only let the caller specify things like flags, width
>  >  and precision but not type conversion specifiers).  Is there one
>  >  where relying on the type system isn't good enough?
> 
> One case we have is wrapping a C API in a C++ one. Hence, the underlying 
> API is C and we must specify types for format arguments. It means that 
> generic conversion is not possible.
> 
>  > Do you have an actual
>  > use case for it or did it just fall out of the varaidic template
>  > implementation?
> 
> Yes, it falls out of the variadic template. It is the only way that 
> helps variadic templates use format attribute. And I also feel the same, 
> it might be useful sometimes.
> 
> Also, your suggestion on the code is helpful! I have drafted a new patch 
> here. I have bootstrapped and regression tested it on x86_64-pc-linux-gnu

You've answered my questions about the design (thank you) and I don't
have any objections to the idea, but I'm not in a position to approve
the patch.  I would suggest to get Jason's input on extending
attribute format to variadic function templates, and Joseph's on
extending it to ordinary (non-variadic) functions.  I've CC'd both.

Thanks
Martin

> 
> Regards,
> Tuan
> 
> gcc/c-family/ChangeLog:
> 
> * c-attribs.c (positional_argument): allow third argument of format 
> attribute to point to parameters of any type if the function is not 
> C-style variadic
> * c-common.h (enum posargflags): add flag POSARG_ANY for the third 
> argument of format attribute
> * c-format.c (decode_format_attr): read third argument with 
> POSARG_ELLIPSIS only if the function has has a variable argument
> (handle_format_attribute): relax explicit checks for non-variadic functions
> 
> gcc/testsuite/ChangeLog:
> 
> * gcc.dg/format/attr-3.c: modify comment
> * objc.dg/attributes/method-format-1.m: errors do not hold anymore, a 
> warning is given instead
> * g++.dg/warn/format9.C: New test.
> * gcc.dg/format/attr-9.c: New test.
> 
> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> index 6bf492afcc0..a46d882feba 100644
> --- a/gcc/c-family/c-attribs.c
> +++ b/gcc/c-family/c-attribs.c
> @@ -714,6 +714,9 @@ positional_argument (const_tree fntype, const_tree 
> atname, tree pos,
>     return NULL_TREE;
>    }
> 
> +      if (flags & POSARG_ANY)
> +    return pos;
> +
>         /* Where the expected code is STRING_CST accept any pointer
>    expected by attribute format (this includes possibly qualified
>    char pointers and, for targets like Darwin, also pointers to
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index be4b29a017b..391cfa685d4 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1462,7 +1462,10 @@ enum posargflags {
>     POSARG_ZERO = 1,
>     /* Consider positional attribute argument value valid if it refers
>        to the ellipsis (i.e., beyond the last typed argument).  */
> -  POSARG_ELLIPSIS = 2
> +  POSARG_ELLIPSIS = 2,
> +  /* Consider positional attribute argument value valid if it refers
> +     to an argument of any type */
> +  POSARG_ANY = 4
>   };
> 
>   extern tree positional_argument (const_tree, const_tree, tree, tree_code,
> diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> index bda3b18fcd0..0cef3152828 100644
> --- a/gcc/c-family/c-format.c
> +++ b/gcc/c-family/c-format.c
> @@ -380,9 +380,16 @@ decode_format_attr (const_tree fntype, tree atname, 
> tree args,
>     else
>       return false;
> 
> +  bool has_variable_arg = !type_argument_type(fntype, 
> type_num_arguments(fntype) + 1);
> +  int extra_flag = 0;
> +  if (has_variable_arg)
> +    extra_flag = POSARG_ELLIPSIS;
> +  else
> +    extra_flag = POSARG_ANY;
> +
>     if (tree val = get_constant (fntype, atname, *first_arg_num_expr,
>          3, &info->first_arg_num,
> -       (POSARG_ZERO | POSARG_ELLIPSIS), validated_p))
> +       (POSARG_ZERO | extra_flag), validated_p))
>       *first_arg_num_expr = val;
>     else
>       return false;
> @@ -5193,11 +5200,11 @@ handle_format_attribute (tree *node, tree 
> atname, tree args,
>     tree arg_type;
> 
>     /* Verify that first_arg_num points to the last arg,
> -     the ...  */
> +     if the last arg is  ... */
>     FOREACH_FUNCTION_ARGS (type, arg_type, iter)
>       arg_num++;
> 
> -  if (arg_num != info.first_arg_num)
> +  if (arg_num != info.first_arg_num && !type_argument_type(type, arg_num))
>       {
>         if (!(flags & (int) ATTR_FLAG_BUILT_IN))
>    error ("argument to be formatted is not %<...%>");
> diff --git a/gcc/testsuite/g++.dg/warn/format9.C 
> b/gcc/testsuite/g++.dg/warn/format9.C
> new file mode 100644
> index 00000000000..39b615859fc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/format9.C
> @@ -0,0 +1,16 @@
> +// Test format attribute used with variadic templates
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wformat" }
> +
> +template<typename...Args>
> +__attribute__((format(printf, 1, 2))) void fa (const char * fmt, 
> Args...args)
> +{
> +    return;
> +}
> +
> +int main()
> +{
> +    fa ("%d%d", 5, 7);
> +    fa ("%d%d%d", 5, 6, 7);
> +    fa ("%s%d", 3, 4); // { dg-warning "format" "printf warning" }
> +}
> diff --git a/gcc/testsuite/gcc.dg/format/attr-3.c 
> b/gcc/testsuite/gcc.dg/format/attr-3.c
> index 1b275c5aba8..64990c43472 100644
> --- a/gcc/testsuite/gcc.dg/format/attr-3.c
> +++ b/gcc/testsuite/gcc.dg/format/attr-3.c
> @@ -55,7 +55,7 @@ extern void fg2 () 
> __attribute__((format(gnu_attr_printf, 1, 1))); /* { dg-error
>   extern void fg3 () __attribute__((format(gnu_attr_printf, 2, 1))); /* 
> { dg-error "follows" "bad number order" } */
> 
>   /* The format string argument must be a string type, and the arguments to
> -   be formatted must be the "...".  */
> +   be formatted must be the "..." if the function is variadic  */
>   extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 
> 2))); /* { dg-error ".format. attribute argument 2 value .1. refers to 
> parameter type .int." "format int string" } */
>   extern void fh1 (signed char *, ...) 
> __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. 
> attribute argument 2 value .1. refers to parameter type .signed char 
> \\\*." "signed char string" } */
>   extern void fh2 (unsigned char *, ...) 
> __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. 
> attribute argument 2 value .1. refers to parameter type .unsigned char 
> \\\*." "unsigned char string" } */
> diff --git a/gcc/testsuite/gcc.dg/format/attr-9.c 
> b/gcc/testsuite/gcc.dg/format/attr-9.c
> new file mode 100644
> index 00000000000..3f6788f291c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/format/attr-9.c
> @@ -0,0 +1,21 @@
> +/* Test for format attributes: test usage of the attribute for 
> non-variadic functions */
> +/* Origin: Tuan Le <lequangtuan6b@gmail.com 
> <mailto:lequangtuan6b@gmail.com>> */
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu99 -Wformat" } */
> +
> +#define DONT_GNU_PROTOTYPE
> +#include "format.h"
> +
> +/* Usage of the attributes */
> +extern void fa (const char *, int, const char *) 
> __attribute__((format(gnu_attr_printf, 1, 2)));
> +extern void fb (const char *, int, int, int) 
> __attribute__((format(gnu_attr_printf, 1, 3)));
> +
> +/* Some bad uses */
> +extern void fc (const char *, int) 
> __attribute__((format(gnu_attr_printf, 1, 1))); /* { dg-error "format 
> string argument follows the arguments to be formatted" } */
> +
> +void
> +foo (const char *s, int *p)
> +{
> +    fa ("%d%s", 5, "hello");
> +    fa ("%d %d", 5, "hello"); /* { dg-warning "format" "attribute 
> format __printf__" } */
> +}
> diff --git a/gcc/testsuite/objc.dg/attributes/method-format-1.m 
> b/gcc/testsuite/objc.dg/attributes/method-format-1.m
> index d3bb997b2aa..443f9c73120 100644
> --- a/gcc/testsuite/objc.dg/attributes/method-format-1.m
> +++ b/gcc/testsuite/objc.dg/attributes/method-format-1.m
> @@ -20,8 +20,8 @@
>   - (void) log2: (int)level  message: (const char *) my_format, ... 
>   __attribute__ ((format (printf, 2)));    /* { dg-error "wrong" } */
>   + (void) debug2: (const char *) my_format, ...  __attribute__ ((format 
> (printf))); /* { dg-error "wrong" } */
>   - (void) debug2: (const char *) my_format, ...  __attribute__ ((format 
> (printf))); /* { dg-error "wrong" } */
> -+ (void) alert: (const char *) my_format __attribute__ ((format 
> (printf, 1, 2))); /* { dg-error "does not refer to a variable argument 
> list" } */
> -- (void) alert: (const char *) my_format __attribute__ ((format 
> (printf, 1, 2))); /* { dg-error "does not refer to a variable argument 
> list" } */
> ++ (void) alert: (const char *) my_format __attribute__ ((format 
> (printf, 1, 2))); /* { dg-warning "exceeds the number of function 
> parameters" } */
> +- (void) alert: (const char *) my_format __attribute__ ((format 
> (printf, 1, 2))); /* { dg-warning "exceeds the number of function 
> parameters" } */
>   @end
> 
>   void test (LogObject *object)
> -- 
> 2.17.1
> 
> 
> 
> Vào Th 4, 30 thg 6, 2021 vào lúc 03:56 Martin Sebor <msebor@gmail.com 
> <mailto:msebor@gmail.com>> đã viết:
> 
>     On 6/27/21 10:24 PM, Tuan Le Quang via Gcc-patches wrote:
>      > Hi,
>      >
>      > Currently, format attribute can be used to do type-checking for
>     arguments
>      > with respect to  a format string. However, only functions with a
>     C-style
>      > ellipsis can use it.
>      > Supporting this attribute for non-variadic functions(functions
>     without a
>      > C-style ellipsis) gives nice convenience especially when writing
>     code in
>      > C++, we can use it for C++ variadic template functions like this
>      >
>      > template<Args...args>
>      > __attribute__((format(printf, 1, 2))) void myPrint (const char * fmt,
>      > Args...args)
> 
>     The main benefit of variadic functions templates over C vararg
>     functions is that they make use of the type system for type safety.
>     I'm not sure I see applying attribute format to them as a very
>     compelling use case.  (I'd expect the format string in a variadic
>     function template to use generic conversion specifiers, say %@ or
>     some such, and only let the caller specify things like flags, width
>     and precision but not type conversion specifiers).  Is there one
>     where relying on the type system isn't good enough?
> 
>      > This patch will introduce these changes:
>      > 1. It is no longer an error simply to have a function with the format
>      > attribute but no C-style variadic arguments
> 
>     I'm a little on the fence about this.  On the one hand it seems
>     unexpected to apply format checking to ordinary (non-variadic)
>     functions.  On the other, I can't think of anything wrong with it
>     and it even seems like it could be useful to specify a format for
>     a fixed number of arguments of fixed types.  Do you have an actual
>     use case for it or did it just fall out of the varaidic template
>     implementation?
> 
>      > 2. Functions are subjected to warnings/errors as before, except
>     errors
>      > mentioned in point 1 about not being variadic. For example, when a
>      > non-variadic function has wrong arguments, e.g
>      > __attribute__((format(printf, 1, 1))) or when being type-checked.
>      >
>      > Note that behaviours of C-style variadic functions do not change,
>     errors
>      > and warnings are given as before.
>      >
>      > This patch does it by:
>      > 1.   Relaxing several conditions for format attribute:
>      >       -  Will only use POSARG_ELLIPSIS flag to call
>     `get_constant` when
>      > getting attribute arguments of a variadic function
>      >       -  Relax the check for the last argument of the attribute
>     (will not
>      > require an ellipsis argument)
>      >       -  (Before this patch) After passing the above check,
>     current gcc will
>      > call `get_constant` to get the function parameter that the third
>     attribute
>      > argument is pointing to. If POSARG_ELLIPSIS is set,
>     `get_constant` will
>      > look for `...`. If not, `get_constant` will look for a C-style
>     string. Note
>      > that POSARG_ELLIPSIS is set automatically for getting the third
>     attribute
>      > argument.
>      >          (After this patch) POSARG_ELLIPSIS is set only when the
>     function
>      > has C-style '...'. Now, if POSARG_ELLIPSIS is not set,
>     `get_constant` will
>      > not check whether the third argument of format attribute points to a
>      > C-style string.
>      > 2.   Modifying expected outcome of a testcase in objc testsuite,
>     where we
>      > expect a warning instead of an error
>      > 3.   Adding 2 test files
>      >
>      > Successully bootstrapped and regression tested on
>     x86_64-pc-linux-gnu.
>      >
>      > Signed-off-by: Le Quang Tuan <lequangtuan6b@gmail.com
>     <mailto:lequangtuan6b@gmail.com>>
>      >
>      > gcc/c-family/ChangeLog:
>      >
>      > * c-attribs.c (positional_argument): allow third argument of format
>      > attribute to point to parameters of any type if the function is
>     not C-style
>      > variadic
>      > * c-format.c (decode_format_attr): read third argument with
>     POSARG_ELLIPSIS
>      > only if the function has has a variable argument
>      > (handle_format_attribute): relax explicit checks for non-variadic
>     functions
>      >
>      > gcc/testsuite/ChangeLog:
>      >
>      > * gcc.dg/format/attr-3.c: modify comment
>      > * objc.dg/attributes/method-format-1.m: errors do not hold anymore, a
>      > warning is given instead
>      > * g++.dg/warn/format9.C: New test with usage of variadic templates.
>      > * gcc.dg/format/attr-9.c: New test.
>      >
>      > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
>      > index 6bf492afcc0..7a17ce671de 100644
>      > --- a/gcc/c-family/c-attribs.c
>      > +++ b/gcc/c-family/c-attribs.c
>      > @@ -714,6 +714,11 @@ positional_argument (const_tree fntype,
>     const_tree
>      > atname, tree pos,
>      >     return NULL_TREE;
>      >    }
>      >
>      > +  /* For format attribute with argno >= 3, we don't expect any type
>      > +   */
>      > +  if (argno >= 3 && strcmp (IDENTIFIER_POINTER(atname),
>     "format") == 0 &&
>      > !(flags & POSARG_ELLIPSIS ) )
>      > +    return pos;
> 
>     Hardcoding knowledge of individual attributes in this function doesn't
>     seem very robust.  Avoiding that is the purpose of the flags argument.
>     I'd suggest adding a bit to the posargflags enum.
> 
>     Also, at this point, (flags & POSARG_ELLIPSIS) should be zero as
>     a result of the test above (not shown) so repeating the test shouldn't
>     be necessary.
> 
>     Martin
> 

Re: [PATCH][gcc] Allow functions without C-style ellipsis to use format attribute

[Joseph Myers]

On Mon, 19 Jul 2021, Martin Sebor via Gcc-patches wrote:

> You've answered my questions about the design (thank you) and I don't
> have any objections to the idea, but I'm not in a position to approve
> the patch.  I would suggest to get Jason's input on extending
> attribute format to variadic function templates, and Joseph's on
> extending it to ordinary (non-variadic) functions.  I've CC'd both.

One design question would be, if you allow this feature, whether it can be 
used in the case where there are no arguments to be formatted, with the 
corresponding attribute argument pointing just after the end of the 
function arguments.  My expectation would be that that case is valid (it 
could be used for a printf-like function that only substitutes no-argument 
formats such as %%), but if the argument number is any greater than that, 
there should be an error.  (And of course there would need to be test 
cases for that in the testsuite, both the valid case and the error for 
larger numbers, and the documentation of format attributes in extend.texi 
would need to describe such cases.)

-- 
Joseph S. Myers
joseph@codesourcery.com