[Sema] Fix missing warnings for unused args with invalid printf specs. #158514

lakreite · 2025-09-14T21:52:51Z

Before this change, the analyzer didn't emit warnings for unused arguments when the format string contained invalid printf specifier (printf, os_log, NSLog) at the end of the format string.
Before:

    printf("%1", 123); // incomplete format specifier
    printf("%d %d %d %1", 123, 123, 123, 123); // incomplete format specifier
    printf("%1 %d", 123, 123); // incomplete format specifier | data argument not used by format string

After:

  printf("%1", 123); // incomplete format specifier | data argument not used by format string
  printf("%d %d %d %1", 123, 123, 123, 123); // incomplete format specifier | data argument not used by format string

Added new test cases in:

clang/test/Sema/format-strings.c
clang/test/SemaObjC/format-strings-objc.m
clang/test/SemaOpenCL/printf-format-strings.cl

github-actions · 2025-09-14T21:53:12Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-09-14T21:53:41Z

@llvm/pr-subscribers-clang

Author: Lakreite (lakreite)

Changes

Before this change, the analyzer didn't emit warnings for unused arguments when the format string contained invalid printf specifier (printf, os_log, NSLog) at the end of the format string.
Before:

    printf("%1", 123); // incomplete format specifier
    printf("%d %d %d %1", 123, 123, 123, 123); // incomplete format specifier
    printf("%1 %d", 123, 123); // incomplete format specifier | data argument not used by format string

After:

  printf("%1", 123); // incomplete format specifier | data argument not used by format string
  printf("%d %d %d %1", 123, 123, 123, 123); // incomplete format specifier | data argument not used by format string

Added new test cases in:

clang/test/Sema/format-strings.c
clang/test/SemaObjC/format-strings-objc.m
clang/test/SemaOpenCL/printf-format-strings.cl

Full diff: https://github.com/llvm/llvm-project/pull/158514.diff

4 Files Affected:

(modified) clang/lib/AST/PrintfFormatString.cpp (+1-1)
(modified) clang/test/Sema/format-strings.c (+14-14)
(modified) clang/test/SemaObjC/format-strings-objc.m (+5-5)
(modified) clang/test/SemaOpenCL/printf-format-strings.cl (+6-6)

diff --git a/clang/lib/AST/PrintfFormatString.cpp b/clang/lib/AST/PrintfFormatString.cpp
index 855550475721a..f829696a26473 100644
--- a/clang/lib/AST/PrintfFormatString.cpp
+++ b/clang/lib/AST/PrintfFormatString.cpp
@@ -440,7 +440,7 @@ bool clang::analyze_format_string::ParsePrintfString(FormatStringHandler &H,
     // Did a fail-stop error of any kind occur when parsing the specifier?
     // If so, don't do any more processing.
     if (FSR.shouldStop())
-      return true;
+      return false;
     // Did we exhaust the string or encounter an error that
     // we can recover from?
     if (!FSR.hasValue())
diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c
index af30ad5d15fe2..e2f03089c12be 100644
--- a/clang/test/Sema/format-strings.c
+++ b/clang/test/Sema/format-strings.c
@@ -207,9 +207,9 @@ void check_invalid_specifier(FILE* fp, char *buf)
 
 void check_null_char_string(char* b)
 {
-  printf("\0this is bogus%d",1); // expected-warning {{string contains '\0'}}
-  snprintf(b,10,"%%%%%d\0%d",1,2); // expected-warning {{string contains '\0'}}
-  printf("%\0d",1); // expected-warning {{string contains '\0'}}
+  printf("\0this is bogus%d",1); // expected-warning {{string contains '\0'}} expected-warning {{data argument not used by format string}}
+  snprintf(b,10,"%%%%%d\0%d",1,2); // expected-warning {{string contains '\0'}} expected-warning {{data argument not used by format string}}
+  printf("%\0d",1); // expected-warning {{string contains '\0'}} expected-warning {{data argument not used by format string}}
 }
 
 void check_empty_format_string(char* buf, ...)
@@ -297,7 +297,7 @@ void test10(int x, float f, int i, long long lli) {
   printf("%W%d\n", x, x); // expected-warning{{invalid conversion specifier 'W'}}  expected-warning {{data argument not used by format string}}
   printf("%"); // expected-warning{{incomplete format specifier}}
   printf("%.d", x); // no-warning
-  printf("%.", x);  // expected-warning{{incomplete format specifier}}
+  printf("%.", x);  // expected-warning{{incomplete format specifier}} expected-warning {{data argument not used by format string}}
   printf("%f", 4); // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
   printf("%qd", lli); // no-warning
   printf("%qd", x); // expected-warning{{format specifies type 'long long' but the argument has type 'int'}}
@@ -390,8 +390,8 @@ void test_unicode_conversions(wchar_t *s) {
 // This is an IEEE extension (IEEE Std 1003.1).
 // FIXME: This is probably not portable everywhere.
 void test_positional_arguments(void) {
-  printf("%0$", (int)2); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
-  printf("%1$*0$d", (int) 2); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
+  printf("%0$", (int)2); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}} expected-warning{{data argument not used by format string}}
+  printf("%1$*0$d", (int) 2); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}} expected-warning{{data argument not used by format string}}
   printf("%1$d", (int) 2); // no-warning
   printf("%1$d", (int) 2, 2); // expected-warning{{data argument not used by format string}}
   printf("%1$d%1$f", (int) 2); // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
