Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 97b7071

Browse filesBrowse files
budimirarandjelovichtecbudimirarandjelovichtec
committed
[clang] Catch missing format attributes
1 parent 7585e2f commit 97b7071
Copy full SHA for 97b7071

File tree

9 files changed

+654
-3
lines changed
Filter options

9 files changed

+654
-3
lines changed

‎clang/docs/ReleaseNotes.rst

Copy file name to clipboardExpand all lines: clang/docs/ReleaseNotes.rst
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,8 @@ Improvements to Clang's diagnostics
464464

465465
- Clang now diagnoses ``[[deprecated]]`` attribute usage on local variables (#GH90073).
466466

467+
- Clang now diagnoses missing format attributes for non-template functions and class/struct/union members. (#GH60718)
468+
467469
Improvements to Clang's time-trace
468470
----------------------------------
469471

‎clang/include/clang/Basic/DiagnosticGroups.td

Copy file name to clipboardExpand all lines: clang/include/clang/Basic/DiagnosticGroups.td
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,6 @@ def MainReturnType : DiagGroup<"main-return-type">;
530530
def MaxUnsignedZero : DiagGroup<"max-unsigned-zero">;
531531
def MissingBraces : DiagGroup<"missing-braces">;
532532
def MissingDeclarations: DiagGroup<"missing-declarations">;
533-
def : DiagGroup<"missing-format-attribute">;
534533
def MissingIncludeDirs : DiagGroup<"missing-include-dirs">;
535534
def MissingNoreturn : DiagGroup<"missing-noreturn">;
536535
def MultiChar : DiagGroup<"multichar">;

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Copy file name to clipboardExpand all lines: clang/include/clang/Basic/DiagnosticSemaKinds.td
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,6 +1055,10 @@ def err_opencl_invalid_param : Error<
10551055
"declaring function parameter of type %0 is not allowed%select{; did you forget * ?|}1">;
10561056
def err_opencl_invalid_return : Error<
10571057
"declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">;
1058+
def warn_missing_format_attribute : Warning<
1059+
"diagnostic behavior may be improved by adding the %0 format attribute to the declaration of %1">,
1060+
InGroup<DiagGroup<"missing-format-attribute">>, DefaultIgnore;
1061+
def note_format_function : Note<"%0 format function">;
10581062
def warn_pragma_options_align_reset_failed : Warning<
10591063
"#pragma options align=reset failed: %0">,
10601064
InGroup<IgnoredPragmas>;

‎clang/include/clang/Sema/Attr.h

Copy file name to clipboardExpand all lines: clang/include/clang/Sema/Attr.h
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,13 @@ inline bool isInstanceMethod(const Decl *D) {
123123
return false;
124124
}
125125

126+
inline bool checkIfMethodHasImplicitObjectParameter(const Decl *D) {
127+
if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(D))
128+
return MethodDecl->isInstance() &&
129+
!MethodDecl->hasCXXExplicitFunctionObjectParameter();
130+
return false;
131+
}
132+
126133
/// Diagnose mutually exclusive attributes when present on a given
127134
/// declaration. Returns true if diagnosed.
128135
template <typename AttrTy>

‎clang/include/clang/Sema/Sema.h

Copy file name to clipboardExpand all lines: clang/include/clang/Sema/Sema.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4576,6 +4576,8 @@ class Sema final : public SemaBase {
45764576

45774577
enum class RetainOwnershipKind { NS, CF, OS };
45784578

4579+
void DiagnoseMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl);
4580+
45794581
UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
45804582
StringRef UuidAsWritten, MSGuidDecl *GuidDecl);
45814583

‎clang/lib/Sema/SemaDecl.cpp

Copy file name to clipboardExpand all lines: clang/lib/Sema/SemaDecl.cpp
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16062,6 +16062,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
1606216062
}
1606316063
}
1606416064

16065+
DiagnoseMissingFormatAttributes(Body, FD);
16066+
1606516067
// We might not have found a prototype because we didn't wish to warn on
1606616068
// the lack of a missing prototype. Try again without the checks for
1606716069
// whether we want to warn on the missing prototype.

‎clang/lib/Sema/SemaDeclAttr.cpp

Copy file name to clipboardExpand all lines: clang/lib/Sema/SemaDeclAttr.cpp
+220-2Lines changed: 220 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3624,7 +3624,7 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
36243624

36253625
// In C++ the implicit 'this' function parameter also counts, and they are
36263626
// counted from one.
3627-
bool HasImplicitThisParam = isInstanceMethod(D);
3627+
bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
36283628
unsigned NumArgs = getFunctionOrMethodNumParams(D) + HasImplicitThisParam;
36293629

36303630
IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
@@ -3737,7 +3737,7 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
37373737
return;
37383738
}
37393739

3740-
bool HasImplicitThisParam = isInstanceMethod(D);
3740+
bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
37413741
int32_t NumArgs = getFunctionOrMethodNumParams(D);
37423742

