Update Catch missing format attributes

[vitalybuka]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Vitaly Buka (vitalybuka)

Changes

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaDeclAttr.cpp (+14-14)
• (modified) clang/test/Sema/attr-format-missing.c (+27-37)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 5166e61bb41d46..94d9ea5762fc17 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5339,9 +5339,9 @@ void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,
 
   // Check if there are more than one format type found. In that case do not
   // emit diagnostic.
-  const FormatAttr *FirstAttr = MissingFormatAttributes[0];
+  const clang::IdentifierInfo *AttrType = MissingFormatAttributes[0]->getType();
   if (llvm::any_of(MissingFormatAttributes, [&](const FormatAttr *Attr) {
-        return FirstAttr->getType() != Attr->getType();
+        return AttrType != Attr->getType();
       }))
     return;
 
@@ -5416,15 +5416,18 @@ Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
     // If child expression is function, check if it is format function.
     // If it is, check if parent function misses format attributes.
 
+    unsigned int ChildFunctionFormatArgumentIndexOffset =
+        checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;
+
+    if (!ChildFunction->hasAttr<FormatAttr>())
+      continue;
+
     // If child function is format function and format arguments are not
     // relevant to emit diagnostic, save only information about format type
     // (format index and first-to-check argument index are set to -1).
     // Information about format type is later used to determine if there are
     // more than one format type found.
 
-    unsigned int ChildFunctionFormatArgumentIndexOffset =
-        checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;
-
     // Check if function has format attribute with forwarded format string.
     IdentifierInfo *AttrType;
     const ParmVarDecl *FormatArg;
@@ -5465,7 +5468,7 @@ Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
       continue;
 
     // Check if format string argument is parent function parameter.
