[clang] [Sema] Suggest missing format attributes (PR #166738)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 6 02:18:52 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Vladimir Vuksanovic (vvuksanovic)
<details>
<summary>Changes</summary>
Implements the `-Wmissing-format-attribute` diagnostic. It suggests adding format attributes to function declarations that call other format functions and pass the format string and arguments to them.
---
Patch is 27.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/166738.diff
7 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+2)
- (modified) clang/include/clang/Basic/DiagnosticGroups.td (-1)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+5)
- (modified) clang/lib/Sema/SemaChecking.cpp (+103-22)
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+2-2)
- (added) clang/test/Sema/attr-format-missing.c (+217)
- (added) clang/test/Sema/attr-format-missing.cpp (+68)
``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 32f669f8d70d8..be384b7db72a3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -401,6 +401,8 @@ Improvements to Clang's diagnostics
or continue (#GH166013)
- Clang now emits a diagnostic in case `vector_size` or `ext_vector_type`
attributes are used with a negative size (#GH165463).
+- Clang now detects potential missing format attributes on function declarations
+ when calling format functions. (#GH60718)
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 1e0321de3f4b6..7b534d416fdd9 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -607,7 +607,6 @@ 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 MissingIncludeDirs : 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 fa509536bf021..78c5caf79eab7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3478,6 +3478,11 @@ def err_format_attribute_result_not : Error<"function does not return %0">;
def err_format_attribute_implicit_this_format_string : Error<
"format attribute cannot specify the implicit this argument as the format "
"string">;
+def warn_missing_format_attribute
+ : Warning<"diagnostic behavior may be improved by adding the '%0' format "
+ "attribute to the declaration of %1">,
+ InGroup<DiagGroup<"missing-format-attribute">>,
+ DefaultIgnore;
def err_callback_attribute_no_callee : Error<
"'callback' attribute specifies no callback callee">;
def err_callback_attribute_invalid_callee : Error<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ad2c2e4a97bb9..502e9fd099144 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6517,7 +6517,8 @@ static StringLiteralCheckType checkFormatStringExpr(
unsigned format_idx, unsigned firstDataArg, FormatStringType Type,
VariadicCallType CallType, bool InFunctionCall,
llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
- llvm::APSInt Offset, bool IgnoreStringsWithoutSpecifiers = false) {
+ llvm::APSInt Offset, std::optional<unsigned> *CallerParamIdx = nullptr,
+ bool IgnoreStringsWithoutSpecifiers = false) {
if (S.isConstantEvaluatedContext())
return SLCT_NotALiteral;
tryAgain:
@@ -6542,7 +6543,7 @@ static StringLiteralCheckType checkFormatStringExpr(
return checkFormatStringExpr(
S, ReferenceFormatString, SLE, Args, APK, format_idx, firstDataArg,
Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs,
- UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
+ UncoveredArg, Offset, CallerParamIdx, IgnoreStringsWithoutSpecifiers);
}
return SLCT_NotALiteral;
case Stmt::BinaryConditionalOperatorClass:
@@ -6577,7 +6578,7 @@ static StringLiteralCheckType checkFormatStringExpr(
Left = checkFormatStringExpr(
S, ReferenceFormatString, C->getTrueExpr(), Args, APK, format_idx,
firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs,
- UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
+ UncoveredArg, Offset, CallerParamIdx, IgnoreStringsWithoutSpecifiers);
if (Left == SLCT_NotALiteral || !CheckRight) {
return Left;
}
@@ -6586,7 +6587,7 @@ static StringLiteralCheckType checkFormatStringExpr(
StringLiteralCheckType Right = checkFormatStringExpr(
S, ReferenceFormatString, C->getFalseExpr(), Args, APK, format_idx,
firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs,
- UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
+ UncoveredArg, Offset, CallerParamIdx, IgnoreStringsWithoutSpecifiers);
return (CheckLeft && Left < Right) ? Left : Right;
}
@@ -6635,10 +6636,11 @@ static StringLiteralCheckType checkFormatStringExpr(
if (InitList->isStringLiteralInit())
Init = InitList->getInit(0)->IgnoreParenImpCasts();
}
- return checkFormatStringExpr(
- S, ReferenceFormatString, Init, Args, APK, format_idx,
- firstDataArg, Type, CallType,
- /*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, Offset);
+ return checkFormatStringExpr(S, ReferenceFormatString, Init, Args,
+ APK, format_idx, firstDataArg, Type,
+ CallType,
+ /*InFunctionCall*/ false, CheckedVarArgs,
+ UncoveredArg, Offset, CallerParamIdx);
}
}
@@ -6690,6 +6692,8 @@ static StringLiteralCheckType checkFormatStringExpr(
// format arguments, in all cases.
//
if (const auto *PV = dyn_cast<ParmVarDecl>(VD)) {
+ if (CallerParamIdx)
+ *CallerParamIdx = PV->getFunctionScopeIndex();
if (const auto *D = dyn_cast<Decl>(PV->getDeclContext())) {
for (const auto *PVFormatMatches :
D->specific_attrs<FormatMatchesAttr>()) {
@@ -6715,7 +6719,7 @@ static StringLiteralCheckType checkFormatStringExpr(
S, ReferenceFormatString, PVFormatMatches->getFormatString(),
Args, APK, format_idx, firstDataArg, Type, CallType,
/*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg,
- Offset, IgnoreStringsWithoutSpecifiers);
+ Offset, CallerParamIdx, IgnoreStringsWithoutSpecifiers);
}
}
@@ -6770,7 +6774,7 @@ static StringLiteralCheckType checkFormatStringExpr(
StringLiteralCheckType Result = checkFormatStringExpr(
S, ReferenceFormatString, Arg, Args, APK, format_idx, firstDataArg,
Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg,
- Offset, IgnoreStringsWithoutSpecifiers);
+ Offset, CallerParamIdx, IgnoreStringsWithoutSpecifiers);
if (IsFirst) {
CommonResult = Result;
IsFirst = false;
@@ -6784,10 +6788,11 @@ static StringLiteralCheckType checkFormatStringExpr(
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
const Expr *Arg = CE->getArg(0);
- return checkFormatStringExpr(
- S, ReferenceFormatString, Arg, Args, APK, format_idx,
- firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs,
- UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
+ return checkFormatStringExpr(S, ReferenceFormatString, Arg, Args, APK,
+ format_idx, firstDataArg, Type, CallType,
+ InFunctionCall, CheckedVarArgs,
+ UncoveredArg, Offset, CallerParamIdx,
+ IgnoreStringsWithoutSpecifiers);
}
}
}
@@ -6795,7 +6800,7 @@ static StringLiteralCheckType checkFormatStringExpr(
return checkFormatStringExpr(
S, ReferenceFormatString, SLE, Args, APK, format_idx, firstDataArg,
Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs,
- UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
+ UncoveredArg, Offset, CallerParamIdx, IgnoreStringsWithoutSpecifiers);
return SLCT_NotALiteral;
}
case Stmt::ObjCMessageExprClass: {
@@ -6821,7 +6826,7 @@ static StringLiteralCheckType checkFormatStringExpr(
return checkFormatStringExpr(
S, ReferenceFormatString, Arg, Args, APK, format_idx, firstDataArg,
Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg,
- Offset, IgnoreStringsWithoutSpecifiers);
+ Offset, CallerParamIdx, IgnoreStringsWithoutSpecifiers);
}
}
@@ -7001,6 +7006,77 @@ bool Sema::CheckFormatString(const FormatMatchesAttr *Format,
return false;
}
+static void CheckMissingFormatAttributes(Sema *S, FormatStringType FormatType,
+ unsigned FormatIdx, unsigned FirstArg,
+ ArrayRef<const Expr *> Args,
+ Sema::FormatArgumentPassingKind APK,
+ unsigned CallerParamIdx,
+ SourceLocation Loc) {
+ const FunctionDecl *Caller = S->getCurFunctionDecl();
+ if (!Caller)
+ return;
+
+ // Find the offset to convert between attribute and parameter indexes.
+ unsigned CallerArgumentIndexOffset =
+ hasImplicitObjectParameter(Caller) ? 2 : 1;
+
+ unsigned FirstArgumentIndex = -1;
+ switch (APK) {
+ case Sema::FormatArgumentPassingKind::FAPK_Fixed:
+ case Sema::FormatArgumentPassingKind::FAPK_Variadic: {
+ // As an extension, clang allows the format attribute on non-variadic
+ // functions.
+ // Caller must have fixed arguments to pass them to a fixed or variadic
+ // function. Try to match caller and callee arguments. If successful, then
+ // emit a diag with the caller idx, otherwise we can't determine the callee
+ // arguments.
+ unsigned NumCalleeArgs = Args.size() - FirstArg;
+ if (NumCalleeArgs == 0 || Caller->getNumParams() < NumCalleeArgs) {
+ // There aren't enough arugments in the caller to pass to callee.
+ return;
+ }
+ for (unsigned CalleeIdx = Args.size() - 1,
+ CallerIdx = Caller->getNumParams() - 1;
+ CalleeIdx >= FirstArg; --CalleeIdx, --CallerIdx) {
+ const auto *Arg =
+ dyn_cast<DeclRefExpr>(Args[CalleeIdx]->IgnoreParenCasts());
+ if (!Arg)
+ return;
+ const auto *Param = dyn_cast<ParmVarDecl>(Arg->getDecl());
+ if (!Param || Param->getFunctionScopeIndex() != CallerIdx)
+ return;
+ }
+ FirstArgumentIndex =
+ Caller->getNumParams() + CallerArgumentIndexOffset - NumCalleeArgs;
+ break;
+ }
+ case Sema::FormatArgumentPassingKind::FAPK_VAList:
+ // Caller arguments are either variadic or a va_list.
+ FirstArgumentIndex =
+ Caller->isVariadic()
+ ? (Caller->getNumParams() + CallerArgumentIndexOffset)
+ : 0;
+ break;
+ case Sema::FormatArgumentPassingKind::FAPK_Elsewhere:
+ // Args are not passed to the callee.
+ return;
+ }
+
+ // Emit the diagnostic and fixit.
+ unsigned FormatStringIndex = CallerParamIdx + CallerArgumentIndexOffset;
+ StringRef FormatTypeName = S->GetFormatStringTypeName(FormatType);
+ S->Diag(Loc, diag::warn_missing_format_attribute)
+ << FormatTypeName << Caller
+ << FixItHint::CreateInsertion(Caller->getFirstDecl()->getLocation(),
+ (llvm::Twine("__attribute__((format(") +
+ FormatTypeName + ", " +
+ llvm::Twine(FormatStringIndex) + ", " +
+ llvm::Twine(FirstArgumentIndex) + ")))")
+ .str());
+ S->Diag(Caller->getFirstDecl()->getLocation(), diag::note_callee_decl)
+ << Caller;
+}
+
bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
Sema::FormatArgumentPassingKind APK,
const StringLiteral *ReferenceFormatString,
@@ -7030,11 +7106,12 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
// ObjC string uses the same format specifiers as C string, so we can use
// the same format string checking logic for both ObjC and C strings.
UncoveredArgHandler UncoveredArg;
+ std::optional<unsigned> CallerParamIdx;
StringLiteralCheckType CT = checkFormatStringExpr(
*this, ReferenceFormatString, OrigFormatExpr, Args, APK, format_idx,
firstDataArg, Type, CallType,
/*IsFunctionCall*/ true, CheckedVarArgs, UncoveredArg,
- /*no string offset*/ llvm::APSInt(64, false) = 0);
+ /*no string offset*/ llvm::APSInt(64, false) = 0, &CallerParamIdx);
// Generate a diagnostic where an uncovered argument is detected.
if (UncoveredArg.hasUncoveredArg()) {
@@ -7047,11 +7124,6 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
// Literal format string found, check done!
return CT == SLCT_CheckedLiteral;
- // Strftime is particular as it always uses a single 'time' argument,
- // so it is safe to pass a non-literal string.
- if (Type == FormatStringType::Strftime)
- return false;
-
// Do not emit diag when the string param is a macro expansion and the
// format is either NSString or CFString. This is a hack to prevent
// diag when using the NSLocalizedString and CFCopyLocalizedString macros
@@ -7061,6 +7133,15 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
SourceMgr.isInSystemMacro(FormatLoc))
return false;
+ if (CallerParamIdx)
+ CheckMissingFormatAttributes(this, Type, format_idx, firstDataArg, Args,
+ APK, *CallerParamIdx, Loc);
+
+ // Strftime is particular as it always uses a single 'time' argument,
+ // so it is safe to pass a non-literal string.
+ if (Type == FormatStringType::Strftime)
+ return false;
+
// If there are no arguments specified, warn with -Wformat-security, otherwise
// warn only with -Wformat-nonliteral.
if (Args.size() == firstDataArg) {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a9e7b44ac9d73..9849e125889cf 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3572,7 +3572,7 @@ static void handleEnumExtensibilityAttr(Sema &S, Decl *D,
}
/// Handle __attribute__((format_arg((idx)))) attribute based on
-/// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
+/// https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
static void handleFormatArgAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
const Expr *IdxExpr = AL.getArgAsExpr(0);
ParamIdx Idx;
@@ -3771,7 +3771,7 @@ struct FormatAttrCommon {
};
/// Handle __attribute__((format(type,idx,firstarg))) attributes based on
-/// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
+/// https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
static bool handleFormatAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
FormatAttrCommon *Info) {
// Checks the first two arguments of the attribute; this is shared between
diff --git a/clang/test/Sema/attr-format-missing.c b/clang/test/Sema/attr-format-missing.c
new file mode 100644
index 0000000000000..7e5131153f661
--- /dev/null
+++ b/clang/test/Sema/attr-format-missing.c
@@ -0,0 +1,217 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+typedef unsigned long size_t;
+typedef long ssize_t;
+typedef __builtin_va_list va_list;
+
+__attribute__((format(printf, 1, 2)))
+int printf(const char *, ...);
+
+__attribute__((format(scanf, 1, 2)))
+int scanf(const char *, ...);
+
+__attribute__((format(printf, 1, 0)))
+int vprintf(const char *, va_list);
+
+__attribute__((format(scanf, 1, 0)))
+int vscanf(const char *, va_list);
+
+__attribute__((format(printf, 2, 0)))
+int vsprintf(char *, const char *, va_list);
+
+struct tm { unsigned i; };
+__attribute__((format(strftime, 3, 0)))
+size_t strftime(char *, size_t, const char *, const struct tm *);
+
+__attribute__((format(strfmon, 3, 4)))
+ssize_t strfmon(char *, size_t, const char *, ...);
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:6}:"__attribute__((format(printf, 1, 0)))"
+void f1(char *out, va_list args) // #f1
+{
+ vprintf(out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f1'}}
+ // expected-note@#f1 {{'f1' declared here}}
+}
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:6}:"__attribute__((format(scanf, 1, 0)))"
+void f2(const char *out, va_list args) // #f2
+{
+ vscanf(out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f2'}}
+ // expected-note@#f2 {{'f2' declared here}}
+}
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:6}:"__attribute__((format(printf, 1, 2)))"
+void f3(const char *out, ... /* args */) // #f3
+{
+ va_list args;
+ vprintf(out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f3'}}
+ // expected-note@#f3 {{'f3' declared here}}
+}
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:6}:"__attribute__((format(scanf, 1, 2)))"
+void f4(const char *out, ... /* args */) // #f4
+{
+ va_list args;
+ vscanf(out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f4'}}
+ // expected-note@#f4 {{'f4' declared here}}
+}
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:6-[[@LINE+2]]:6}:"__attribute__((format(printf, 2, 3)))"
+__attribute__((format(printf, 1, 3)))
+void f5(char *out, const char *format, ... /* args */) // #f5
+{
+ va_list args;
+ vsprintf(out, format, args); // expected-warning {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f5'}}
+ // expected-note@#f5 {{'f5' declared here}}
+}
+
+__attribute__((format(scanf, 1, 3)))
+void f6(char *out, const char *format, ... /* args */) // #f6
+{
+ va_list args;
+ vsprintf(out, format, args); // expected-warning {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f6'}}
+ // expected-note@#f6 {{'f6' declared here}}
+}
+
+// Ok, out is not passed to print functions.
+void f7(char* out, ... /* args */)
+{
+ va_list args;
+
+ const char *ch = "format";
+ vprintf(ch, args);
+ vprintf("test", args);
+}
+
+// Ok, out is not passed to print functions.
+void f8(char *out, va_list args)
+{
+ const char *ch = "format";
+ vprintf(ch, args);
+ vprintf("test", args);
+}
+
+// Ok, out is not passed to scan functions.
+void f9(va_list args)
+{
+ const char *ch = "format";
+ vscanf(ch, args);
+ vscanf("test", args);
+}
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:6-[[@LINE+2]]:6}:"__attribute__((format(scanf, 1, 2)))"
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:6}:"__attribute__((format(printf, 1, 2)))"
+void f10(const char *out, ... /* args */) // #f10
+{
+ va_list args;
+ vscanf(out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f10'}}
+ // expected-note@#f10 {{'f10' declared here}}
+ vprintf(out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f10'}}
+ // expected-note@#f10 {{'f10' declared here}}
+}
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:6}:"__attribute__((format(printf, 1, 2)))"
+void f11(const char out[], ... /* args */) // #f11
+{
+ va_list args;
+ char ch[10] = "format";
+ vprintf(ch, args);
+ vsprintf(ch, out, args); // expected-warning {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f11'}}
+ // expected-note@#f11 {{'f11' declared here}}
+}
+
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:6}:"__attribute__((format(printf, 1, 0)))"
+void f12(char* out) // #f12
+{
+ va_list args;
+ const char *ch = "format";
+ vsprintf(ou...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/166738
More information about the cfe-commits
mailing list