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 e5f4b28

Browse filesBrowse files
budimirarandjelovichtecbudimirarandjelovichtec
committed
[clang] Catch missing format attributes
1 parent d595d5a commit e5f4b28
Copy full SHA for e5f4b28

File tree

Expand file treeCollapse file tree

10 files changed

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

10 files changed

+649
-4
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
@@ -138,6 +138,9 @@ Improvements to Clang's diagnostics
138138
- A statement attribute applied to a ``case`` label no longer suppresses
139139
'bypassing variable initialization' diagnostics (#84072).
140140

141+
- Clang now diagnoses missing format attributes for non-template functions
142+
and class/struct/union members. (#GH60718)
143+
141144
Improvements to Clang's time-trace
142145
----------------------------------
143146

‎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
@@ -534,7 +534,6 @@ def MainReturnType : DiagGroup<"main-return-type">;
534534
def MaxUnsignedZero : DiagGroup<"max-unsigned-zero">;
535535
def MissingBraces : DiagGroup<"missing-braces">;
536536
def MissingDeclarations: DiagGroup<"missing-declarations">;
537-
def : DiagGroup<"missing-format-attribute">;
538537
def MissingIncludeDirs : DiagGroup<"missing-include-dirs">;
539538
def MissingNoreturn : DiagGroup<"missing-noreturn">;
540539
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
@@ -1064,6 +1064,10 @@ def err_opencl_invalid_param : Error<
10641064
"declaring function parameter of type %0 is not allowed%select{; did you forget * ?|}1">;
10651065
def err_opencl_invalid_return : Error<
10661066
"declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">;
1067+
def warn_missing_format_attribute : Warning<
1068+
"diagnostic behavior may be improved by adding the %0 format attribute to the declaration of %1">,
1069+
InGroup<DiagGroup<"missing-format-attribute">>, DefaultIgnore;
1070+
def note_format_function : Note<"%0 format function">;
10671071
def warn_pragma_options_align_reset_failed : Warning<
10681072
"#pragma options align=reset failed: %0">,
10691073
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
@@ -4620,6 +4620,11 @@ class Sema final : public SemaBase {
46204620

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

4623+
void DetectMissingFormatAttributes(const FunctionDecl *Callee,
4624+
ArrayRef<const Expr *> Args,
4625+
SourceLocation Loc);
4626+
void EmitMissingFormatAttributesDiagnostic(const FunctionDecl *Caller);
4627+
46234628
UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
46244629
StringRef UuidAsWritten, MSGuidDecl *GuidDecl);
46254630

‎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
@@ -3456,8 +3456,10 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
34563456
}
34573457
}
34583458

3459-
if (FD)
3459+
if (FD) {
34603460
diagnoseArgDependentDiagnoseIfAttrs(FD, ThisArg, Args, Loc);
3461+
DetectMissingFormatAttributes(FD, Args, Loc);
3462+
}
34613463
}
34623464

34633465
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
@@ -16278,6 +16278,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
1627816278
}
1627916279
}
1628016280

16281+
EmitMissingFormatAttributesDiagnostic(FD);
16282+
1628116283
// We might not have found a prototype because we didn't wish to warn on
1628216284
// the lack of a missing prototype. Try again without the checks for
1628316285
// 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
@@ -3677,7 +3677,7 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
36773677

36783678
// In C++ the implicit 'this' function parameter also counts, and they are
36793679
// counted from one.
3680-
bool HasImplicitThisParam = isInstanceMethod(D);
3680+
bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
36813681
unsigned NumArgs = getFunctionOrMethodNumParams(D) + HasImplicitThisParam;
36823682

36833683
IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
@@ -3790,7 +3790,7 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
37903790
return;
37913791
}
37923792

3793-
bool HasImplicitThisParam = isInstanceMethod(D);
3793+
bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
37943794
int32_t NumArgs = getFunctionOrMethodNumParams(D);
37953795

37963796
FunctionDecl *FD = D->getAsFunction();
@@ -5596,6 +5596,181 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
55965596
D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
55975597
}
55985598

