From bc7cc2074b7b7043e05cb46346f1368eb4ae9949 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Thu, 1 Jul 2021 12:39:17 -0400 Subject: [PATCH] Fix an accepts-invalid issue with [[]] attributes in the type position in C A user reported an issue to me via email that Clang was accepting some code that GCC was rejecting. After investigation, it turned out to be a general problem of us failing to properly reject attributes written in the type position in C when they don't apply to types. The root cause was a terminology issue -- we sometimes use "CXX11Attr" to mean [[]] in C++11 mode and sometimes [[]] in general -- and this came back to bite us because in this particular case, it really meant [[]] in C++ mode. I fixed the issue by introducing a new function AttributeCommonInfo::isStandardAttributeSyntax() to represent [[]] in either C or C++ mode. This fix pointed out that we've had the issue in some of our existing tests, which have all been corrected. This resolves https://bugs.llvm.org/show_bug.cgi?id=50954. --- clang/include/clang/Basic/AttributeCommonInfo.h | 6 ++++++ clang/lib/Parse/Parser.cpp | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 10 +++++----- clang/lib/Sema/SemaType.cpp | 14 +++++++------- clang/test/AST/ast-dump-c-attr.c | 5 ----- .../test/Sema/attr-availability-square-brackets.c | 9 +++++---- clang/test/Sema/attr-c2x.c | 10 +++++----- clang/test/Sema/attr-deprecated-c2x.c | 12 ++++++++---- clang/test/Sema/attr-external-source-symbol.c | 14 +++++++------- clang/test/Sema/c2x-maybe_unused-errors.c | 3 +++ clang/test/Sema/overloadable.c | 3 ++- 11 files changed, 49 insertions(+), 39 deletions(-) diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h index f4a5db84aa9f..4be598e109fd 100644 --- a/clang/include/clang/Basic/AttributeCommonInfo.h +++ b/clang/include/clang/Basic/AttributeCommonInfo.h @@ -155,6 +155,12 @@ public: bool isC2xAttribute() const { return SyntaxUsed == AS_C2x; } + /// The attribute is spelled [[]] in either C or C++ mode, including standard + /// attributes spelled with a keyword, like alignas. + bool isStandardAttributeSyntax() const { + return isCXX11Attribute() || isC2xAttribute(); + } + bool isKeywordAttribute() const { return SyntaxUsed == AS_Keyword || SyntaxUsed == AS_ContextSensitiveKeyword; } diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index c0b83db69ce9..55b25d20db51 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1214,7 +1214,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D, // a definition. Late parsed attributes are checked at the end. if (Tok.isNot(tok::equal)) { for (const ParsedAttr &AL : D.getAttributes()) - if (AL.isKnownToGCC() && !AL.isCXX11Attribute()) + if (AL.isKnownToGCC() && !AL.isStandardAttributeSyntax()) Diag(AL.getLoc(), diag::warn_attribute_on_function_definition) << AL; } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index d8416c6b5769..0741b5f6fda9 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2093,7 +2093,7 @@ static void handleAnalyzerNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &AL) { ValueDecl *VD = dyn_cast(D); if (!VD || (!VD->getType()->isBlockPointerType() && !VD->getType()->isFunctionPointerType())) { - S.Diag(AL.getLoc(), AL.isCXX11Attribute() + S.Diag(AL.getLoc(), AL.isStandardAttributeSyntax() ? diag::err_attribute_wrong_decl_type : diag::warn_attribute_wrong_decl_type) << AL << ExpectedFunctionMethodOrBlock; @@ -2863,7 +2863,7 @@ static void handleWarnUnusedResult(Sema &S, Decl *D, const ParsedAttr &AL) { } StringRef Str; - if ((AL.isCXX11Attribute() || AL.isC2xAttribute()) && !AL.getScopeName()) { + if (AL.isStandardAttributeSyntax() && !AL.getScopeName()) { // The standard attribute cannot be applied to variable declarations such // as a function pointer. if (isa(D)) @@ -7280,8 +7280,8 @@ static void handleDeprecatedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { !S.checkStringLiteralArgumentAttr(AL, 0, Str)) return; - // Only support a single optional message for Declspec and CXX11. - if (AL.isDeclspecAttribute() || AL.isCXX11Attribute()) + // Support a single optional message only for Declspec and [[]] spellings. + if (AL.isDeclspecAttribute() || AL.isStandardAttributeSyntax()) AL.checkAtMostNumArgs(S, 1); else if (AL.isArgExpr(1) && AL.getArgAsExpr(1) && !S.checkStringLiteralArgumentAttr(AL, 1, Replacement)) @@ -7348,7 +7348,7 @@ static void handleNoSanitizeSpecificAttr(Sema &S, Decl *D, // getSpelling() or prettyPrint() on the resulting semantic attribute object // without failing assertions. unsigned TranslatedSpellingIndex = 0; - if (AL.isC2xAttribute() || AL.isCXX11Attribute()) + if (AL.isStandardAttributeSyntax()) TranslatedSpellingIndex = 1; AttributeCommonInfo Info = AL; diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 2434554ba465..ef0320fb26f8 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -634,7 +634,7 @@ static void distributeFunctionTypeAttrFromDeclSpec(TypeProcessingState &state, // C++11 attributes before the decl specifiers actually appertain to // the declarators. Move them straight there. We don't support the // 'put them wherever you like' semantics we allow for GNU attributes. - if (attr.isCXX11Attribute()) { + if (attr.isStandardAttributeSyntax()) { moveAttrFromListToList(attr, state.getCurrentAttributes(), state.getDeclarator().getAttributes()); return; @@ -687,9 +687,9 @@ static void distributeTypeAttrsFromDeclarator(TypeProcessingState &state, // non-owning copy and iterate over that. ParsedAttributesView AttrsCopy{state.getDeclarator().getAttributes()}; for (ParsedAttr &attr : AttrsCopy) { - // Do not distribute C++11 attributes. They have strict rules for what + // Do not distribute [[]] attributes. They have strict rules for what // they appertain to. - if (attr.isCXX11Attribute()) + if (attr.isStandardAttributeSyntax()) continue; switch (attr.getKind()) { @@ -8058,7 +8058,7 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, if (attr.isInvalid()) continue; - if (attr.isCXX11Attribute()) { + if (attr.isStandardAttributeSyntax()) { // [[gnu::...]] attributes are treated as declaration attributes, so may // not appertain to a DeclaratorChunk. If we handle them as type // attributes, accept them in that position and diagnose the GCC @@ -8087,8 +8087,8 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, // otherwise, add it to the FnAttrs list for rechaining. switch (attr.getKind()) { default: - // A C++11 attribute on a declarator chunk must appertain to a type. - if (attr.isCXX11Attribute() && TAL == TAL_DeclChunk) { + // A [[]] attribute on a declarator chunk must appertain to a type. + if (attr.isStandardAttributeSyntax() && TAL == TAL_DeclChunk) { state.getSema().Diag(attr.getLoc(), diag::err_attribute_not_type_attr) << attr; attr.setUsedAsTypeAttr(); @@ -8096,7 +8096,7 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, break; case ParsedAttr::UnknownAttribute: - if (attr.isCXX11Attribute() && TAL == TAL_DeclChunk) + if (attr.isStandardAttributeSyntax() && TAL == TAL_DeclChunk) state.getSema().Diag(attr.getLoc(), diag::warn_unknown_attribute_ignored) << attr << attr.getRange(); diff --git a/clang/test/AST/ast-dump-c-attr.c b/clang/test/AST/ast-dump-c-attr.c index 7d18f0cdc9f1..1f28501f1bb0 100644 --- a/clang/test/AST/ast-dump-c-attr.c +++ b/clang/test/AST/ast-dump-c-attr.c @@ -52,8 +52,3 @@ struct [[deprecated]] Test8; void Test11 [[deprecated]](void); // CHECK: FunctionDecl{{.*}}Test11 // CHECK-NEXT: DeprecatedAttr 0x{{[^ ]*}} "" "" - -void Test12(void) [[deprecated]] {} -// CHECK: FunctionDecl{{.*}}Test12 -// CHECK-NEXT: CompoundStmt -// CHECK-NEXT: DeprecatedAttr 0x{{[^ ]*}} "" "" diff --git a/clang/test/Sema/attr-availability-square-brackets.c b/clang/test/Sema/attr-availability-square-brackets.c index 13dbf0abb17f..b03e19e4da23 100644 --- a/clang/test/Sema/attr-availability-square-brackets.c +++ b/clang/test/Sema/attr-availability-square-brackets.c @@ -1,11 +1,12 @@ // RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fdouble-square-bracket-attributes -verify %s -void f0() [[clang::availability(macosx,introduced=10.4,deprecated=10.2)]]; // expected-warning{{feature cannot be deprecated in macOS version 10.2 before it was introduced in version 10.4; attribute ignored}} -void f1() [[clang::availability(ios,obsoleted=2.1,deprecated=3.0)]]; // expected-warning{{feature cannot be obsoleted in iOS version 2.1 before it was deprecated in version 3.0; attribute ignored}} -void f2() [[clang::availability(ios,introduced=2.1,deprecated=2.1)]]; +[[clang::availability(macosx,introduced=10.4,deprecated=10.2)]] void f0(); // expected-warning{{feature cannot be deprecated in macOS version 10.2 before it was introduced in version 10.4; attribute ignored}} +[[clang::availability(ios,obsoleted=2.1,deprecated=3.0)]] void f1(); // expected-warning{{feature cannot be obsoleted in iOS version 2.1 before it was deprecated in version 3.0; attribute ignored}} +[[clang::availability(ios,introduced=2.1,deprecated=2.1)]] void f2(); +[[clang::availability(macosx,introduced=8.0,deprecated=9.0, message="use CTFontCopyFullName")]] extern void -ATSFontGetName(const char *oName) [[clang::availability(macosx,introduced=8.0,deprecated=9.0, message="use CTFontCopyFullName")]]; // expected-note {{'ATSFontGetName' has been explicitly marked deprecated here}} +ATSFontGetName(const char *oName); // expected-note {{'ATSFontGetName' has been explicitly marked deprecated here}} void test_10095131() { ATSFontGetName("Hello"); // expected-warning {{'ATSFontGetName' is deprecated: first deprecated in macOS 9.0 - use CTFontCopyFullName}} diff --git a/clang/test/Sema/attr-c2x.c b/clang/test/Sema/attr-c2x.c index fae4c5d0fa90..016b1f58e3a7 100644 --- a/clang/test/Sema/attr-c2x.c +++ b/clang/test/Sema/attr-c2x.c @@ -11,16 +11,16 @@ enum [[clang::flag_enum]] EnumFlag { D0 = 1, D1 = 8 }; -void foo(void *c) [[clang::overloadable]]; -void foo(char *c) [[clang::overloadable]]; +[[clang::overloadable]] void foo(void *c); +[[clang::overloadable]] void foo(char *c); void context_okay(void *context [[clang::swift_context]]) [[clang::swiftcall]]; void context_okay2(void *context [[clang::swift_context]], void *selfType, char **selfWitnessTable) [[clang::swiftcall]]; -void *f1(void) [[clang::ownership_returns(foo)]]; -void *f2() [[clang::ownership_returns(foo)]]; // expected-warning {{'ownership_returns' attribute only applies to non-K&R-style functions}} +[[clang::ownership_returns(foo)]] void *f1(void); +[[clang::ownership_returns(foo)]] void *f2(); // expected-warning {{'ownership_returns' attribute only applies to non-K&R-style functions}} -void foo2(void) [[clang::unavailable("not available - replaced")]]; // expected-note {{'foo2' has been explicitly marked unavailable here}} +[[clang::unavailable("not available - replaced")]] void foo2(void); // expected-note {{'foo2' has been explicitly marked unavailable here}} void bar(void) { foo2(); // expected-error {{'foo2' is unavailable: not available - replaced}} } diff --git a/clang/test/Sema/attr-deprecated-c2x.c b/clang/test/Sema/attr-deprecated-c2x.c index 744fb1f7c400..ba8434e8b094 100644 --- a/clang/test/Sema/attr-deprecated-c2x.c +++ b/clang/test/Sema/attr-deprecated-c2x.c @@ -1,7 +1,7 @@ -// RUN: %clang_cc1 %s -verify -fsyntax-only --std=c2x +// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c2x -int f() [[deprecated]]; // expected-note 2 {{'f' has been explicitly marked deprecated here}} -void g() [[deprecated]];// expected-note {{'g' has been explicitly marked deprecated here}} +[[deprecated]] int f(); // expected-note 2 {{'f' has been explicitly marked deprecated here}} +[[deprecated]] void g();// expected-note {{'g' has been explicitly marked deprecated here}} void g(); extern int var [[deprecated]]; // expected-note 2 {{'var' has been explicitly marked deprecated here}} @@ -22,7 +22,7 @@ int w() { return var; // expected-warning {{'var' is deprecated}} } -int old_fn() [[deprecated]];// expected-note {{'old_fn' has been explicitly marked deprecated here}} +[[deprecated]] int old_fn();// expected-note {{'old_fn' has been explicitly marked deprecated here}} int old_fn(); int (*fn_ptr)() = old_fn; // expected-warning {{'old_fn' is deprecated}} @@ -52,3 +52,7 @@ struct bar_dep *test3; // expected-warning {{'bar_dep' is deprecated}} void test4(void) { i = 12; // expected-warning {{'i' is deprecated: this is the message}} } + +// Ensure that deprecated only accepts one argument, not the replacement +// argument supported as a GNU extension. +[[deprecated("message", "replacement not supported")]] void test5(void); // expected-error {{'deprecated' attribute takes no more than 1 argument}} diff --git a/clang/test/Sema/attr-external-source-symbol.c b/clang/test/Sema/attr-external-source-symbol.c index dfed609c8e87..f257a63504d4 100644 --- a/clang/test/Sema/attr-external-source-symbol.c +++ b/clang/test/Sema/attr-external-source-symbol.c @@ -18,14 +18,14 @@ void namedDeclsOnly() { }; } -void threeClauses2() [[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration)]]; +[[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration)]] void threeClauses2(); -void twoClauses2() [[clang::external_source_symbol(language="Swift", defined_in="module")]]; +[[clang::external_source_symbol(language="Swift", defined_in="module")]] void twoClauses2(); -void fourClauses2() -[[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration, generated_declaration)]]; // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}} +[[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration, generated_declaration)]] // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}} +void fourClauses2(); -void oneClause2() [[clang::external_source_symbol(generated_declaration)]]; +[[clang::external_source_symbol(generated_declaration)]] void oneClause2(); -void noArguments2() -[[clang::external_source_symbol]]; // expected-error {{'external_source_symbol' attribute takes at least 1 argument}} +[[clang::external_source_symbol]] // expected-error {{'external_source_symbol' attribute takes at least 1 argument}} +void noArguments2(); diff --git a/clang/test/Sema/c2x-maybe_unused-errors.c b/clang/test/Sema/c2x-maybe_unused-errors.c index 72cefd10291a..bb9931cd8d3d 100644 --- a/clang/test/Sema/c2x-maybe_unused-errors.c +++ b/clang/test/Sema/c2x-maybe_unused-errors.c @@ -10,3 +10,6 @@ struct [[maybe_unused("Wrong")]] S3 { // expected-error {{'maybe_unused' cannot int a; }; +void func(void) { + int a[10] [[maybe_unused]]; // expected-error {{'maybe_unused' attribute cannot be applied to types}} +} diff --git a/clang/test/Sema/overloadable.c b/clang/test/Sema/overloadable.c index 360f3308302e..b520d76f9e7e 100644 --- a/clang/test/Sema/overloadable.c +++ b/clang/test/Sema/overloadable.c @@ -1,6 +1,7 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s -Wincompatible-pointer-types +// RUN: %clang_cc1 -fsyntax-only -fdouble-square-bracket-attributes -verify %s -Wincompatible-pointer-types int var __attribute__((overloadable)); // expected-error{{'overloadable' attribute only applies to functions}} +void bad_attr_target(int) [[clang::overloadable]]; // expected-error{{'overloadable' attribute cannot be applied to types}} void params(void) __attribute__((overloadable(12))); // expected-error {{'overloadable' attribute takes no arguments}} int *f(int) __attribute__((overloadable)); // expected-note{{previous overload of function is here}}