-    unsigned int StringIndex = 0;
+    int StringIndex = 0;
     if (!llvm::any_of(FDecl->parameters(), [&](const ParmVarDecl *Param) {
           if (Param != FormatArg)
             return false;
@@ -5506,16 +5509,13 @@ Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
     }
 
     // Get first argument index
-    unsigned FirstToCheck = [&]() -> unsigned {
+    int FirstToCheck = [&]() -> unsigned {
       if (!FDecl->isVariadic())
         return 0;
-      const auto *FirstToCheckArg =
-          dyn_cast<DeclRefExpr>(Args[NumArgs - 1]->IgnoreParenCasts());
-      if (!FirstToCheckArg)
-        return 0;
-
-      if (FirstToCheckArg->getType().getCanonicalType() !=
-          Context.getBuiltinVaListType().getCanonicalType())
+      const auto *FirstToCheckArg = Args[NumArgs - 1]->IgnoreParenCasts();
+      if (!FirstToCheckArg ||
+          FirstToCheckArg->getType().getCanonicalType() !=
+              Context.getBuiltinVaListType().getCanonicalType())
         return 0;
       return NumOfParentFunctionParams + FunctionFormatArgumentIndexOffset;
     }();
diff --git a/clang/test/Sema/attr-format-missing.c b/clang/test/Sema/attr-format-missing.c
index 4f9e91eb1becbc..e887e11d767040 100644
--- a/clang/test/Sema/attr-format-missing.c
+++ b/clang/test/Sema/attr-format-missing.c
@@ -3,14 +3,11 @@
 // RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -Wmissing-format-attribute %s
 // RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -std=c++2b -Wmissing-format-attribute %s
 // RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -std=c++23 -Wmissing-format-attribute %s
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple x86_64-linux %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-LIN64
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple x86_64-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple i386-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple i386-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
+// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 #ifndef __cplusplus
-typedef unsigned short char16_t;
-typedef unsigned int char32_t;
+typedef __CHAR16_TYPE__ char16_t;
+typedef __CHAR32_TYPE__ char32_t;
 typedef __WCHAR_TYPE__ wchar_t;
 #endif
 
@@ -84,16 +81,16 @@ void f7(const char *out, ... /* args */) // #f7
 {
     va_list args;
 
-    vscanf(out, &args[0]); // expected-warning@#f7 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f7'}}
-                           // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:6-[[@LINE-5]]:6}:"__attribute__((format(scanf, 1, 0)))"
+    vscanf(out, args); // expected-warning@#f7 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f7'}}
+                       // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:6-[[@LINE-5]]:6}:"__attribute__((format(scanf, 1, 2)))"
 }
 
 void f8(const char *out, ... /* args */) // #f8
 {
     va_list args;
 
-    vscanf(out, &args[0]); // expected-no-warning@#f8
-    vprintf(out, &args[0]); // expected-no-warning@#f8
+    vscanf(out, args); // expected-no-warning@#f8
+    vprintf(out, args); // expected-no-warning@#f8
 }
 
 void f9(const char out[], ... /* args */) // #f9
@@ -109,16 +106,18 @@ void f10(const wchar_t *out, ... /* args */) // #f10
 {
     va_list args;
     vscanf(out, args);
-#if __SIZEOF_WCHAR_T__ == 4
-                        // c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
+#if (defined(__aarch64__) && !defined(_WIN64)) || (defined(__arm__) && !defined(_WIN32))
+                        // c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned int *') to parameter of type 'const char *'}}
+#elif __SIZEOF_WCHAR_T__ == 4
+                        // c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
 #else
-                        // c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
+                        // c_diagnostics-warning@-6 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
 #endif
                         // c_diagnostics-note@#vscanf {{passing argument to parameter here}}
                         // c_diagnostics-warning@#f10 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f10'}}
-                        // cpp_diagnostics-error@-8 {{no matching function for call to 'vscanf'}}
-                        // cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st argument}}
                         // C-CHECK: fix-it:"{{.*}}":{[[@LINE-13]]:6-[[@LINE-13]]:6}:"__attribute__((format(scanf, 1, 2)))"
+                        // cpp_diagnostics-error@-11 {{no matching function for call to 'vscanf'}}
+                        // cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st argument}}
 }
 
 void f11(const wchar_t *out, ... /* args */) // #f11
@@ -139,16 +138,18 @@ void f13(const wchar_t *out, ... /* args */) // #f13
 {
     va_list args;
     vscanf(out, args);
-#if __SIZEOF_WCHAR_T__ == 4
-                        // c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
-#else
-                        // c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
+#if (defined(__aarch64__) && !defined(_WIN64)) || (defined(__arm__) && !defined(_WIN32))
+                        // c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned int *') to parameter of type 'const char *'}}
+#elif (defined(__x86_64__) && !defined(_WIN64)) || __SIZEOF_WCHAR_T__ == 4
+                        // c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
+#elif __SIZEOF_WCHAR_T__ == 2
+                        // c_diagnostics-warning@-6 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
 #endif
                         // c_diagnostics-note@#vscanf {{passing argument to parameter here}}
-                        // cpp_diagnostics-error@-7 {{no matching function for call to 'vscanf'}}
+                        // cpp_diagnostics-error@-9 {{no matching function for call to 'vscanf'}}
                         // cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st argument}}
                         // expected-warning@#f13 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f13'}}
-                        // CHECK: fix-it:"{{.*}}":{[[@LINE-13]]:6-[[@LINE-13]]:6}:"__attribute__((format(scanf, 1, 2)))"
+                        // CHECK: fix-it:"{{.*}}":{[[@LINE-15]]:6-[[@LINE-15]]:6}:"__attribute__((format(scanf, 1, 2)))"
     vscanf((const char *) out, args);
     vscanf((char *) out, args);
 }
@@ -327,9 +328,9 @@ void f33(char *out, va_list args) // #f33
 void f34(char *out, ... /* args */) // #f34
 {
     va_list args;
-    scanf(out, args); // expected-no-warning@#f34
+    scanf(out, args);
     {
-        scanf(out, args); // expected-no-warning@#f34
+        scanf(out, args);
     }
 }
 
@@ -341,7 +342,7 @@ void f35(char* ch, const char *out, ... /* args */) // #f35
     int a;
     printf(out, a); // expected-warning@#f35 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f35'}}
                     // CHECK: fix-it:"{{.*}}":{[[@LINE-7]]:6-[[@LINE-7]]:6}:"__attribute__((format(printf, 2, 0)))"
-    printf(out, 1); // no warning because first command above emitted same warning with same fix-it text
+    printf(out, 1); // no warning because first command above emitted same warning and fix-it
     printf(out, args); // expected-warning@#f35 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f35'}}
                        // CHECK: fix-it:"{{.*}}":{[[@LINE-10]]:6-[[@LINE-10]]:6}:"__attribute__((format(printf, 2, 3)))"
 }
@@ -387,17 +388,6 @@ void f40(char *out, ... /* args */) // #f40
 void f41(char *out, ... /* args */) // #f41
 {
     va_list args;
-    char *ch;
-    vscanf("%s", ch);
-#if defined(__x86_64__) && defined(__linux__)
-                        // c_diagnostics-warning@-2 {{incompatible pointer types passing 'char *' to parameter of type 'struct __va_list_tag *'}}
-                        // c_diagnostics-note@#vscanf {{passing argument to parameter here}}
-                        // cpp_diagnostics-error@-4 {{no matching function for call to 'vscanf'}}
-                        // cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'char *' to '__va_list_tag *' for 2nd argument}}
-#endif
-    vprintf(out, args);
-#if defined(__x86_64__) && defined(__linux__)
-                        // cpp_diagnostics-warning@#f41 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f41'}}
-                        // CHECK-LIN64: fix-it:"{{.*}}":{[[@LINE-14]]:6-[[@LINE-14]]:6}:"__attribute__((format(printf, 1, 2)))"
-#endif
+    vscanf("%s", args); // expected-no-warning@#f41
+    vprintf(out, args); // expected-no-warning@#f41
 }

[budimirarandjelovichtec]

Here are major changes.

[budimirarandjelovichtec]

I removed dynamic cast to DeclRefAttr of last function argument. In C++ mode casting to DeclRefAttr resulted in unrecognizing argument type as va_list for several architectures and OS.

[budimirarandjelovichtec]

This check was added because there is llvm::any_of later and one pointer can only be assigned inside llvm::any_of. This pointer is later called if format attribute was caught. This change prevents throwing error that unitialized value was used.

[budimirarandjelovichtec]

Seems that second argument is not passed properly.

[budimirarandjelovichtec]

Char * was passed to second parameter which accepts va_list type.