]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
Fix format attribute for printf
authorTomas Kalibera <tomas.kalibera@gmail.com>
Sun, 20 Aug 2023 02:16:16 +0000 (02:16 +0000)
committerJonathan Yong <10walls@gmail.com>
Sun, 20 Aug 2023 02:22:02 +0000 (02:22 +0000)
Since a long time (GCC 4.4?) GCC does support annotating functions
with either the format attribute "gnu_printf" or "ms_printf" to
distinguish between different format string interpretations.

However, it seems like the attribute is ignored for the "printf"
symbol; regardless what the function declaration says, GCC treats
it as "ms_printf". This has become an issue now that mingw-w64
supports using the UCRT instead of msvcrt.dll, and in this case
the stdio functions are declared with the gnu_printf attribute,
and inttypes.h uses the same format specifiers as in GNU mode.

A reproducible example of the problem:

$ cat format.c
__attribute__((__format__ (gnu_printf, 1, 2))) int printf (const char *__format, ...);
__attribute__((__format__ (gnu_printf, 1, 2))) int othername (const char *__format, ...);

void function(void) {
    long long unsigned x = 42;
    othername("%llu\n", x);
    printf("%llu\n", x);
}
$ x86_64-w64-mingw32-gcc -c -Wformat format.c
format.c: In function 'function':
format.c:7:15: warning: unknown conversion type character 'l' in format [-Wformat=]
    7 |     printf("%llu\n", x);
      |               ^
format.c:7:12: warning: too many arguments for format [-Wformat-extra-args]
    7 |     printf("%llu\n", x);
      |            ^~~~~~~~

Note how both functions, printf and othername, are declare with
identical gnu_printf format attributes - GCC does take this into
account for "othername" and doesn't produce a warning, but GCC
seems to disregard the attribute in the printf declaration and
behave as if it was declared as ms_printf.

If the printf function declaration is changed into a static inline
function, the actual attribute used is honored though.

gcc/c-family/ChangeLog:

PR c/95130
* c-format.cc: skip default format for printf symbol if
explicitly declared by prototype.

Signed-off-by: Tomas Kalibera <tomas.kalibera@gmail.com>
Signed-off-by: Jonathan Yong <10walls@gmail.com>
gcc/c-family/c-format.cc

index b4eeebcb30e12120de674ab85401d2a4ea97a5e7..122ff9bd1cdafeb3e40b1e241124f94b21201b95 100644 (file)
@@ -1175,6 +1175,7 @@ check_function_format (const_tree fn, tree attrs, int nargs,
   tree a;
 
   tree atname = get_identifier ("format");
+  bool skipped_default_format = false;
 
   /* See if this function has any format attributes.  */
   for (a = attrs; a; a = TREE_CHAIN (a))
@@ -1185,6 +1186,38 @@ check_function_format (const_tree fn, tree attrs, int nargs,
          function_format_info info;
          decode_format_attr (fn, atname, TREE_VALUE (a), &info,
                              /*validated=*/true);
+
+         /* Mingw32 targets have traditionally used ms_printf format for the
+            printf function, and this format is built in GCC. But nowadays,
+            if mingw-w64 is configured to target UCRT, the printf function
+            uses the gnu_printf format (specified in the stdio.h header). This
+            causes GCC to check both formats, which means that GCC would
+            warn twice about the same issue when both formats are violated,
+            e.g. for %lu used to print long long unsigned.
+
+            Hence, if there is a built-in attribute specifier and at least
+            one another, we skip the built-in one. See PR 95130 (but note that
+            GCC ms_printf already supports %llu) and PR 92292.  */
+
+         if (!skipped_default_format
+             && fn
+             && TREE_CODE (fn) == FUNCTION_DECL
+             && fndecl_built_in_p (fn, BUILT_IN_NORMAL)
+             && (tree_to_uhwi (TREE_PURPOSE (TREE_VALUE (a)))
+                 & (int) ATTR_FLAG_BUILT_IN))
+           {
+             tree aa;
+             for (aa = attrs; aa; aa = TREE_CHAIN (aa))
+               if (a != aa
+                   && is_attribute_p ("format", get_attribute_name (aa)))
+                 {
+                   skipped_default_format = true;
+                   break;
+                 }
+               if (skipped_default_format)
+                 continue;
+           }
+
          if (warn_format)
            {
              /* FIXME: Rewrite all the internal functions in this file
@@ -5190,6 +5223,9 @@ handle_format_attribute (tree node[3], tree atname, tree args,
   if (TREE_CODE (TREE_VALUE (args)) == IDENTIFIER_NODE)
     TREE_VALUE (args) = canonicalize_attr_name (TREE_VALUE (args));
 
+  /* record the flags for check_function_format */
+  TREE_PURPOSE (args) = build_int_cst (unsigned_type_node, flags);
+
   if (!decode_format_attr (fndecl ? fndecl : type, atname, args, &info,
                           /* validated_p = */false))
     {