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 e57e2fb

Browse filesBrowse files
budimirarandjelovichtecbudimirarandjelovichtec
committed
[clang] Catch missing format attributes
1 parent 2e9cbb6 commit e57e2fb
Copy full SHA for e57e2fb

File tree

Expand file treeCollapse file tree

9 files changed

+659
-3
lines changed
Filter options
Expand file treeCollapse file tree

9 files changed

+659
-3
lines changed

‎clang/docs/ReleaseNotes.rst

Copy file name to clipboardExpand all lines: clang/docs/ReleaseNotes.rst
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,9 @@ Improvements to Clang's diagnostics
254254
compilation speed with modules. This warning is disabled by default and it needs
255255
to be explicitly enabled or by ``-Weverything``.
256256

257+
- Clang now diagnoses missing format attributes for non-template functions and
258+
class/struct/union members. Fixes #GH60718
259+
257260
Improvements to Clang's time-trace
258261
----------------------------------
259262

‎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
@@ -529,7 +529,6 @@ def MainReturnType : DiagGroup<"main-return-type">;
529529
def MaxUnsignedZero : DiagGroup<"max-unsigned-zero">;
530530
def MissingBraces : DiagGroup<"missing-braces">;
531531
def MissingDeclarations: DiagGroup<"missing-declarations">;
532-
def : DiagGroup<"missing-format-attribute">;
533532
def MissingIncludeDirs : DiagGroup<"missing-include-dirs">;
534533
def MissingNoreturn : DiagGroup<"missing-noreturn">;
535534
def MultiChar : DiagGroup<"multichar">;

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

Copy file name to clipboardExpand all lines: clang/include/clang/Basic/DiagnosticSemaKinds.td
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,9 @@ def err_opencl_invalid_param : Error<
10401040
def err_opencl_invalid_return : Error<
10411041
"declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">;
10421042
def warn_enum_value_overflow : Warning<"overflow in enumeration value">;
1043+
def warn_missing_format_attribute : Warning<
1044+
"diagnostic behavior may be improved by adding the %0 format attribute to the declaration of %1">,
1045+
InGroup<DiagGroup<"missing-format-attribute">>, DefaultIgnore;
10431046
def warn_pragma_options_align_reset_failed : Warning<
10441047
"#pragma options align=reset failed: %0">,
10451048
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
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4619,6 +4619,10 @@ class Sema final : public SemaBase {
46194619

46204620
enum class RetainOwnershipKind { NS, CF, OS };
46214621

4622+
void DiagnoseMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl);
4623+
std::vector<FormatAttr *>
4624+
GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl);
4625+
46224626
UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
46234627
StringRef UuidAsWritten, MSGuidDecl *GuidDecl);
46244628

‎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
@@ -16004,6 +16004,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
1600416004
}
1600516005
}
1600616006

16007+
DiagnoseMissingFormatAttributes(Body, FD);
16008+
1600716009
// We might not have found a prototype because we didn't wish to warn on
1600816010
// the lack of a missing prototype. Try again without the checks for
1600916011
// 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
+213-2Lines changed: 213 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3519,7 +3519,7 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
35193519

35203520
// In C++ the implicit 'this' function parameter also counts, and they are
35213521
// counted from one.
3522-
bool HasImplicitThisParam = isInstanceMethod(D);
3522+
bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
35233523
unsigned NumArgs = getFunctionOrMethodNumParams(D) + HasImplicitThisParam;
35243524

35253525
IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
@@ -3632,7 +3632,7 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
36323632
return;
36333633
}
36343634

3635-
bool HasImplicitThisParam = isInstanceMethod(D);
3635+
bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
36363636
int32_t NumArgs = getFunctionOrMethodNumParams(D);
36373637

36383638
FunctionDecl *FD = D->getAsFunction();
@@ -5335,6 +5335,217 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
53355335
D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
53365336
}
53375337

