Skip to content

Commit

Permalink
Revert "[clang] Catch missing format attributes (#70024)"
Browse files Browse the repository at this point in the history 
This reverts commit 70f57d2.
  • Loading branch information
@AaronBallman
AaronBallman committed Jul 12, 2024
1 parent 2369a54 commit f1a5264
Show file tree
Hide file tree
Showing 9 changed files with 3 additions and 813 deletions.
3 changes: 0 additions & 3 deletions clang/docs/ReleaseNotes.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -716,9 +716,6 @@ Improvements to Clang's diagnostics

- Clang now diagnoses integer constant expressions that are folded to a constant value as an extension in more circumstances. Fixes #GH59863

- Clang now diagnoses missing format attributes for non-template functions and
class/struct/union members. Fixes #GH60718

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

Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ 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
3 changes: 0 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1031,9 +1031,6 @@ def err_opencl_invalid_param : Error<
def err_opencl_invalid_return : Error<
"declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">;
def warn_enum_value_overflow : Warning<"overflow in enumeration value">;
def warn_missing_format_attribute : Warning<
"diagnostic behavior may be improved by adding the %0 format attribute to the declaration of %1">,
InGroup<DiagGroup<"missing-format-attribute">>, DefaultIgnore;
def warn_pragma_options_align_reset_failed : Warning<
"#pragma options align=reset failed: %0">,
InGroup<IgnoredPragmas>;
Expand Down
7 changes: 0 additions & 7 deletions clang/include/clang/Sema/Attr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,6 @@ inline bool isInstanceMethod(const Decl *D) {
return false;
}

inline bool checkIfMethodHasImplicitObjectParameter(const Decl *D) {
if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(D))
return MethodDecl->isInstance() &&
!MethodDecl->hasCXXExplicitFunctionObjectParameter();
return false;
}

/// Diagnose mutually exclusive attributes when present on a given
/// declaration. Returns true if diagnosed.
template <typename AttrTy>
Expand Down
4 changes: 0 additions & 4 deletions clang/include/clang/Sema/Sema.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4594,10 +4594,6 @@ class Sema final : public SemaBase {

enum class RetainOwnershipKind { NS, CF, OS };

void DiagnoseMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl);
std::vector<FormatAttr *>
GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl);

UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
StringRef UuidAsWritten, MSGuidDecl *GuidDecl);

Expand Down
2 changes: 0 additions & 2 deletions clang/lib/Sema/SemaDecl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15934,8 +15934,6 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
}
}

DiagnoseMissingFormatAttributes(Body, FD);

// We might not have found a prototype because we didn't wish to warn on
// the lack of a missing prototype. Try again without the checks for
// whether we want to warn on the missing prototype.
Expand Down
219 changes: 2 additions & 217 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3508,7 +3508,7 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) {

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

IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
Expand Down Expand Up @@ -3621,7 +3621,7 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
return;
}

bool HasImplicitThisParam = checkIfMethodHasImplicitObjectParameter(D);
bool HasImplicitThisParam = isInstanceMethod(D);
int32_t NumArgs = getFunctionOrMethodNumParams(D);

FunctionDecl *FD = D->getAsFunction();
Expand Down Expand Up @@ -5320,221 +5320,6 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI));
}

// This function is called only if function call is not inside template body.
// TODO: Add call for function calls inside template body.
// Emit warnings if parent function misses format attributes.
void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,
const FunctionDecl *FDecl) {
assert(FDecl);

// If there are no function body, exit.
if (!Body)
return;

// Get missing format attributes
std::vector<FormatAttr *> MissingFormatAttributes =
GetMissingFormatAttributes(Body, FDecl);
if (MissingFormatAttributes.empty())
return;

// Check if there are more than one format type found. In that case do not
// emit diagnostic.
const FormatAttr *FirstAttr = MissingFormatAttributes[0];
if (llvm::any_of(MissingFormatAttributes, [&](const FormatAttr *Attr) {
return FirstAttr->getType() != Attr->getType();
}))
return;

for (const FormatAttr *FA : MissingFormatAttributes) {
// If format index and first-to-check argument index are negative, it means
// that this attribute is only saved for multiple format types checking.
if (FA->getFormatIdx() < 0 || FA->getFirstArg() < 0)
continue;

// Emit diagnostic
SourceLocation Loc = FDecl->getLocation();
Diag(Loc, diag::warn_missing_format_attribute)
<< FA->getType() << FDecl
<< FixItHint::CreateInsertion(Loc,
(llvm::Twine("__attribute__((format(") +
FA->getType()->getName() + ", " +
llvm::Twine(FA->getFormatIdx()) + ", " +
llvm::Twine(FA->getFirstArg()) + ")))")
.str());
}
}

// Returns vector of format attributes. There are no two attributes with same
// arguments in returning vector. There can be attributes that effectivelly only
// store information about format type.
std::vector<FormatAttr *>
Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
unsigned int FunctionFormatArgumentIndexOffset =
checkIfMethodHasImplicitObjectParameter(FDecl) ? 2 : 1;

std::vector<FormatAttr *> MissingAttributes;

