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 e4faa01

Browse filesBrowse files
budimirarandjelovichtecbudimirarandjelovichtec
committed
[clang] Catch missing format attributes
1 parent 0a7e5e3 commit e4faa01
Copy full SHA for e4faa01

File tree

Expand file treeCollapse file tree

10 files changed

+648
-4
lines changed
Filter options
Expand file treeCollapse file tree

10 files changed

+648
-4
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
@@ -511,6 +511,8 @@ Improvements to Clang's diagnostics
511511

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

514+
- Clang now diagnoses missing format attributes for non-template functions and class/struct/union members. (#GH60718)
515+
514516
Improvements to Clang's time-trace
515517
----------------------------------
516518

‎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
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4578,6 +4578,11 @@ class Sema final : public SemaBase {
45784578

45794579
enum class RetainOwnershipKind { NS, CF, OS };
45804580

4581+
void DetectMissingFormatAttributes(const FunctionDecl *Callee,
4582+
ArrayRef<const Expr *> Args,
4583+
SourceLocation Loc);
4584+
void EmitMissingFormatAttributesDiagnostic(const FunctionDecl *Caller);
4585+
45814586
UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
45824587
StringRef UuidAsWritten, MSGuidDecl *GuidDecl);
45834588

‎clang/lib/Sema/SemaChecking.cpp

Copy file name to clipboardExpand all lines: clang/lib/Sema/SemaChecking.cpp
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3410,8 +3410,10 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
34103410
}
34113411
}
34123412

3413-
if (FD)
3413+
if (FD) {
34143414
diagnoseArgDependentDiagnoseIfAttrs(FD, ThisArg, Args, Loc);
3415+
DetectMissingFormatAttributes(FD, Args, Loc);
3416+
}
34153417
}
34163418

34173419
void Sema::CheckConstrainedAuto(const AutoType *AutoT, SourceLocation Loc) {

‎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
@@ -16080,6 +16080,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
1608016080
}
1608116081
}
1608216082

16083+
EmitMissingFormatAttributesDiagnostic(FD);
16084+
1608316085
// We might not have found a prototype because we didn't wish to warn on
1608416086
// the lack of a missing prototype. Try again without the checks for
1608516087
// 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
+177-2Lines changed: 177 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,181 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
54305430
D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
54315431
}
54325432