@@ -595,16 +595,16 @@ void pr9751(void) {
   printf("%y", 5); // expected-warning{{invalid conversion specifier 'y'}}
 
   const char kFormat5[] = "%."; // expected-note{{format string is defined here}}
-  printf(kFormat5, 5); // expected-warning{{incomplete format specifier}}
-  printf("%.", 5); // expected-warning{{incomplete format specifier}}
+  printf(kFormat5, 5); // expected-warning{{incomplete format specifier}} expected-warning{{data argument not used by format string}}
+  printf("%.", 5); // expected-warning{{incomplete format specifier}} expected-warning{{data argument not used by format string}}
 
   const char kFormat6[] = "%s"; // expected-note{{format string is defined here}}
   printf(kFormat6, 5); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
   printf("%s", 5); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
 
   const char kFormat7[] = "%0$"; // expected-note{{format string is defined here}}
-  printf(kFormat7, 5); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
-  printf("%0$", 5); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}}
+  printf(kFormat7, 5); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}} expected-warning{{data argument not used by format string}}
+  printf("%0$", 5); // expected-warning{{position arguments in format strings start counting at 1 (not 0)}} expected-warning{{data argument not used by format string}}
 
   const char kFormat8[] = "%1$d %d"; // expected-note{{format string is defined here}}
   printf(kFormat8, 4, 4); // expected-warning{{cannot mix positional and non-positional arguments in format string}}