// Iterate over body statements.
for (auto *Child : Body->children()) {
// If child statement is compound statement, recursively get missing
// attributes.
if (dyn_cast_or_null<CompoundStmt>(Child)) {
std::vector<FormatAttr *> CompoundStmtMissingAttributes =
GetMissingFormatAttributes(Child, FDecl);

// If there are already missing attributes with same arguments, do not add
// duplicates.
for (FormatAttr *FA : CompoundStmtMissingAttributes) {
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
return FA->getType() == Attr->getType() &&
FA->getFormatIdx() == Attr->getFormatIdx() &&
FA->getFirstArg() == Attr->getFirstArg();
}))
MissingAttributes.push_back(FA);
}

continue;
}

ValueStmt *VS = dyn_cast_or_null<ValueStmt>(Child);
if (!VS)
continue;
Expr *TheExpr = VS->getExprStmt();
if (!TheExpr)
continue;
CallExpr *TheCall = dyn_cast_or_null<CallExpr>(TheExpr);
if (!TheCall)
continue;
const FunctionDecl *ChildFunction =
dyn_cast_or_null<FunctionDecl>(TheCall->getCalleeDecl());
if (!ChildFunction)
continue;

Expr **Args = TheCall->getArgs();
unsigned int NumArgs = TheCall->getNumArgs();

// If child expression is function, check if it is format function.
// If it is, check if parent function misses format attributes.

// If child function is format function and format arguments are not
// relevant to emit diagnostic, save only information about format type
// (format index and first-to-check argument index are set to -1).
// Information about format type is later used to determine if there are
// more than one format type found.

unsigned int ChildFunctionFormatArgumentIndexOffset =
checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;

// Check if function has format attribute with forwarded format string.
IdentifierInfo *AttrType;
const ParmVarDecl *FormatArg;
if (!llvm::any_of(ChildFunction->specific_attrs<FormatAttr>(),
[&](const FormatAttr *Attr) {
AttrType = Attr->getType();

int OffsetFormatIndex =
Attr->getFormatIdx() -
ChildFunctionFormatArgumentIndexOffset;
if (OffsetFormatIndex < 0 ||
(unsigned)OffsetFormatIndex >= NumArgs)
return false;

const auto *FormatArgExpr = dyn_cast<DeclRefExpr>(
Args[OffsetFormatIndex]->IgnoreParenCasts());
if (!FormatArgExpr)
return false;

FormatArg = dyn_cast_or_null<ParmVarDecl>(
FormatArgExpr->getReferencedDeclOfCallee());
if (!FormatArg)
return false;

return true;
})) {
MissingAttributes.push_back(
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
continue;
}

// Do not add in a vector format attributes whose type is different than
// parent function attribute type.
if (llvm::any_of(FDecl->specific_attrs<FormatAttr>(),
[&](const FormatAttr *FunctionAttr) {
return AttrType != FunctionAttr->getType();
}))
continue;

// Check if format string argument is parent function parameter.
unsigned int StringIndex = 0;
if (!llvm::any_of(FDecl->parameters(), [&](const ParmVarDecl *Param) {
if (Param != FormatArg)
return false;

StringIndex = Param->getFunctionScopeIndex() +
FunctionFormatArgumentIndexOffset;

return true;
})) {
MissingAttributes.push_back(
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
continue;
}

unsigned NumOfParentFunctionParams = FDecl->getNumParams();

// Compare parent and calling function format attribute arguments (archetype
// and format string).
if (llvm::any_of(
FDecl->specific_attrs<FormatAttr>(), [&](const FormatAttr *Attr) {
if (Attr->getType() != AttrType)
return false;
int OffsetFormatIndex =
Attr->getFormatIdx() - FunctionFormatArgumentIndexOffset;

if (OffsetFormatIndex < 0 ||
(unsigned)OffsetFormatIndex >= NumOfParentFunctionParams)
return false;

if (FDecl->parameters()[OffsetFormatIndex] != FormatArg)
return false;

return true;
})) {
MissingAttributes.push_back(
FormatAttr::CreateImplicit(getASTContext(), AttrType, -1, -1));
continue;
}

// Get first argument index
unsigned FirstToCheck = [&]() -> unsigned {
if (!FDecl->isVariadic())
return 0;
const auto *FirstToCheckArg =
dyn_cast<DeclRefExpr>(Args[NumArgs - 1]->IgnoreParenCasts());
if (!FirstToCheckArg)
return 0;

if (FirstToCheckArg->getType().getCanonicalType() !=
Context.getBuiltinVaListType().getCanonicalType())
return 0;
return NumOfParentFunctionParams + FunctionFormatArgumentIndexOffset;
}();

// If there are already attributes which arguments matches arguments
// detected in this iteration, do not add new attribute as it would be
// duplicate.
if (!llvm::any_of(MissingAttributes, [&](const FormatAttr *Attr) {
return Attr->getType() == AttrType &&
Attr->getFormatIdx() == StringIndex &&
Attr->getFirstArg() == FirstToCheck;
}))
MissingAttributes.push_back(FormatAttr::CreateImplicit(
getASTContext(), AttrType, StringIndex, FirstToCheck));
}

return MissingAttributes;
}

//===----------------------------------------------------------------------===//
// Microsoft specific attribute handlers.
//===----------------------------------------------------------------------===//
Expand Down
Loading

0 comments on commit f1a5264

Please sign in to comment.