[clang] [clang] Catch missing format attributes (PR #70024)

Budimir Aranđelović via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 1 05:03:46 PST 2023


https://github.com/budimirarandjelovicsyrmia updated https://github.com/llvm/llvm-project/pull/70024

>From a70de331784f4058f11ab6e01efaa025263bd232 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia <budimir.arandjelovic at syrmia.com>
Date: Fri, 13 Oct 2023 14:45:15 +0200
Subject: [PATCH] [clang] Catch missing format attributes

---
 clang/include/clang/Basic/DiagnosticGroups.td |   2 +-
 .../clang/Basic/DiagnosticSemaKinds.td        |   5 +
 clang/include/clang/Sema/Sema.h               |   4 +
 clang/lib/Sema/SemaChecking.cpp               |   4 +-
 clang/lib/Sema/SemaDeclAttr.cpp               | 107 ++++++++++++++++++
 clang/test/Sema/attr-format-missing.c         |  13 +++
 6 files changed, 133 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Sema/attr-format-missing.c

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 17fdcffa2d42740..b8b77df84beb2be 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -482,7 +482,7 @@ def MainReturnType : DiagGroup<"main-return-type">;
 def MaxUnsignedZero : DiagGroup<"max-unsigned-zero">;
 def MissingBraces : DiagGroup<"missing-braces">;
 def MissingDeclarations: DiagGroup<"missing-declarations">;
-def : DiagGroup<"missing-format-attribute">;
+def MissingFormatAttribute: DiagGroup<"missing-format-attribute">;
 def : DiagGroup<"missing-include-dirs">;
 def MissingNoreturn : DiagGroup<"missing-noreturn">;
 def MultiChar : DiagGroup<"multichar">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6d6f474f6dcdab9..6864fe3057a1df9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -936,6 +936,11 @@ def err_opencl_invalid_param : Error<
 def err_opencl_invalid_return : Error<
   "declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">;
 def warn_enum_value_overflow : Warning<"overflow in enumeration value">;
+def warn_missing_format_attribute : Warning<
+  "diagnostic behavior may be improved by adding the %0 format attribute to the declaration of %1">,
+  InGroup<MissingFormatAttribute>, DefaultIgnore;
+def note_insert_format_attribute_fixit: Note<
+  "insert %0 to silence this warning">;
 def warn_pragma_options_align_reset_failed : Warning<
   "#pragma options align=reset failed: %0">,
   InGroup<IgnoredPragmas>;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 741c2503127af7a..064506e70960333 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10615,6 +10615,10 @@ class Sema final {
     ChangedStateAtExit
   };
 
+  void DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+                                       ArrayRef<const Expr *> Args,
+                                       SourceLocation Loc);
+
   void DiagnoseNonDefaultPragmaAlignPack(PragmaAlignPackDiagnoseKind Kind,
                                          SourceLocation IncludeLoc);
   void DiagnoseUnterminatedPragmaAlignPack();
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 4602284309491c1..d3ac6cb519c56ed 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6014,8 +6014,10 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
     }
   }
 
-  if (FD)
+  if (FD) {
     diagnoseArgDependentDiagnoseIfAttrs(FD, ThisArg, Args, Loc);
+    DiagnoseMissingFormatAttributes(FD, Args, Range.getBegin());
+  }
 }
 
 /// CheckConstructorCall - Check a constructor call for correctness and safety
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 1a0bfb3d91bcc87..ea3c50de3b51bf8 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6849,6 +6849,113 @@ static void handleSwiftAsyncAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
 }
 