5338+
// This function is called only if function call is not inside template body.
5339+
// TODO: Add call for function calls inside template body.
5340+
// Emit warnings if parent function misses format attributes.
5341+
void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,
5342+
const FunctionDecl *FDecl) {
5343+
assert(FDecl);
5344+
5345+
// If there are no function body, exit.
5346+
if (!Body)
5347+
return;
5348+
5349+
// Get missing format attributes
5350+
std::vector<FormatAttr *> MissingFormatAttributes =
5351+
GetMissingFormatAttributes(Body, FDecl);
5352+
if (MissingFormatAttributes.empty())
5353+
return;
5354+
5355+
// Check if there are more than one format type found. In that case do not
5356+
// emit diagnostic.
5357+
const clang::IdentifierInfo *AttrType = MissingFormatAttributes[0]->getType();
5358+
if (llvm::any_of(MissingFormatAttributes, [&](const FormatAttr *Attr) {
5359+
return AttrType != Attr->getType();
5360+
}))
5361+
return;
5362+
5363+
for (const FormatAttr *FA : MissingFormatAttributes) {
5364+
// If format index and first-to-check argument index are negative, it means
5365+
// that this attribute is only saved for multiple format types checking.
5366+
if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
5367+
continue;
5368+
5369+
// Emit diagnostic
5370+
SourceLocation Loc = FDecl->getLocation();
5371+
Diag(Loc, diag::warn_missing_format_attribute)
5372+
<< FA->getType() << FDecl
5373+
<< FixItHint::CreateInsertion(Loc,
5374+
(llvm::Twine("__attribute__((format(") +
5375+
FA->getType()->getName() + ", " +
5376+
llvm::Twine(FA->getFormatIdx()) + ", " +
5377+
llvm::Twine(FA->getFirstArg()) + ")))")
5378+
.str());
5379+
}
5380+
}
5381+
5382+
// Returns vector of format attributes. There are no two attributes with same
5383+
// arguments in returning vector. There can be attributes that effectivelly only
5384+
// store information about format type.
5385+
std::vector<FormatAttr *>
5386+
Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
5387+
unsigned int FunctionFormatArgumentIndexOffset =
5388+
checkIfMethodHasImplicitObjectParameter(FDecl) ? 2 : 1;
5389+
5390+
std::vector<FormatAttr *> MissingAttributes;
5391+
5392+
// Iterate over body statements.
5393+
for (auto *Child : Body->children()) {
5394+
// If child statement is compound statement, recursively get missing
5395+
// attributes.
5396+
if (dyn_cast_or_null<CompoundStmt>(Child)) {
5397+
std::vector<FormatAttr *> CompoundStmtMissingAttributes =
5398+
GetMissingFormatAttributes(Child, FDecl);
5399+
5400+
// If there are already missing attributes with same arguments, do not add
5401+
// duplicates.
5402+
for (FormatAttr *FA : CompoundStmtMissingAttributes) {
5403+
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
5404+
return FA->getType() == Attr->getType() &&
5405+
FA->getFormatIdx() == Attr->getFormatIdx() &&
5406+
FA->getFirstArg() == Attr->getFirstArg();
5407+
}))
5408+
MissingAttributes.push_back(FA);
5409+
}
5410+
5411+
continue;
5412+
}
5413+
5414+
ValueStmt *VS = dyn_cast_or_null<ValueStmt>(Child);
5415+
if (!VS)
5416+
continue;
5417+
Expr *TheExpr = VS->getExprStmt();
5418+
if (!TheExpr)
5419+
continue;
5420+
CallExpr *TheCall = dyn_cast_or_null<CallExpr>(TheExpr);
5421+
if (!TheCall)
5422+
continue;
5423+
const FunctionDecl *ChildFunction =
5424+
dyn_cast_or_null<FunctionDecl>(TheCall->getCalleeDecl());
5425+
if (!ChildFunction || !ChildFunction->hasAttr<FormatAttr>())
5426+
continue;
5427+
5428+
Expr **Args = TheCall->getArgs();
5429+
unsigned int NumArgs = TheCall->getNumArgs();
5430+
5431+
// If child expression is function, check if it is format function.
5432+
// If it is, check if parent function misses format attributes.
5433+
5434+
unsigned int ChildFunctionFormatArgumentIndexOffset =
5435+
checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;
5436+
5437+
// If child function is format function and format arguments are not
5438+
// relevant to emit diagnostic, save only information about format type
5439+
// (format index and first-to-check argument index are set to -1).
5440+
// Information about format type is later used to determine if there are
5441+
// more than one format type found.
5442+
5443+
// Check if function has format attribute with forwarded format string.
5444+
IdentifierInfo *AttrType;
5445+
const ParmVarDecl *FormatArg;
5446+
if (!llvm::any_of(ChildFunction->specific_attrs<FormatAttr>(),
5447+
[&](const FormatAttr *Attr) {
5448+
AttrType = Attr->getType();
5449+
5450+
int OffsetFormatIndex =
5451+
Attr->getFormatIdx() -
5452+
ChildFunctionFormatArgumentIndexOffset;
5453+
if (OffsetFormatIndex < 0 ||
5454+
(unsigned)OffsetFormatIndex >= NumArgs)
5455+
return false;
5456+
5457+
const auto *FormatArgExpr = dyn_cast<DeclRefExpr>(
5458+
Args[OffsetFormatIndex]->IgnoreParenCasts());
5459+
if (!FormatArgExpr)
5460+
return false;
5461+
5462+
FormatArg = dyn_cast_or_null<ParmVarDecl>(
5463+
FormatArgExpr->getReferencedDeclOfCallee());
5464+
if (!FormatArg)
5465+
return false;
5466+
5467+
return true;
5468+
})) {
5469+
MissingAttributes.push_back(
5470+
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
5471+
continue;
5472+
}
5473+
5474+
// Do not add in a vector format attributes whose type is different than
5475+
// parent function attribute type.
5476+
if (llvm::any_of(FDecl->specific_attrs<FormatAttr>(),
5477+
[&](const FormatAttr *FunctionAttr) {
5478+
return AttrType != FunctionAttr->getType();
5479+
}))
5480+
continue;
5481+
5482+
// Check if format string argument is parent function parameter.
5483+
int StringIndex = 0;
5484+
if (!llvm::any_of(FDecl->parameters(), [&](const ParmVarDecl *Param) {
5485+
if (Param != FormatArg)
5486+
return false;
5487+
5488+
StringIndex = Param->getFunctionScopeIndex() +
5489+
FunctionFormatArgumentIndexOffset;
5490+
5491+
return true;
5492+
})) {
5493+
MissingAttributes.push_back(
5494+
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
5495+
continue;
5496+
}
5497+
5498+
unsigned NumOfParentFunctionParams = FDecl->getNumParams();
5499+
5500+
// Compare parent and calling function format attribute arguments (archetype
5501+
// and format string).
5502+
if (llvm::any_of(
5503+
FDecl->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
5504+
if (Attr->getType() != AttrType)
5505+
return false;
5506+
int OffsetFormatIndex =
5507+
Attr->getFormatIdx() - FunctionFormatArgumentIndexOffset;
5508+
5509+
if (OffsetFormatIndex < 0 ||
5510+
(unsigned)OffsetFormatIndex >= NumOfParentFunctionParams)
5511+
return false;
5512+
5513+
if (FDecl->parameters()[OffsetFormatIndex] != FormatArg)
5514+
return false;
5515+
5516+
return true;
5517+
})) {
5518+
MissingAttributes.push_back(
5519+
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
5520+
continue;
5521+
}
5522+
5523+
// Get first argument index
5524+
int FirstToCheck = [&]() -> unsigned {
5525+
if (!FDecl->isVariadic())
5526+
return 0;
5527+
const auto *FirstToCheckArg = Args[NumArgs - 1]->IgnoreParenCasts();
5528+
if (FirstToCheckArg->getType().getCanonicalType() !=
5529+
Context.getBuiltinVaListType().getCanonicalType())
5530+
return 0;
5531+
return NumOfParentFunctionParams + FunctionFormatArgumentIndexOffset;
5532+
}();
5533+
5534+
// If there are already attributes which arguments matches arguments
5535+
// detected in this iteration, do not add new attribute as it would be
5536+
// duplicate.
5537+
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
5538+
return Attr->getType() == AttrType &&
5539+
Attr->getFormatIdx() == StringIndex &&
5540+
Attr->getFirstArg() == FirstToCheck;
5541+
}))
5542+
MissingAttributes.push_back(FormatAttr::CreateImplicit(
5543+
getASTContext(), AttrType, StringIndex, FirstToCheck));
5544+
}
5545+
5546+
return MissingAttributes;
5547+
}
5548+
53385549
//===----------------------------------------------------------------------===//
53395550
// Microsoft specific attribute handlers.
53405551
//===----------------------------------------------------------------------===//

0 commit comments

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