37433743
FunctionDecl *FD = D->getAsFunction();
@@ -5430,6 +5430,224 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
54305430
D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
54315431
}
54325432

5433+
// Returns vector of format attributes. There are no two attributes with same
5434+
// arguments in returning vector. There can be attributes that effectively only
5435+
// store information about format type.
5436+
static std::vector<FormatAttr *>
5437+
GetMissingFormatAttributes(Sema &S, Stmt *Body, const FunctionDecl *FDecl) {
5438+
unsigned int ArgumentIndexOffset =
5439+
checkIfMethodHasImplicitObjectParameter(FDecl) ? 2 : 1;
5440+
5441+
std::vector<FormatAttr *> MissingAttributes;
5442+
5443+
// Iterate over body statements.
5444+
for (auto *Child : Body->children()) {
5445+
// If child statement is compound statement, recursively get missing
5446+
// attributes.
5447+
if (dyn_cast_or_null<CompoundStmt>(Child)) {
5448+
std::vector<FormatAttr *> CompoundStmtMissingAttributes =
5449+
GetMissingFormatAttributes(S, Child, FDecl);
5450+
5451+
// If there are already missing attributes with same arguments, do not add
5452+
// duplicates.
5453+
for (FormatAttr *FA : CompoundStmtMissingAttributes) {
5454+
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
5455+
return FA->getType() == Attr->getType() &&
5456+
FA->getFormatIdx() == Attr->getFormatIdx() &&
5457+
FA->getFirstArg() == Attr->getFirstArg();
5458+
}))
5459+
MissingAttributes.push_back(FA);
5460+
}
5461+
5462+
continue;
5463+
}
5464+
5465+
ValueStmt *VS = dyn_cast<ValueStmt>(Child);
5466+
if (!VS)
5467+
continue;
5468+
CallExpr *TheCall = dyn_cast_or_null<CallExpr>(VS->getExprStmt());
5469+
if (!TheCall)
5470+
continue;
5471+
5472+
const FunctionDecl *CalleeFunction =
5473+
dyn_cast_or_null<FunctionDecl>(TheCall->getCalleeDecl());
5474+
if (!CalleeFunction || !CalleeFunction->hasAttr<FormatAttr>())
5475+
continue;
5476+
5477+
// va_list is not intended to be passed to variadic function.
5478+
if (CalleeFunction->isVariadic())
5479+
continue;
5480+
5481+
Expr **Args = TheCall->getArgs();
5482+
unsigned int NumArgs = TheCall->getNumArgs();
5483+
5484+
// Check if va_list is passed to callee function.
5485+
// If va_list is not passed, continue to the next iteration.
5486+
bool hasVaList = false;
5487+
for (const auto *Param : CalleeFunction->parameters()) {
5488+
if (Param->getOriginalType().getCanonicalType() ==
5489+
S.getASTContext().getBuiltinVaListType().getCanonicalType()) {
5490+
hasVaList = true;
5491+
break;
5492+
}
5493+
}
5494+
if (!hasVaList)
5495+
continue;
5496+
5497+
// If callee expression is function, check if it is format function.
5498+
// If it is, check if caller function misses format attributes.
5499+
5500+
unsigned int CalleeFunctionFormatArgumentIndexOffset =
5501+
checkIfMethodHasImplicitObjectParameter(CalleeFunction) ? 2 : 1;
5502+
5503+
// If callee function is format function and format arguments are not
5504+
// relevant to emit diagnostic, save only information about format type
5505+
// (format index and first-to-check argument index are set to -1).
5506+
// Information about format type is later used to determine if there are
5507+
// more than one format type found.
5508+
5509+
// Check if function has format attribute with forwarded format string.
5510+
IdentifierInfo *AttrType;
5511+
const ParmVarDecl *FormatArg;
5512+
if (!llvm::any_of(CalleeFunction->specific_attrs<FormatAttr>(),
5513+
[&](const FormatAttr *Attr) {
5514+
AttrType = Attr->getType();
5515+
5516+
int OffsetFormatIndex =
5517+
Attr->getFormatIdx() -
5518+
CalleeFunctionFormatArgumentIndexOffset;
5519+
if (OffsetFormatIndex < 0 ||
5520+
(unsigned)OffsetFormatIndex >= NumArgs)
5521+
return false;
5522+
5523+
if (const auto *FormatArgExpr = dyn_cast<DeclRefExpr>(
5524+
Args[OffsetFormatIndex]->IgnoreParenCasts()))
5525+
if (FormatArg = dyn_cast_or_null<ParmVarDecl>(
5526+
FormatArgExpr->getReferencedDeclOfCallee()))
5527+
return true;
5528+
return false;
5529+
})) {
5530+
MissingAttributes.push_back(
5531+
FormatAttr::CreateImplicit(S.getASTContext(), AttrType, -1, -1));
5532+
continue;
5533+
}
5534+
5535+
// If format string argument is caller function parameter, get string index.
5536+
// Otherwise, save only attribute type and go to next iteration.
5537+
int StringIndex = [=]() -> int {
5538+
for (const ParmVarDecl *Param : FDecl->parameters()) {
5539+
if (Param == FormatArg)
5540+
return Param->getFunctionScopeIndex() + ArgumentIndexOffset;
5541+
}
5542+
return 0;
5543+
}();
5544+
5545+
if (StringIndex == 0) {
5546+
MissingAttributes.push_back(
5547+
FormatAttr::CreateImplicit(S.getASTContext(), AttrType, -1, -1));
5548+
continue;
5549+
}
5550+
5551+
unsigned NumOfCallerFunctionParams = FDecl->getNumParams();
5552+
5553+
// Compare caller and callee function format attribute arguments (archetype
5554+
// and format string).
5555+
if (llvm::any_of(
5556+
FDecl->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
5557+
if (Attr->getType() != AttrType)
5558+
return false;
5559+
int OffsetFormatIndex =
5560+
Attr->getFormatIdx() - ArgumentIndexOffset;
5561+
5562+
if (OffsetFormatIndex < 0 ||
5563+
(unsigned)OffsetFormatIndex >= NumOfCallerFunctionParams)
5564+
return false;
5565+
5566+
if (FDecl->parameters()[OffsetFormatIndex] != FormatArg)
5567+
return false;
5568+
5569+
return true;
5570+
})) {
5571+
MissingAttributes.push_back(
5572+
FormatAttr::CreateImplicit(S.getASTContext(), AttrType, -1, -1));
5573+
continue;
5574+
}
5575+
5576+
// Get first argument index
5577+
int FirstToCheck = FDecl->isVariadic()
5578+
? (NumOfCallerFunctionParams + ArgumentIndexOffset)
5579+
: 0;
5580+
5581+
// If there are already attributes which arguments matches arguments
5582+
// detected in this iteration, do not add new attribute as it would be
5583+
// duplicate.
5584+
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
5585+
return Attr->getType() == AttrType &&
5586+
Attr->getFormatIdx() == StringIndex &&
5587+
Attr->getFirstArg() == FirstToCheck;
5588+
}))
5589+
MissingAttributes.push_back(
5590+
FormatAttr::CreateImplicit(S.getASTContext(), AttrType, StringIndex,
5591+
FirstToCheck, TheCall->getBeginLoc()));
5592+
}
5593+
5594+
return MissingAttributes;
5595+
}
5596+
5597+
// This function is called only if function call is not inside template body.
5598+
// TODO: Add call for function calls inside template body.
5599+
// Emit warnings if caller function misses format attributes.
5600+
void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,
5601+
const FunctionDecl *FDecl) {
5602+
assert(FDecl);
5603+
5604+
// If there are no function body, exit.
5605+
if (!Body)
5606+
return;
5607+
5608+
// Get missing format attributes
5609+
std::vector<FormatAttr *> MissingFormatAttributes =
5610+
GetMissingFormatAttributes(*this, Body, FDecl);
5611+
if (MissingFormatAttributes.empty())
5612+
return;
5613+
5614+
// Check if there are more than one format type found. In that case do not
5615+
// emit diagnostic.
5616+
const clang::IdentifierInfo *AttrType = MissingFormatAttributes[0]->getType();
5617+
for (unsigned i = 1; i < MissingFormatAttributes.size(); ++i) {
5618+
if (AttrType != MissingFormatAttributes[i]->getType())
5619+
return;
5620+
}
5621+
5622+
for (const FormatAttr *FA : MissingFormatAttributes) {
5623+
// If format index and first-to-check argument index are negative, it means
5624+
// that this attribute is only saved for multiple format types checking.
5625+
if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
5626+
continue;
5627+
5628+
// If caller function has format attributes and callee format attribute type
5629+
// mismatches caller attribute type, do not emit diagnostic.
5630+
if (FDecl->hasAttr<FormatAttr>() &&
5631+
!llvm::any_of(FDecl->specific_attrs<FormatAttr>(),
5632+
[FA](const FormatAttr *FunctionAttr) {
5633+
return FA->getType() == FunctionAttr->getType();
5634+
}))
5635+
continue;
5636+
5637+
// Emit diagnostic
5638+
SourceLocation Loc = FDecl->getFirstDecl()->getLocation();
5639+
Diag(Loc, diag::warn_missing_format_attribute)
5640+
<< FA->getType() << FDecl
5641+
<< FixItHint::CreateInsertion(Loc,
5642+
(llvm::Twine("__attribute__((format(") +
5643+
FA->getType()->getName() + ", " +
5644+
llvm::Twine(FA->getFormatIdx()) + ", " +
5645+
llvm::Twine(FA->getFirstArg()) + ")))")
5646+
.str());
5647+
Diag(FA->getLocation(), diag::note_format_function) << FA->getType();
5648+
}
5649+
}
5650+
54335651
//===----------------------------------------------------------------------===//
54345652
// Microsoft specific attribute handlers.
54355653
//===----------------------------------------------------------------------===//

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.