[PATCH] D158318: [Sema] tolerate more promotion matches in format string checking

[Félix Cloutier via Phabricator via cfe-commits]

fcloutier created this revision.
fcloutier added a reviewer: aaron.ballman.
Herald added a project: All.
fcloutier requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It's been reported that when using __attribute__((format)) on non-variadic functions, certain values that normally get promoted when passed as variadic arguments now unconditionally emit a diagnostic:

  c
  void foo(const char *fmt, float f) __attribute__((format(printf, 1, 2)));
  void bar(void) {
  	foo("%g", 123.f);
  	//   ^ format specifies type 'double' but the argument has type 'float'
  }

This is normally not an issue because float values get promoted to doubles when passed as variadic arguments, but needless to say, variadic argument promotion does not apply to non-variadic arguments.

While this can be fixed by adjusting the prototype of `foo`, this is sometimes undesirable in C (for instance, if `foo` is ABI). In C++, using variadic templates, this might instead require call-site fixing, which is tedious and arguably needless work:

  c++
  template<typename... Args>
  void foo(const char *fmt, Args &&...args) __attribute__((format(printf, 1, 2)));
  void bar(void) {
  	foo("%g", 123.f);
  	//   ^ format specifies type 'double' but the argument has type 'float'
  }

To address this issue, we teach FormatString about a few promotions that have always been around but that have never been exercised in the direction that FormatString checks for:

- `char`, `unsigned char` -> `int`, `unsigned`
- `half`, `float16`, `float` -> `double`

This addresses issue https://github.com/llvm/llvm-project/issues/59824.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158318

Files:
  clang/lib/AST/FormatString.cpp
  clang/test/Sema/attr-format.c
  clang/test/SemaCXX/attr-format.cpp


Index: clang/test/SemaCXX/attr-format.cpp
===================================================================
--- clang/test/SemaCXX/attr-format.cpp
+++ clang/test/SemaCXX/attr-format.cpp
@@ -78,6 +78,9 @@
   format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
   format("bad format %s"); // expected-warning{{more '%' conversions than data arguments}}
 
+  format("%c %c %hhd %hd %d\n", (char)'a', 'a', 'a', (short)123, (int)123);
+  format("%f %f %f\n", (__fp16)123.f, 123.f, 123.);
+
   struct foo f;
   format_invalid_nonpod("hello %i", f); // expected-warning{{format specifies type 'int' but the argument has type 'struct foo'}}
 
Index: clang/test/Sema/attr-format.c
===================================================================
--- clang/test/Sema/attr-format.c
+++ clang/test/Sema/attr-format.c
@@ -94,13 +94,9 @@
   d3("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
 }
 
-__attribute__((format(printf, 1, 2))) void forward_fixed(const char *fmt, int i) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
-  forward_fixed(fmt, i);
-  a(fmt, i);
-}
-
-__attribute__((format(printf, 1, 2))) void forward_fixed_2(const char *fmt, int i, int j) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
-  forward_fixed_2(fmt, i, j);
-  a(fmt, i);
+__attribute__((format(printf, 1, 2)))
+void forward_fixed(const char *fmt, char i, short j, int k, float l, double m) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
+  forward_fixed(fmt, i, j, k, l, m);
+  a(fmt, i, j, k, l, m);
 }
 
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -465,6 +465,24 @@
                   T == C.WideCharTy)
                 return MatchPromotion;
               break;
+            case BuiltinType::Char_U:
+              if (T == C.UnsignedIntTy)
+                return MatchPromotion;
+              if (T == C.UnsignedShortTy)
+                return NoMatchPromotionTypeConfusion;
+              break;
+            case BuiltinType::Char_S:
+              if (T == C.IntTy)
+                return MatchPromotion;
+              if (T == C.ShortTy)
+                return NoMatchPromotionTypeConfusion;
+              break;
+            case BuiltinType::Half:
+            case BuiltinType::Float16:
+            case BuiltinType::Float:
+              if (T == C.DoubleTy)
+                return MatchPromotion;
+              break;
             case BuiltinType::Short:
             case BuiltinType::UShort:
               if (T == C.SignedCharTy || T == C.UnsignedCharTy)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D158318.551649.patch
Type: text/x-patch
Size: 2794 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230818/13ddae87/attachment-0001.bin>