@@ -615,8 +615,8 @@ void pr9751(void) {
   printf("", 4, 4); // expected-warning{{format string is empty}}
 
   const char kFormat10[] = "\0%d"; // expected-note{{format string is defined here}}
-  printf(kFormat10, 4); // expected-warning{{format string contains '\0' within the string body}}
-  printf("\0%d", 4); // expected-warning{{format string contains '\0' within the string body}}
+  printf(kFormat10, 4); // expected-warning{{format string contains '\0' within the string body}} expected-warning{{data argument not used by format string}}
+  printf("\0%d", 4); // expected-warning{{format string contains '\0' within the string body}} expected-warning{{data argument not used by format string}}
 
   const char kFormat11[] = "%*d"; // expected-note{{format string is defined here}}
   printf(kFormat11); // expected-warning{{'*' specified field width is missing a matching 'int' argument}}
@@ -720,8 +720,8 @@ extern void test14_bar(const char *, const char *, ...)
      __attribute__((__format__(__printf__, 1, 3)));
 
 void test14_zed(int *p) {
-  test14_foo("%", "%d", p); // expected-warning{{incomplete format specifier}}
-  test14_bar("%", "%d", p); // expected-warning{{incomplete format specifier}}
+  test14_foo("%", "%d", p); // expected-warning{{incomplete format specifier}} expected-warning{{data argument not used by format string}}
+  test14_bar("%", "%d", p); // expected-warning{{incomplete format specifier}} expected-warning{{data argument not used by format string}}
 }
 
 #if !defined(__ANDROID__) && !defined(__Fuchsia__)
diff --git a/clang/test/SemaObjC/format-strings-objc.m b/clang/test/SemaObjC/format-strings-objc.m
index 40c1d31b1fd4c..aba1f6153a9a4 100644
--- a/clang/test/SemaObjC/format-strings-objc.m
+++ b/clang/test/SemaObjC/format-strings-objc.m
@@ -274,15 +274,15 @@ void testUnicode(void) {
 
 // Test Objective-C modifier flags.
 void testObjCModifierFlags(void) {
-  NSLog(@"%[]@", @"Foo"); // expected-warning {{missing object format flag}}
-  NSLog(@"%[", @"Foo"); // expected-warning {{incomplete format specifier}}
-  NSLog(@"%[tt", @"Foo");  // expected-warning {{incomplete format specifier}}
+  NSLog(@"%[]@", @"Foo"); // expected-warning {{missing object format flag}} expected-warning{{data argument not used by format string}}
+  NSLog(@"%[", @"Foo"); // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
+  NSLog(@"%[tt", @"Foo");  // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
   NSLog(@"%[tt]@", @"Foo"); // no-warning
   NSLog(@"%[tt]@ %s", @"Foo", "hello"); // no-warning
   NSLog(@"%s %[tt]@", "hello", @"Foo"); // no-warning
-  NSLog(@"%[blark]@", @"Foo"); // expected-warning {{'blark' is not a valid object format flag}}
+  NSLog(@"%[blark]@", @"Foo"); // expected-warning {{'blark' is not a valid object format flag}} expected-warning{{data argument not used by format string}}
   NSLog(@"%2$[tt]@ %1$[tt]@", @"Foo", @"Bar"); // no-warning
-  NSLog(@"%2$[tt]@ %1$[tt]s", @"Foo", @"Bar"); // expected-warning {{object format flags cannot be used with 's' conversion specifier}}
+  NSLog(@"%2$[tt]@ %1$[tt]s", @"Foo", @"Bar"); // expected-warning {{object format flags cannot be used with 's' conversion specifier}} expected-warning{{data argument not used by format string}}
 }
 
 @interface RD23622446_Tester: NSObject
diff --git a/clang/test/SemaOpenCL/printf-format-strings.cl b/clang/test/SemaOpenCL/printf-format-strings.cl
index 6cdfc7e60b379..e5b1e5a914290 100644
--- a/clang/test/SemaOpenCL/printf-format-strings.cl
+++ b/clang/test/SemaOpenCL/printf-format-strings.cl
@@ -96,18 +96,18 @@ kernel void format_v4f32(float4 arg)
 
 kernel void format_only_v(int arg)
 {
-    printf("%v", arg); // expected-warning {{incomplete format specifier}}
+    printf("%v", arg); // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
 }
 
 kernel void format_missing_num(int arg)
 {
-    printf("%v4", arg); // expected-warning {{incomplete format specifier}}
+    printf("%v4", arg); // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
 }
 
 kernel void format_not_num(int arg)
 {
-    printf("%vNd", arg); // expected-warning {{incomplete format specifier}}
-    printf("%v*d", arg); // expected-warning {{incomplete format specifier}}
+    printf("%vNd", arg); // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
+    printf("%v*d", arg); // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
 }
 
 kernel void format_v16i32(int16 arg)
@@ -127,7 +127,7 @@ kernel void format_v4i32_wrong_num_elts_2_to_4(int2 arg)
 
 kernel void format_missing_num_elts_format(int4 arg)
 {
-    printf("%vd\n", arg); // expected-warning {{incomplete format specifier}}
+    printf("%vd\n", arg); // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
 }
 
 kernel void format_v4f32_scalar(float arg)
@@ -142,7 +142,7 @@ kernel void format_v4f32_wrong_num_elts(float2 arg)
 
 kernel void format_missing_num_elts(float4 arg)
 {
-    printf("%vf\n", arg); // expected-warning {{incomplete format specifier}}
+    printf("%vf\n", arg); // expected-warning {{incomplete format specifier}} expected-warning{{data argument not used by format string}}
 }
 
 kernel void vector_precision_modifier_v4i32_to_v4f32(int4 arg)

lakreite · 2025-09-15T10:23:31Z

Please review by @erichkeane, @haoNoQ, or anyone familiar with Sema and format strings.

Fznamznon

I'm not sure we want this more chatty? It is kind of obvious that for an invalid format specifier the data argument wont be used. Adding more reviewers

erichkeane

I don't think this is really a good idea. Typically any time in the compiler we 'continue' after failing parsing like this, we get bogus errors as much as valuable errors. At worst, we get crashes as the rest of the assumptions in the HandlePrintfSpecifier are going to depend on successful parse.

I'm not convinced as to the value here.

lakreite · 2025-09-15T13:18:38Z

Thank you for the feedback. I think there's a bug, because...
With invalid conversion specifiers clang emits a warning:

main.cpp:5:17: warning: invalid conversion specifier 'k' [-Wformat-invalid-specifier]
    5 |     printf("%d %k", 1, 1);
      |                ~^
main.cpp:5:24: warning: data argument not used by format string [-Wformat-extra-args]
    5 |     printf("%d %k", 1, 1);
      |            ~~~~~~~     ^
2 warnings generated.

With incomplete specifier not at the last position Clang emits a warning:

main.cpp:5:15: warning: invalid conversion specifier ' ' [-Wformat-invalid-specifier]
    5 |     printf("%1 %d", 1, 1);
      |             ~~^
main.cpp:5:24: warning: data argument not used by format string [-Wformat-extra-args]
    5 |     printf("%1 %d", 1, 1);
      |            ~~~~~~~     ^
2 warnings generated.

So missing a warning looks strange:

main.cpp:5:13: warning: incomplete format specifier [-Wformat]
    5 |     printf("%1", 1, 1, 1);
      |             ^~
1 warning generated.

Sirraide

Yeah, I agree that it doesn’t seem helpful to try and determine how many arguments we actually want if there’s a problem w/ the format string.

lakreite · 2025-09-25T10:53:34Z

This diagnostic already exists and only flags the very first unconsumed argument in printf.
My fix addresses only the case where an incomplete format specifier appears at the end of the format string.
HandlePrintfSpecifier is unrelated to control flow here (whether it returns true or false), because my change affects only the call to H.DoneProcessing(), which is responsible for diagnosing the first unconsumed argument.
I've merely added new tests for the edge case, and no existing diagnostics were removed or altered – all previous diagnostic messages remain intact.

lakreite added 2 commits September 14, 2025 23:46

[Sema] Fix missing warnings for unused args with invalid printf specs.

ef99801

Merge branch 'main' into fix-format-unused-args-invalid-spec

0fafb04

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 14, 2025

lakreite added 3 commits September 15, 2025 00:57

Merge branch 'main' into fix-format-unused-args-invalid-spec

16f6dd3

Merge branch 'main' into fix-format-unused-args-invalid-spec

fa8e5a1

Merge branch 'main' into fix-format-unused-args-invalid-spec

3e9d70f

Fznamznon requested review from erichkeane, Sirraide and cor3ntin September 15, 2025 12:22

Fznamznon reviewed Sep 15, 2025

View reviewed changes

Fznamznon requested a review from shafik September 15, 2025 12:24

erichkeane reviewed Sep 15, 2025

View reviewed changes

Sirraide reviewed Sep 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Sema] Fix missing warnings for unused args with invalid printf specs. #158514

[Sema] Fix missing warnings for unused args with invalid printf specs. #158514

lakreite commented Sep 14, 2025

Uh oh!

github-actions bot commented Sep 14, 2025

Uh oh!

llvmbot commented Sep 14, 2025

Uh oh!

lakreite commented Sep 15, 2025 •

edited

Loading

Uh oh!

Fznamznon left a comment

Uh oh!

erichkeane left a comment

Uh oh!

lakreite commented Sep 15, 2025

Uh oh!

Sirraide left a comment

Uh oh!

lakreite commented Sep 25, 2025

Uh oh!

Uh oh!

[Sema] Fix missing warnings for unused args with invalid printf specs. #158514

Are you sure you want to change the base?

[Sema] Fix missing warnings for unused args with invalid printf specs. #158514

Conversation

lakreite commented Sep 14, 2025

Uh oh!

github-actions bot commented Sep 14, 2025

Uh oh!

llvmbot commented Sep 14, 2025

Uh oh!

lakreite commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fznamznon left a comment

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

lakreite commented Sep 15, 2025

Uh oh!

Sirraide left a comment

Choose a reason for hiding this comment

Uh oh!

lakreite commented Sep 25, 2025

Uh oh!

Uh oh!

lakreite commented Sep 15, 2025 •

edited

Loading