Revert "[clang] Catch missing format attributes" #98617
Merged
+3
−813
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 70f57d2.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) ChangesReverts llvm/llvm-project#70024 It broke several post-commit bots: Patch is 41.74 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/98617.diff 9 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index afcd3c655f0f6..781fc8ab1de1e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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
----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index da6a3b2fe3571..2241f8481484e 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -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">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f5c18edb65217..0ea3677355169 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -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>;
diff --git a/clang/include/clang/Sema/Attr.h b/clang/include/clang/Sema/Attr.h
index 37c124ca7b454..3f0b10212789a 100644
--- a/clang/include/clang/Sema/Attr.h
+++ b/clang/include/clang/Sema/Attr.h
@@ -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>
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e30252749d2c3..6be6f6725e5b7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -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);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0039df2a21bc6..80b5a8cd4bae6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -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.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 5166e61bb41d4..f2cd46d1e7c93 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -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;
@@ -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();
@@ -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.
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Sema/attr-format-missing.c b/clang/test/Sema/attr-format-missing.c
deleted file mode 100644
index 4f9e91eb1becb..0000000000000
--- a/clang/test/Sema/attr-format-missing.c
+++ /dev/null
@@ -1,403 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,c_diagnostics -Wmissing-format-attribute %s
-// RUN: %clang_cc1 -fsyntax-only -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefixes=CHECK,C-CHECK
-// RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -Wmissing-format-attribute %s
-// RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -std=c++2b -Wmissing-format-attribute %s
-// RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -std=c++23 -Wmissing-format-attribute %s
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple x86_64-linux %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-LIN64
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple x86_64-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple i386-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
-// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple i386-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
-
-#ifndef __cplusplus
-typedef unsigned short char16_t;
-typedef unsigned int char32_t;
-typedef __WCHAR_TYPE__ wchar_t;
-#endif
-
-typedef __SIZE_TYPE__ size_t;
-typedef __builtin_va_list va_list;
-
-__attribute__((__format__(__printf__, 1, 2)))
-int printf(const char *, ...); // #printf
-
-__attribute__((__format__(__scanf__, 1, 2)))
-int scanf(const char *, ...); // #scanf
-
-__attribute__((__format__(__printf__, 1, 0)))
-int vprintf(const char *, va_list); // #vprintf
-
-__attribute__((__format__(__scanf__, 1, 0)))
-int vscanf(const char *, va_list); // #vscanf
-
-__attribute__((__format__(__printf__, 2, 0)))
-int vsprintf(char *, const char *, va_list); // #vsprintf
-
-__attribute__((__format__(__printf__, 3, 0)))
-int vsnprintf(char *ch, size_t, const char *, va_list); // #vsnprintf
-
-__attribute__((__format__(__scanf__, 1, 4)))
-void f1(char *out, const size_t len, const char *format, ... /* args */) // #f1
-{
- va_list args;
- vsnprintf(out, len, format, args); // expected-no-warning@#f1
-}
-
-__attribute__((__format__(__printf__, 1, 4)))
-void f2(char *out, const size_t len, const char *format, ... /* args */) // #f2
-{
- va_list args;
- vsnprintf(out, len, format, args); // expected-warning@#f2 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f2'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:6-[[@LINE-4]]:6}:"__attribute__((format(printf, 3, 4)))"
-}
-
-void f3(char *out, va_list args) // #f3
-{
- vprintf(out, args); // expected-warning@#f3 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f3'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:6-[[@LINE-3]]:6}:"__attribute__((format(printf, 1, 0)))"
-}
-
-void f4(char* out, ... /* args */) // #f4
-{
- va_list args;
- vprintf("test", args); // expected-no-warning@#f4
-
- const char *ch;
- vprintf(ch, args); // expected-no-warning@#f4
-}
-
-void f5(va_list args) // #f5
-{
- char *ch;
- vscanf(ch, args); // expected-no-warning@#f5
-}
-
-void f6(char *out, va_list args) // #f6
-{
- char *ch;
- vprintf(ch, args); // expected-no-warning@#f6
- vprintf("test", args); // expected-no-warning@#f6
- vprintf(out, args); // expected-warning@#f6 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f6'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:6-[[@LINE-6]]:6}:"__attribute__((format(printf, 1, 0)))"
-}
-
-void f7(const char *out, ... /* args */) // #f7
-{
- va_list args;
-
- vscanf(out, &args[0]); // expected-warning@#f7 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f7'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:6-[[@LINE-5]]:6}:"__attribute__((format(scanf, 1, 0)))"
-}
-
-void f8(const char *out, ... /* args */) // #f8
-{
- va_list args;
-
- vscanf(out, &args[0]); // expected-no-warning@#f8
- vprintf(out, &args[0]); // expected-no-warning@#f8
-}
-
-void f9(const char out[], ... /* args */) // #f9
-{
- va_list args;
- char *ch;
- vprintf(ch, args); // expected-no-warning
- vsprintf(ch, out, args); // expected-warning@#f9 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f9'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:6-[[@LINE-6]]:6}:"__attribute__((format(printf, 1, 2)))"
-}
-
-void f10(const wchar_t *out, ... /* args */) // #f10
-{
- va_list args;
- vscanf(out, args);
-#if __SIZEOF_WCHAR_T__ == 4
- // c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
-#else
- // c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
-#endif
- // c_diagnostics-note@#vscanf {{passing argument to parameter here}}
- // c_diagnostics-warning@#f10 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f10'}}
- // cpp_diagnostics-error@-8 {{no matching function for call to 'vscanf'}}
- // cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st argument}}
- // C-CHECK: fix-it:"{{.*}}":{[[@LINE-13]]:6-[[@LINE-13]]:6}:"__attribute__((format(scanf, 1, 2)))"
-}
-
-void f11(const wchar_t *out, ... /* args */) // #f11
-{
- va_list args;
- vscanf((const char *) out, args); // expected-warning@#f11 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f11'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:6-[[@LINE-4]]:6}:"__attribute__((format(s...
[truncated]
|
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
Reverts llvm#70024 It broke several post-commit bots: https://lab.llvm.org/buildbot/#/builders/193/builds/896 https://lab.llvm.org/buildbot/#/builders/23/builds/925 https://lab.llvm.org/buildbot/#/builders/13/builds/686 and others
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #70024
It broke several post-commit bots:
https://lab.llvm.org/buildbot/#/builders/193/builds/896
https://lab.llvm.org/buildbot/#/builders/23/builds/925
https://lab.llvm.org/buildbot/#/builders/13/builds/686
and others