5433+
// Diagnosing missing format attributes is implemented in two steps:
5434+
// 1. Detect missing format attributes while checking function calls.
5435+
// 2. Emit diagnostic in part that processes function body.
5436+
// For this purpose it is created vector that stores information about format
5437+
// attributes. There are no two format attributes with same arguments in a
5438+
// vector. Vector could contains attributes that only store information about
5439+
// format type (format string and first to check argument are set to -1).
5440+
namespace {
5441+
std::vector<FormatAttr *> MissingAttributes;
5442+
} // end anonymous namespace
5443+
5444+
// This function is called only if function call is not inside template body.
5445+
// TODO: Add call for function calls inside template body.
5446+
// Detects and stores missing format attributes in a vector.
5447+
void Sema::DetectMissingFormatAttributes(const FunctionDecl *Callee,
5448+
ArrayRef<const Expr *> Args,
5449+
SourceLocation Loc) {
5450+
assert(Callee);
5451+
5452+
// If there is no caller, exit.
5453+
const FunctionDecl *Caller = getCurFunctionDecl();
5454+
if (!getCurFunctionDecl())
5455+
return;
5456+
5457+
// Check if callee function is a format function.
5458+
// If it is, check if caller function misses format attributes.
5459+
5460+
if (!Callee->hasAttr<FormatAttr>())
5461+
return;
5462+
5463+
// va_list is not intended to be passed to variadic function.
5464+
if (Callee->isVariadic())
5465+
return;
5466+
5467+
// Check if va_list is passed to callee function.
5468+
// If va_list is not passed, return.
5469+
bool hasVaList = false;
5470+
for (const auto *Param : Callee->parameters()) {
5471+
if (Param->getOriginalType().getCanonicalType() ==
5472+
getASTContext().getBuiltinVaListType().getCanonicalType()) {
5473+
hasVaList = true;
5474+
break;
5475+
}
5476+
}
5477+
if (!hasVaList)
5478+
return;
5479+
5480+
unsigned int FormatArgumentIndexOffset =
5481+
checkIfMethodHasImplicitObjectParameter(Callee) ? 2 : 1;
5482+
5483+
// If callee function is format function and format arguments are not
5484+
// relevant to emit diagnostic, save only information about format type
5485+
// (format index and first-to-check argument index are set to -1).
5486+
// Information about format type is later used to determine if there are
5487+
// more than one format type found.
5488+
5489+
unsigned int NumArgs = Args.size();
5490+
// Check if function has format attribute with forwarded format string.
5491+
IdentifierInfo *AttrType;
5492+
const ParmVarDecl *FormatStringArg;
5493+
if (!llvm::any_of(
5494+
Callee->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
5495+
AttrType = Attr->getType();
5496+
5497+
int OffsetFormatIndex =
5498+
Attr->getFormatIdx() - FormatArgumentIndexOffset;
5499+
if (OffsetFormatIndex < 0 || (unsigned)OffsetFormatIndex >= NumArgs)
5500+
return false;
5501+
5502+
if (const auto *FormatArgExpr = dyn_cast<DeclRefExpr>(
5503+
Args[OffsetFormatIndex]->IgnoreParenCasts()))
5504+
if (FormatStringArg = dyn_cast_or_null<ParmVarDecl>(
5505+
FormatArgExpr->getReferencedDeclOfCallee()))
5506+
return true;
5507+
return false;
5508+
})) {
5509+
MissingAttributes.push_back(
5510+
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
5511+
return;
5512+
}
5513+
5514+
unsigned ArgumentIndexOffset =
5515+
checkIfMethodHasImplicitObjectParameter(Caller) ? 2 : 1;
5516+
5517+
unsigned NumOfCallerFunctionParams = Caller->getNumParams();
5518+
5519+
// Compare caller and callee function format attribute arguments (archetype
5520+
// and format string). If they don't match, caller misses format attribute.
5521+
if (llvm::any_of(
5522+
Caller->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
5523+
if (Attr->getType() != AttrType)
5524+
return false;
5525+
int OffsetFormatIndex = Attr->getFormatIdx() - ArgumentIndexOffset;
5526+
5527+
if (OffsetFormatIndex < 0 ||
5528+
(unsigned)OffsetFormatIndex >= NumOfCallerFunctionParams)
5529+
return false;
5530+
5531+
if (Caller->parameters()[OffsetFormatIndex] != FormatStringArg)
5532+
return false;
5533+
5534+
return true;
5535+
})) {
5536+
MissingAttributes.push_back(
5537+
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
5538+
return;
5539+
}
5540+
5541+
// Get format string index
5542+
int FormatStringIndex =
5543+
FormatStringArg->getFunctionScopeIndex() + ArgumentIndexOffset;
5544+
5545+
// Get first argument index
5546+
int FirstToCheck = Caller->isVariadic()
5547+
? (NumOfCallerFunctionParams + ArgumentIndexOffset)
5548+
: 0;
5549+
5550+
// Do not add duplicate in a vector of missing format attributes.
5551+
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
5552+
return Attr->getType() == AttrType &&
5553+
Attr->getFormatIdx() == FormatStringIndex &&
5554+
Attr->getFirstArg() == FirstToCheck;
5555+
}))
5556+
MissingAttributes.push_back(FormatAttr::CreateImplicit(
5557+
getASTContext(), AttrType, FormatStringIndex, FirstToCheck, Loc));
5558+
}
5559+
5560+
// This function is called only if caller function is not template.
5561+
// TODO: Add call for template functions.
5562+
// Emits missing format attribute diagnostics.
5563+
void Sema::EmitMissingFormatAttributesDiagnostic(const FunctionDecl *Caller) {
5564+
const clang::IdentifierInfo *AttrType = MissingAttributes[0]->getType();
5565+
for (unsigned i = 1; i < MissingAttributes.size(); ++i) {
5566+
if (AttrType != MissingAttributes[i]->getType()) {
5567+
// Clear vector of missing attributes because it could be used in
5568+
// diagnosing missing format attributes in another caller.
5569+
MissingAttributes.clear();
5570+
return;
5571+
}
5572+
}
5573+
5574+
for (const FormatAttr *FA : MissingAttributes) {
5575+
// If format index and first-to-check argument index are negative, it means
5576+
// that this attribute is only saved for multiple format types checking.
5577+
if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
5578+
continue;
5579+
5580+
// If caller function has format attributes and callee format attribute type
5581+
// mismatches caller attribute type, do not emit diagnostic.
5582+
if (Caller->hasAttr<FormatAttr>() &&
5583+
!llvm::any_of(Caller->specific_attrs<FormatAttr>(),
5584+
[FA](const FormatAttr *FunctionAttr) {
5585+
return FA->getType() == FunctionAttr->getType();
5586+
}))
5587+
continue;
5588+
5589+
// Emit diagnostic
5590+
SourceLocation Loc = Caller->getFirstDecl()->getLocation();
5591+
Diag(Loc, diag::warn_missing_format_attribute)
5592+
<< FA->getType() << Caller
5593+
<< FixItHint::CreateInsertion(Loc,
5594+
(llvm::Twine("__attribute__((format(") +
5595+
FA->getType()->getName() + ", " +
5596+
llvm::Twine(FA->getFormatIdx()) + ", " +
5597+
llvm::Twine(FA->getFirstArg()) + ")))")
5598+
.str());
5599+
Diag(FA->getLocation(), diag::note_format_function) << FA->getType();
5600+
}
5601+
5602+
// Clear vector of missing attributes after emitting diagnostics for caller
5603+
// function because it could be used in diagnosing missing format attributes
5604+
// in another caller.
5605+
MissingAttributes.clear();
5606+
}
5607+
54335608
//===----------------------------------------------------------------------===//
54345609
// Microsoft specific attribute handlers.
54355610
//===----------------------------------------------------------------------===//

0 commit comments

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