5599+
// Diagnosing missing format attributes is implemented in two steps:
5600+
// 1. Detect missing format attributes while checking function calls.
5601+
// 2. Emit diagnostic in part that processes function body.
5602+
// For this purpose it is created vector that stores information about format
5603+
// attributes. There are no two format attributes with same arguments in a
5604+
// vector. Vector could contains attributes that only store information about
5605+
// format type (format string and first to check argument are set to -1).
5606+
namespace {
5607+
std::vector<FormatAttr *> MissingAttributes;
5608+
} // end anonymous namespace
5609+
5610+
// This function is called only if function call is not inside template body.
5611+
// TODO: Add call for function calls inside template body.
5612+
// Detects and stores missing format attributes in a vector.
5613+
void Sema::DetectMissingFormatAttributes(const FunctionDecl *Callee,
5614+
ArrayRef<const Expr *> Args,
5615+
SourceLocation Loc) {
5616+
assert(Callee);
5617+
5618+
// If there is no caller, exit.
5619+
const FunctionDecl *Caller = getCurFunctionDecl();
5620+
if (!getCurFunctionDecl())
5621+
return;
5622+
5623+
// Check if callee function is a format function.
5624+
// If it is, check if caller function misses format attributes.
5625+
5626+
if (!Callee->hasAttr<FormatAttr>())
5627+
return;
5628+
5629+
// va_list is not intended to be passed to variadic function.
5630+
if (Callee->isVariadic())
5631+
return;
5632+
5633+
// Check if va_list is passed to callee function.
5634+
// If va_list is not passed, return.
5635+
bool hasVaList = false;
5636+
for (const auto *Param : Callee->parameters()) {
5637+
if (Param->getOriginalType().getCanonicalType() ==
5638+
getASTContext().getBuiltinVaListType().getCanonicalType()) {
5639+
hasVaList = true;
5640+
break;
5641+
}
5642+
}
5643+
if (!hasVaList)
5644+
return;
5645+
5646+
unsigned int FormatArgumentIndexOffset =
5647+
checkIfMethodHasImplicitObjectParameter(Callee) ? 2 : 1;
5648+
5649+
// If callee function is format function and format arguments are not
5650+
// relevant to emit diagnostic, save only information about format type
5651+
// (format index and first-to-check argument index are set to -1).
5652+
// Information about format type is later used to determine if there are
5653+
// more than one format type found.
5654+
5655+
unsigned int NumArgs = Args.size();
5656+
// Check if function has format attribute with forwarded format string.
5657+
IdentifierInfo *AttrType;
5658+
const ParmVarDecl *FormatStringArg;
5659+
if (!llvm::any_of(
5660+
Callee->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
5661+
AttrType = Attr->getType();
5662+
5663+
int OffsetFormatIndex =
5664+
Attr->getFormatIdx() - FormatArgumentIndexOffset;
5665+
if (OffsetFormatIndex < 0 || (unsigned)OffsetFormatIndex >= NumArgs)
5666+
return false;
5667+
5668+
if (const auto *FormatArgExpr = dyn_cast<DeclRefExpr>(
5669+
Args[OffsetFormatIndex]->IgnoreParenCasts()))
5670+
if (FormatStringArg = dyn_cast_or_null<ParmVarDecl>(
5671+
FormatArgExpr->getReferencedDeclOfCallee()))
5672+
return true;
5673+
return false;
5674+
})) {
5675+
MissingAttributes.push_back(
5676+
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
5677+
return;
5678+
}
5679+
5680+
unsigned ArgumentIndexOffset =
5681+
checkIfMethodHasImplicitObjectParameter(Caller) ? 2 : 1;
5682+
5683+
unsigned NumOfCallerFunctionParams = Caller->getNumParams();
5684+
5685+
// Compare caller and callee function format attribute arguments (archetype
5686+
// and format string). If they don't match, caller misses format attribute.
5687+
if (llvm::any_of(
5688+
Caller->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
5689+
if (Attr->getType() != AttrType)
5690+
return false;
5691+
int OffsetFormatIndex = Attr->getFormatIdx() - ArgumentIndexOffset;
5692+
5693+
if (OffsetFormatIndex < 0 ||
5694+
(unsigned)OffsetFormatIndex >= NumOfCallerFunctionParams)
5695+
return false;
5696+
5697+
if (Caller->parameters()[OffsetFormatIndex] != FormatStringArg)
5698+
return false;
5699+
5700+
return true;
5701+
})) {
5702+
MissingAttributes.push_back(
5703+
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
5704+
return;
5705+
}
5706+
5707+
// Get format string index
5708+
int FormatStringIndex =
5709+
FormatStringArg->getFunctionScopeIndex() + ArgumentIndexOffset;
5710+
5711+
// Get first argument index
5712+
int FirstToCheck = Caller->isVariadic()
5713+
? (NumOfCallerFunctionParams + ArgumentIndexOffset)
5714+
: 0;
5715+
5716+
// Do not add duplicate in a vector of missing format attributes.
5717+
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
5718+
return Attr->getType() == AttrType &&
5719+
Attr->getFormatIdx() == FormatStringIndex &&
5720+
Attr->getFirstArg() == FirstToCheck;
5721+
}))
5722+
MissingAttributes.push_back(FormatAttr::CreateImplicit(
5723+
getASTContext(), AttrType, FormatStringIndex, FirstToCheck, Loc));
5724+
}
5725+
5726+
// This function is called only if caller function is not template.
5727+
// TODO: Add call for template functions.
5728+
// Emits missing format attribute diagnostics.
5729+
void Sema::EmitMissingFormatAttributesDiagnostic(const FunctionDecl *Caller) {
5730+
const clang::IdentifierInfo *AttrType = MissingAttributes[0]->getType();
5731+
for (unsigned i = 1; i < MissingAttributes.size(); ++i) {
5732+
if (AttrType != MissingAttributes[i]->getType()) {
5733+
// Clear vector of missing attributes because it could be used in
5734+
// diagnosing missing format attributes in another caller.
5735+
MissingAttributes.clear();
5736+
return;
5737+
}
5738+
}
5739+
5740+
for (const FormatAttr *FA : MissingAttributes) {
5741+
// If format index and first-to-check argument index are negative, it means
5742+
// that this attribute is only saved for multiple format types checking.
5743+
if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
5744+
continue;
5745+
5746+
// If caller function has format attributes and callee format attribute type
5747+
// mismatches caller attribute type, do not emit diagnostic.
5748+
if (Caller->hasAttr<FormatAttr>() &&
5749+
!llvm::any_of(Caller->specific_attrs<FormatAttr>(),
5750+
[FA](const FormatAttr *FunctionAttr) {
5751+
return FA->getType() == FunctionAttr->getType();
5752+
}))
5753+
continue;
5754+
5755+
// Emit diagnostic
5756+
SourceLocation Loc = Caller->getFirstDecl()->getLocation();
5757+
Diag(Loc, diag::warn_missing_format_attribute)
5758+
<< FA->getType() << Caller
5759+
<< FixItHint::CreateInsertion(Loc,
5760+
(llvm::Twine("__attribute__((format(") +
5761+
FA->getType()->getName() + ", " +
5762+
llvm::Twine(FA->getFormatIdx()) + ", " +
5763+
llvm::Twine(FA->getFirstArg()) + ")))")
5764+
.str());
5765+
Diag(FA->getLocation(), diag::note_format_function) << FA->getType();
5766+
}
5767+
5768+
// Clear vector of missing attributes after emitting diagnostics for caller
5769+
// function because it could be used in diagnosing missing format attributes
5770+
// in another caller.
5771+
MissingAttributes.clear();
5772+
}
5773+
55995774
//===----------------------------------------------------------------------===//
56005775
// Microsoft specific attribute handlers.
56015776
//===----------------------------------------------------------------------===//

0 commit comments

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