+// Warn if passed function has format attribute and their parent not.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+                                           ArrayRef<const Expr *> Args,
+                                           SourceLocation Loc) {
+  assert(FDecl);
+
+  unsigned BuiltinID = FDecl->getBuiltinID(/*ConsiderWrappers=*/true);
+
+  // If function is builtin and not listed in switch, exit.
+  switch (BuiltinID) {
+  case Builtin::NotBuiltin:
+    break;
+  case Builtin::BIprintf:
+  case Builtin::BIfprintf:
+  case Builtin::BIsprintf:
+  case Builtin::BIscanf:
+  case Builtin::BIfscanf:
+  case Builtin::BIsscanf:
+  case Builtin::BIvprintf:
+  case Builtin::BIvfprintf:
+  case Builtin::BIvsprintf:
+    break;
+  // C99 mode
+  case Builtin::BIsnprintf:
+  case Builtin::BIvsnprintf:
+  case Builtin::BIvscanf:
+  case Builtin::BIvfscanf:
+  case Builtin::BIvsscanf:
+    if (!getLangOpts().C99)
+      return;
+    break;
+  default:
+    return;
+  }
+
+  const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+  if (!ParentFuncDecl)
+    return;
+
+  // Iterate through function format attributes. Check if parent function
+  // has these attributes. If parent function does not have format
+  // attribute, emit warning.
+  for (const FormatAttr *Attr : FDecl->specific_attrs<FormatAttr>()) {
+    // Check if parent function has format attribute with correct arguments.
+    bool hasCorrectFormatAttr = llvm::any_of(
+        ParentFuncDecl->specific_attrs<FormatAttr>(),
+        [&](const FormatAttr *ParentAttr) {
+          // Check if first argument matches child function format argument
+          // type.
+          if (ParentAttr->getType() != Attr->getType())
+            return false;
+
+          // In C++ the implicit 'this' function parameter also counts
+          // Number of arguments does not count variadic argument
+          unsigned NumArgs = getFunctionOrMethodNumParams(ParentFuncDecl) +
+                             isInstanceMethod(ParentFuncDecl);
+
+          // Checks for second argument
+          unsigned FormatIdx = ParentAttr->getFormatIdx();
+          if (FormatIdx < 1 || FormatIdx > NumArgs)
+            return false;
+
+          // Check if second argument matches correct type.
+          QualType FormatType =
+              ParentFuncDecl->parameters()[FormatIdx - 1]->getType();
+          if (!isNSStringType(FormatType, Context, true) &&
+              !isCFStringType(FormatType, Context) &&
+              (!FormatType->isPointerType() ||
+               !FormatType->getPointeeType()->isCharType()))
+            return false;
+
+          // Check third argument, which is valid if it is either zero
+          // or greater by one than number of parent function arguments.
+          unsigned FirstArg = ParentAttr->getFirstArg();
+          return FirstArg == 0 || FirstArg == NumArgs + 1;
+        });
+
+    // porveri velicinu argumenta Args; da li je ovo provereno ranije?
+    // ako se kod izvrsava ne izvrsava van ovog uslova, obrni ga
+
+    if (hasCorrectFormatAttr)
+      continue;
+
+    Diag(Loc, diag::warn_missing_format_attribute)
+        << Attr->getType() << ParentFuncDecl;
+
+    int StringIndex = 0;
+    for (ParmVarDecl *Param : ParentFuncDecl->parameters()) {
+      if (Param->getType() == Args[Attr->getFirstArg()]->getType()) {
+        StringIndex = Param->getFunctionScopeIndex() + 1;
+        break;
+      }
+    }
+
+    int FirstToCheck =
+        ParentFuncDecl->isVariadic() ? (ParentFuncDecl->getNumParams() + 1) : 0;
+    SourceLocation ParentFuncLoc = ParentFuncDecl->getLocation();
+    std::string InsertionText;
+    llvm::raw_string_ostream OS(InsertionText);
+    OS << "__attribute__((format(" + Attr->getType()->getName() + ", " +
+              std::to_string(StringIndex) + ", " +
+              std::to_string(FirstToCheck) + ")))";
+    Diag(ParentFuncLoc, diag::note_insert_format_attribute_fixit)
+        << FixItHint::CreateInsertion(ParentFuncLoc, InsertionText);
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // Microsoft specific attribute handlers.
 //===----------------------------------------------------------------------===//
diff --git a/clang/test/Sema/attr-format-missing.c b/clang/test/Sema/attr-format-missing.c
new file mode 100644
index 000000000000000..1926d3d3163124b
--- /dev/null
+++ b/clang/test/Sema/attr-format-missing.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wmissing-format-attribute %s
+
+#include <stdarg.h>
+#include <stdio.h>
+
+__attribute__((__format__ (__scanf__, 1, 4)))
+void foo(char *out, const size_t len, const char *format, ... /* args */)
+{
+    va_list args;
+
+    vsnprintf(out, len, format, args); // expected-warning {{Function 'foo' might be candidate for 'printf' format attribute}}
+}



More information about the cfe-commits mailing list