Skip to content

rdar://166228589 [Sema] Suggest missing format attributes (#166738) #11971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
apple-fcloutier wants to merge 2 commits into
base: stable/21.x
Choose a base branch
from
Open

rdar://166228589 [Sema] Suggest missing format attributes (#166738) #11971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
618397a
[Sema] Suggest missing format attributes (#166738)
vvuksanovic Dec 5, 2025
f3c9366
Fix incompatibilities
apple-fcloutier Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,10 @@ Improvements to Clang's diagnostics
comparison operators when mixed with bitwise operators in enum value initializers.
This can be locally disabled by explicitly casting the initializer value.

- Clang now detects potential missing format and format_matches attributes on function,
Objective-C method and block declarations when calling format functions. It is part
of the format-nonliteral diagnostic (#GH60718)

Improvements to Clang's time-trace
----------------------------------

Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/DiagnosticGroups.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,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">;
Expand Down Expand Up @@ -1175,6 +1174,7 @@ def FormatY2K : DiagGroup<"format-y2k">;
def FormatPedantic : DiagGroup<"format-pedantic">;
def FormatSignedness : DiagGroup<"format-signedness">;
def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
def MissingFormatAttribute : DiagGroup<"missing-format-attribute">;

def FormatOverflowNonKprintf: DiagGroup<"format-overflow-non-kprintf">;
def FormatOverflow: DiagGroup<"format-overflow", [FormatOverflowNonKprintf]>;
Expand All @@ -1186,7 +1186,7 @@ def Format : DiagGroup<"format",
FormatSecurity, FormatY2K, FormatInvalidSpecifier,
FormatInsufficientArgs, FormatOverflow, FormatTruncation]>,
DiagCategory<"Format String Issue">;
def FormatNonLiteral : DiagGroup<"format-nonliteral">;
def FormatNonLiteral : DiagGroup<"format-nonliteral", [MissingFormatAttribute]>;
def Format2 : DiagGroup<"format=2",
[FormatNonLiteral, FormatSecurity, FormatY2K]>;

Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3490,6 +3490,10 @@ 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' attribute to the "
"declaration of %1">,
InGroup<MissingFormatAttribute>, DefaultIgnore;
def err_callback_attribute_no_callee : Error<
"'callback' attribute specifies no callback callee">;
def err_callback_attribute_invalid_callee : Error<
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3433,7 +3433,7 @@ class Sema final : public SemaBase {
llvm::SmallBitVector &CheckedVarArgs);
bool CheckFormatArguments(ArrayRef<const Expr *> Args,
FormatArgumentPassingKind FAPK,
const StringLiteral *ReferenceFormatString,
StringLiteral *ReferenceFormatString,
unsigned format_idx, unsigned firstDataArg,
FormatStringType Type, VariadicCallType CallType,
SourceLocation Loc, SourceRange range,
Expand Down
226 changes: 193 additions & 33 deletions clang/lib/Sema/SemaChecking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6249,13 +6249,16 @@ static const Expr *maybeConstEvalStringLiteral(ASTContext &Context,
// If this function returns false on the arguments to a function expecting a
// format string, we will usually need to emit a warning.
// True string literals are then checked by CheckFormatString.
static StringLiteralCheckType checkFormatStringExpr(
Sema &S, const StringLiteral *ReferenceFormatString, const Expr *E,
ArrayRef<const Expr *> Args, Sema::FormatArgumentPassingKind APK,
unsigned format_idx, unsigned firstDataArg, FormatStringType Type,
VariadicCallType CallType, bool InFunctionCall,
llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
llvm::APSInt Offset, bool IgnoreStringsWithoutSpecifiers = false) {
static StringLiteralCheckType
checkFormatStringExpr(Sema &S, const StringLiteral *ReferenceFormatString,
const Expr *E, ArrayRef<const Expr *> Args,
Sema::FormatArgumentPassingKind APK, unsigned format_idx,
unsigned firstDataArg, FormatStringType Type,
VariadicCallType CallType, bool InFunctionCall,
llvm::SmallBitVector &CheckedVarArgs,
UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset,
std::optional<unsigned> *CallerFormatParamIdx = nullptr,
bool IgnoreStringsWithoutSpecifiers = false) {
if (S.isConstantEvaluatedContext())
return SLCT_NotALiteral;
tryAgain:
Expand All @@ -6277,10 +6280,11 @@ static StringLiteralCheckType checkFormatStringExpr(
case Stmt::InitListExprClass:
// Handle expressions like {"foobar"}.
if (const clang::Expr *SLE = maybeConstEvalStringLiteral(S.Context, E)) {
return checkFormatStringExpr(
S, ReferenceFormatString, SLE, Args, APK, format_idx, firstDataArg,
Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs,
UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
return checkFormatStringExpr(S, ReferenceFormatString, SLE, Args, APK,
format_idx, firstDataArg, Type, CallType,
/*InFunctionCall*/ false, CheckedVarArgs,
UncoveredArg, Offset, CallerFormatParamIdx,
IgnoreStringsWithoutSpecifiers);
}
return SLCT_NotALiteral;
case Stmt::BinaryConditionalOperatorClass:
Expand Down Expand Up @@ -6312,10 +6316,11 @@ static StringLiteralCheckType checkFormatStringExpr(
if (!CheckLeft)
Left = SLCT_UncheckedLiteral;
else {
Left = checkFormatStringExpr(
S, ReferenceFormatString, C->getTrueExpr(), Args, APK, format_idx,
firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs,
UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
Left = checkFormatStringExpr(S, ReferenceFormatString, C->getTrueExpr(),
Args, APK, format_idx, firstDataArg, Type,
CallType, InFunctionCall, CheckedVarArgs,
UncoveredArg, Offset, CallerFormatParamIdx,
IgnoreStringsWithoutSpecifiers);
if (Left == SLCT_NotALiteral || !CheckRight) {
return Left;
}
Expand All @@ -6324,7 +6329,8 @@ static StringLiteralCheckType checkFormatStringExpr(
StringLiteralCheckType Right = checkFormatStringExpr(
S, ReferenceFormatString, C->getFalseExpr(), Args, APK, format_idx,
firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs,
UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
UncoveredArg, Offset, CallerFormatParamIdx,
IgnoreStringsWithoutSpecifiers);

return (CheckLeft && Left < Right) ? Left : Right;
}
Expand Down Expand Up @@ -6375,8 +6381,8 @@ static StringLiteralCheckType checkFormatStringExpr(
}
return checkFormatStringExpr(
S, ReferenceFormatString, Init, Args, APK, format_idx,
firstDataArg, Type, CallType,
/*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, Offset);
firstDataArg, Type, CallType, /*InFunctionCall=*/false,
CheckedVarArgs, UncoveredArg, Offset, CallerFormatParamIdx);
}
}

Expand Down Expand Up @@ -6428,6 +6434,8 @@ static StringLiteralCheckType checkFormatStringExpr(
// format arguments, in all cases.
//
if (const auto *PV = dyn_cast<ParmVarDecl>(VD)) {
if (CallerFormatParamIdx)
*CallerFormatParamIdx = PV->getFunctionScopeIndex();
if (const auto *D = dyn_cast<Decl>(PV->getDeclContext())) {
for (const auto *PVFormatMatches :
D->specific_attrs<FormatMatchesAttr>()) {
Expand All @@ -6453,7 +6461,7 @@ static StringLiteralCheckType checkFormatStringExpr(
S, ReferenceFormatString, PVFormatMatches->getFormatString(),
Args, APK, format_idx, firstDataArg, Type, CallType,
/*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg,
Offset, IgnoreStringsWithoutSpecifiers);
Offset, CallerFormatParamIdx, IgnoreStringsWithoutSpecifiers);
}
}

Expand Down Expand Up @@ -6508,7 +6516,7 @@ static StringLiteralCheckType checkFormatStringExpr(
StringLiteralCheckType Result = checkFormatStringExpr(
S, ReferenceFormatString, Arg, Args, APK, format_idx, firstDataArg,
Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg,
Offset, IgnoreStringsWithoutSpecifiers);
Offset, CallerFormatParamIdx, IgnoreStringsWithoutSpecifiers);
if (IsFirst) {
CommonResult = Result;
IsFirst = false;
Expand All @@ -6525,15 +6533,17 @@ static StringLiteralCheckType checkFormatStringExpr(
return checkFormatStringExpr(
S, ReferenceFormatString, Arg, Args, APK, format_idx,
firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs,
UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
UncoveredArg, Offset, CallerFormatParamIdx,
IgnoreStringsWithoutSpecifiers);
}
}
}
if (const Expr *SLE = maybeConstEvalStringLiteral(S.Context, E))
return checkFormatStringExpr(
S, ReferenceFormatString, SLE, Args, APK, format_idx, firstDataArg,
Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs,
UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
return checkFormatStringExpr(S, ReferenceFormatString, SLE, Args, APK,
format_idx, firstDataArg, Type, CallType,
/*InFunctionCall*/ false, CheckedVarArgs,
UncoveredArg, Offset, CallerFormatParamIdx,
IgnoreStringsWithoutSpecifiers);
return SLCT_NotALiteral;
}
case Stmt::ObjCMessageExprClass: {
Expand All @@ -6559,7 +6569,7 @@ static StringLiteralCheckType checkFormatStringExpr(
return checkFormatStringExpr(
S, ReferenceFormatString, Arg, Args, APK, format_idx, firstDataArg,
Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg,
Offset, IgnoreStringsWithoutSpecifiers);
Offset, CallerFormatParamIdx, IgnoreStringsWithoutSpecifiers);
}
}

Expand Down Expand Up @@ -6738,9 +6748,153 @@ bool Sema::CheckFormatString(const FormatMatchesAttr *Format,
return false;
}

static bool CheckMissingFormatAttribute(
Sema *S, ArrayRef<const Expr *> Args, Sema::FormatArgumentPassingKind APK,
StringLiteral *ReferenceFormatString, unsigned FormatIdx,
unsigned FirstDataArg, FormatStringType FormatType, unsigned CallerParamIdx,
SourceLocation Loc) {
if (S->getDiagnostics().isIgnored(diag::warn_missing_format_attribute, Loc))
return false;

DeclContext *DC = S->CurContext;
if (!isa<ObjCMethodDecl>(DC) && !isa<FunctionDecl>(DC) && !isa<BlockDecl>(DC))
return false;
Decl *Caller = cast<Decl>(DC)->getCanonicalDecl();

unsigned NumCallerParams = getFunctionOrMethodNumParams(Caller);

// Find the offset to convert between attribute and parameter indexes.

// Conflict note: this is equivalent to:
//
// hasImplicitObjectParameter(Caller) ? 2 : 1
//
// hasImplicitObjectParameter needed to be dropped because it was
// introduced by another change that is too large to take in.
unsigned CallerArgumentIndexOffset = 1;
if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(Caller)) {
if (MethodDecl->isImplicitObjectMemberFunction()) {
CallerArgumentIndexOffset = 2;
}
}

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() - FirstDataArg;
if (NumCalleeArgs == 0 || NumCallerParams < NumCalleeArgs) {
// There aren't enough arguments in the caller to pass to callee.
return false;
}
for (unsigned CalleeIdx = Args.size() - 1, CallerIdx = NumCallerParams - 1;
CalleeIdx >= FirstDataArg; --CalleeIdx, --CallerIdx) {
const auto *Arg =
dyn_cast<DeclRefExpr>(Args[CalleeIdx]->IgnoreParenCasts());
if (!Arg)
return false;
const auto *Param = dyn_cast<ParmVarDecl>(Arg->getDecl());
if (!Param || Param->getFunctionScopeIndex() != CallerIdx)
return false;
}
FirstArgumentIndex =
NumCallerParams + CallerArgumentIndexOffset - NumCalleeArgs;
break;
}
case Sema::FormatArgumentPassingKind::FAPK_VAList:
// Caller arguments are either variadic or a va_list.
FirstArgumentIndex = isFunctionOrMethodVariadic(Caller)
? (NumCallerParams + CallerArgumentIndexOffset)
: 0;
break;
case Sema::FormatArgumentPassingKind::FAPK_Elsewhere:
// The callee has a format_matches attribute. We will emit that instead.
if (!ReferenceFormatString)
return false;
break;
}

// Emit the diagnostic and fixit.
unsigned FormatStringIndex = CallerParamIdx + CallerArgumentIndexOffset;
StringRef FormatTypeName = S->GetFormatStringTypeName(FormatType);
NamedDecl *ND = dyn_cast<NamedDecl>(Caller);
do {
std::string Attr, Fixit;
llvm::raw_string_ostream AttrOS(Attr);
if (APK != Sema::FormatArgumentPassingKind::FAPK_Elsewhere) {
AttrOS << "format(" << FormatTypeName << ", " << FormatStringIndex << ", "
<< FirstArgumentIndex << ")";
} else {
AttrOS << "format_matches(" << FormatTypeName << ", " << FormatStringIndex
<< ", \"";
AttrOS.write_escaped(ReferenceFormatString->getString());
AttrOS << "\")";
}
AttrOS.flush();
auto DB = S->Diag(Loc, diag::warn_missing_format_attribute) << Attr;
if (ND)
DB << ND;
else
DB << "block";

// Blocks don't provide a correct end loc, so skip emitting a fixit.
if (isa<BlockDecl>(Caller))
break;

SourceLocation SL;
llvm::raw_string_ostream IS(Fixit);
// The attribute goes at the start of the declaration in C/C++ functions
// and methods, but after the declaration for Objective-C methods.
if (isa<ObjCMethodDecl>(Caller)) {
IS << ' ';
SL = Caller->getEndLoc();
}
const LangOptions &LO = S->getLangOpts();
if (LO.C23 || LO.CPlusPlus11)
IS << "[[gnu::" << Attr << "]]";
else if (LO.ObjC || LO.GNUMode)
IS << "__attribute__((" << Attr << "))";
else
break;
if (!isa<ObjCMethodDecl>(Caller)) {
IS << ' ';
SL = Caller->getBeginLoc();
}
IS.flush();

DB << FixItHint::CreateInsertion(SL, Fixit);
} while (false);

// Add implicit format or format_matches attribute.
if (APK != Sema::FormatArgumentPassingKind::FAPK_Elsewhere) {
Caller->addAttr(FormatAttr::CreateImplicit(
S->getASTContext(), &S->getASTContext().Idents.get(FormatTypeName),
FormatStringIndex, FirstArgumentIndex));
} else {
Caller->addAttr(FormatMatchesAttr::CreateImplicit(
S->getASTContext(), &S->getASTContext().Idents.get(FormatTypeName),
FormatStringIndex, ReferenceFormatString));
}

{
auto DB = S->Diag(Caller->getLocation(), diag::note_entity_declared_at);
if (ND)
DB << ND;
else
DB << "block";
}
return true;
}

bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
Sema::FormatArgumentPassingKind APK,
const StringLiteral *ReferenceFormatString,
StringLiteral *ReferenceFormatString,
unsigned format_idx, unsigned firstDataArg,
FormatStringType Type,
VariadicCallType CallType, SourceLocation Loc,
Expand Down Expand Up @@ -6770,11 +6924,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()) {
Expand All @@ -6787,11 +6942,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
Expand All @@ -6801,6 +6951,16 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
SourceMgr.isInSystemMacro(FormatLoc))
return false;

if (CallerParamIdx && CheckMissingFormatAttribute(
this, Args, APK, ReferenceFormatString, format_idx,
firstDataArg, Type, *CallerParamIdx, Loc))
return false;

// 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) {
Expand Down
Loading