Skip to content

Revert "[clang] Catch missing format attributes" #98617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions clang/docs/ReleaseNotes.rst
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
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
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
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
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
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
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
Loading