Skip to content

[Clang] restrict use of attribute names reserved by the C++ standard #106036

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 13 commits into from
Jan 23, 2025
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
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,8 @@ Improvements to Clang's diagnostics

- Clang now emits a ``-Wignored-qualifiers`` diagnostic when a base class includes cv-qualifiers (#GH55474).

- Clang now diagnoses the use of attribute names reserved by the C++ standard (#GH92196).

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

Expand Down
9 changes: 8 additions & 1 deletion clang/include/clang/Basic/AttributeCommonInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,18 @@ class AttributeCommonInfo {
};
enum Kind {
#define PARSED_ATTR(NAME) AT_##NAME,
#include "clang/Sema/AttrParsedAttrList.inc"
#include "clang/Basic/AttrParsedAttrList.inc"
#undef PARSED_ATTR
NoSemaHandlerAttribute,
IgnoredAttribute,
UnknownAttribute,
};
enum class Scope { NONE, CLANG, GNU, MSVC, OMP, HLSL, GSL, RISCV };
enum class AttrArgsInfo {
None,
Optional,
Required,
};

private:
const IdentifierInfo *AttrName = nullptr;
Expand Down Expand Up @@ -241,6 +246,8 @@ class AttributeCommonInfo {
static Kind getParsedKind(const IdentifierInfo *Name,
const IdentifierInfo *Scope, Syntax SyntaxUsed);

static AttrArgsInfo getCXX11AttrArgsInfo(const IdentifierInfo *Name);

private:
/// Get an index into the attribute spelling list
/// defined in Attr.td. This index is used by an attribute
Expand Down
5 changes: 5 additions & 0 deletions clang/include/clang/Basic/Attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ int hasAttribute(AttributeCommonInfo::Syntax Syntax,
const IdentifierInfo *Scope, const IdentifierInfo *Attr,
const TargetInfo &Target, const LangOptions &LangOpts);

int hasAttribute(AttributeCommonInfo::Syntax Syntax,
const IdentifierInfo *Scope, const IdentifierInfo *Attr,
const TargetInfo &Target, const LangOptions &LangOpts,
bool CheckPlugins);

} // end namespace clang

#endif // LLVM_CLANG_BASIC_ATTRIBUTES_H
11 changes: 11 additions & 0 deletions clang/include/clang/Basic/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ clang_tablegen(AttrList.inc -gen-clang-attr-list
SOURCE Attr.td
TARGET ClangAttrList)

clang_tablegen(AttrParsedAttrList.inc -gen-clang-attr-parsed-attr-list
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE Attr.td
TARGET ClangAttrParsedAttrList)

clang_tablegen(AttrSubMatchRulesList.inc -gen-clang-attr-subject-match-rule-list
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE Attr.td
Expand All @@ -53,6 +58,12 @@ clang_tablegen(AttrHasAttributeImpl.inc -gen-clang-attr-has-attribute-impl
TARGET ClangAttrHasAttributeImpl
)

clang_tablegen(CXX11AttributeInfo.inc -gen-cxx11-attribute-info
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE Attr.td
TARGET CXX11AttributeInfo
)

clang_tablegen(Builtins.inc -gen-clang-builtins
SOURCE Builtins.td
TARGET ClangBuiltins)
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ def AmbiguousMacro : DiagGroup<"ambiguous-macro">;
def KeywordAsMacro : DiagGroup<"keyword-macro">;
def ReservedIdAsMacro : DiagGroup<"reserved-macro-identifier">;
def ReservedIdAsMacroAlias : DiagGroup<"reserved-id-macro", [ReservedIdAsMacro]>;
def ReservedAttributeIdentifier : DiagGroup<"reserved-attribute-identifier">;
def RestrictExpansionMacro : DiagGroup<"restrict-expansion">;
def FinalMacro : DiagGroup<"final-macro">;

Expand Down Expand Up @@ -935,7 +936,8 @@ def SignedEnumBitfield : DiagGroup<"signed-enum-bitfield">;

def ReservedModuleIdentifier : DiagGroup<"reserved-module-identifier">;
def ReservedIdentifier : DiagGroup<"reserved-identifier",
[ReservedIdAsMacro, ReservedModuleIdentifier, UserDefinedLiterals]>;
[ReservedIdAsMacro, ReservedModuleIdentifier,
UserDefinedLiterals, ReservedAttributeIdentifier]>;

// Unreachable code warning groups.
//
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticLexKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,9 @@ def warn_pp_macro_hides_keyword : Extension<
def warn_pp_macro_is_reserved_id : Warning<
"macro name is a reserved identifier">, DefaultIgnore,
InGroup<ReservedIdAsMacro>;
def warn_pp_macro_is_reserved_attribute_id : Warning<
"%0 is a reserved attribute identifier">, DefaultIgnore,
InGroup<ReservedAttributeIdentifier>;
def warn_pp_objc_macro_redef_ignored : Warning<
"ignoring redefinition of Objective-C qualifier macro">,
InGroup<DiagGroup<"objc-macro-redefinition">>;
Expand Down
10 changes: 5 additions & 5 deletions clang/include/clang/Lex/Preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -2271,6 +2271,11 @@ class Preprocessor {
}
}

/// Determine whether the next preprocessor token to be
/// lexed is a '('. If so, consume the token and return true, if not, this
/// method should have no observable side-effect on the lexed tokens.
bool isNextPPTokenLParen();

private:
/// Identifiers used for SEH handling in Borland. These are only
/// allowed in particular circumstances
Expand Down Expand Up @@ -2648,11 +2653,6 @@ class Preprocessor {

void removeCachedMacroExpandedTokensOfLastLexer();

/// Determine whether the next preprocessor token to be
/// lexed is a '('. If so, consume the token and return true, if not, this
/// method should have no observable side-effect on the lexed tokens.
bool isNextPPTokenLParen();

/// After reading "MACRO(", this method is invoked to read all of the formal
/// arguments specified for the macro invocation. Returns null on error.
MacroArgs *ReadMacroCallArgumentList(Token &MacroName, MacroInfo *MI,
Expand Down
5 changes: 0 additions & 5 deletions clang/include/clang/Sema/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@ clang_tablegen(AttrTemplateInstantiate.inc -gen-clang-attr-template-instantiate
SOURCE ../Basic/Attr.td
TARGET ClangAttrTemplateInstantiate)

clang_tablegen(AttrParsedAttrList.inc -gen-clang-attr-parsed-attr-list
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE ../Basic/Attr.td
TARGET ClangAttrParsedAttrList)

clang_tablegen(AttrParsedAttrKinds.inc -gen-clang-attr-parsed-attr-kinds
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../
SOURCE ../Basic/Attr.td
Expand Down
31 changes: 26 additions & 5 deletions clang/lib/Basic/Attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,

int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
const IdentifierInfo *Scope, const IdentifierInfo *Attr,
const TargetInfo &Target, const LangOptions &LangOpts) {
const TargetInfo &Target, const LangOptions &LangOpts,
bool CheckPlugins) {
StringRef Name = Attr->getName();
// Normalize the attribute name, __foo__ becomes foo.
if (Name.size() >= 4 && Name.starts_with("__") && Name.ends_with("__"))
Expand Down Expand Up @@ -61,14 +62,23 @@ int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
if (res)
return res;

// Check if any plugin provides this attribute.
for (auto &Ptr : getAttributePluginInstances())
if (Ptr->hasSpelling(Syntax, Name))
return 1;
if (CheckPlugins) {
// Check if any plugin provides this attribute.
for (auto &Ptr : getAttributePluginInstances())
if (Ptr->hasSpelling(Syntax, Name))
return 1;
}

return 0;
}

int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
const IdentifierInfo *Scope, const IdentifierInfo *Attr,
const TargetInfo &Target, const LangOptions &LangOpts) {
return hasAttribute(Syntax, Scope, Attr, Target, LangOpts,
/*CheckPlugins=*/true);
}

const char *attr::getSubjectMatchRuleSpelling(attr::SubjectMatchRule Rule) {
switch (Rule) {
#define ATTR_MATCH_RULE(NAME, SPELLING, IsAbstract) \
Expand Down Expand Up @@ -151,6 +161,17 @@ AttributeCommonInfo::getParsedKind(const IdentifierInfo *Name,
return ::getAttrKind(normalizeName(Name, ScopeName, SyntaxUsed), SyntaxUsed);
}

AttributeCommonInfo::AttrArgsInfo
AttributeCommonInfo::getCXX11AttrArgsInfo(const IdentifierInfo *Name) {
StringRef AttrName =
normalizeAttrName(Name, /*NormalizedScopeName*/ "", Syntax::AS_CXX11);
#define CXX11_ATTR_ARGS_INFO
return llvm::StringSwitch<AttributeCommonInfo::AttrArgsInfo>(AttrName)
#include "clang/Basic/CXX11AttributeInfo.inc"
.Default(AttributeCommonInfo::AttrArgsInfo::None);
#undef CXX11_ATTR_ARGS_INFO
}

std::string AttributeCommonInfo::getNormalizedFullName() const {
return static_cast<std::string>(
normalizeName(getAttrName(), getScopeName(), getSyntax()));
Expand Down
28 changes: 27 additions & 1 deletion clang/lib/Lex/PPDirectives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
///
//===----------------------------------------------------------------------===//

#include "clang/Basic/AttributeCommonInfo.h"
#include "clang/Basic/Attributes.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/DirectoryEntry.h"
#include "clang/Basic/FileManager.h"
Expand Down Expand Up @@ -97,7 +99,8 @@ SourceRange Preprocessor::DiscardUntilEndOfDirective(Token &Tmp) {
enum MacroDiag {
MD_NoWarn, //> Not a reserved identifier
MD_KeywordDef, //> Macro hides keyword, enabled by default
MD_ReservedMacro //> #define of #undef reserved id, disabled by default
MD_ReservedMacro, //> #define of #undef reserved id, disabled by default
MD_ReservedAttributeIdentifier
};

/// Enumerates possible %select values for the pp_err_elif_after_else and
Expand Down Expand Up @@ -173,6 +176,22 @@ static bool isLanguageDefinedBuiltin(const SourceManager &SourceMgr,
return false;
}

static bool isReservedCXXAttributeName(Preprocessor &PP, IdentifierInfo *II) {
const LangOptions &Lang = PP.getLangOpts();
if (Lang.CPlusPlus &&
hasAttribute(AttributeCommonInfo::AS_CXX11, /* Scope*/ nullptr, II,
PP.getTargetInfo(), Lang, /*CheckPlugins*/ false) > 0) {
AttributeCommonInfo::AttrArgsInfo AttrArgsInfo =
AttributeCommonInfo::getCXX11AttrArgsInfo(II);
if (AttrArgsInfo == AttributeCommonInfo::AttrArgsInfo::Required)
return PP.isNextPPTokenLParen();

return !PP.isNextPPTokenLParen() ||
AttrArgsInfo == AttributeCommonInfo::AttrArgsInfo::Optional;
}
return false;
}

static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
const LangOptions &Lang = PP.getLangOpts();
StringRef Text = II->getName();
Expand All @@ -182,6 +201,8 @@ static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
return MD_KeywordDef;
if (Lang.CPlusPlus11 && (Text == "override" || Text == "final"))
return MD_KeywordDef;
if (isReservedCXXAttributeName(PP, II))
return MD_ReservedAttributeIdentifier;
return MD_NoWarn;
}

Expand All @@ -190,6 +211,8 @@ static MacroDiag shouldWarnOnMacroUndef(Preprocessor &PP, IdentifierInfo *II) {
// Do not warn on keyword undef. It is generally harmless and widely used.
if (isReservedInAllContexts(II->isReserved(Lang)))
return MD_ReservedMacro;
if (isReservedCXXAttributeName(PP, II))
return MD_ReservedAttributeIdentifier;
return MD_NoWarn;
}

Expand Down Expand Up @@ -365,6 +388,9 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef,
}
if (D == MD_ReservedMacro)
Diag(MacroNameTok, diag::warn_pp_macro_is_reserved_id);
if (D == MD_ReservedAttributeIdentifier)
Diag(MacroNameTok, diag::warn_pp_macro_is_reserved_attribute_id)
<< II->getName();
}

// Okay, we got a good identifier.
Expand Down
100 changes: 100 additions & 0 deletions clang/test/Preprocessor/macro-reserved-attrs-cxx11.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -Wreserved-attribute-identifier -fsyntax-only -verify %s -DTEST1
// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -Wreserved-attribute-identifier -fsyntax-only -verify %s -DTEST2
// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -Wreserved-attribute-identifier -fsyntax-only -verify %s -DTEST3
// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -Wreserved-attribute-identifier -fsyntax-only -verify %s -DTEST4

#ifdef TEST1

#define assume
#undef assume

#define noreturn // expected-warning {{noreturn is a reserved attribute identifier}}
#undef noreturn // expected-warning {{noreturn is a reserved attribute identifier}}

#define carries_dependency // expected-warning {{carries_dependency is a reserved attribute identifier}}
#undef carries_dependency // expected-warning {{carries_dependency is a reserved attribute identifier}}

#define deprecated // expected-warning {{deprecated is a reserved attribute identifier}}
#undef deprecated // expected-warning {{deprecated is a reserved attribute identifier}}

#define fallthrough // expected-warning {{fallthrough is a reserved attribute identifier}}
#undef fallthrough // expected-warning {{fallthrough is a reserved attribute identifier}}

#define likely // expected-warning {{likely is a reserved attribute identifier}}
#undef likely // expected-warning {{likely is a reserved attribute identifier}}

#define no_unique_address // expected-warning {{no_unique_address is a reserved attribute identifier}}
#undef no_unique_address // expected-warning {{no_unique_address is a reserved attribute identifier}}

#define unlikely // expected-warning {{unlikely is a reserved attribute identifier}}
#undef unlikely // expected-warning {{unlikely is a reserved attribute identifier}}

#define maybe_unused // expected-warning {{maybe_unused is a reserved attribute identifier}}
#undef maybe_unused // expected-warning {{maybe_unused is a reserved attribute identifier}}

#define nodiscard // expected-warning {{nodiscard is a reserved attribute identifier}}
#undef nodiscard // expected-warning {{nodiscard is a reserved attribute identifier}}

#elif TEST2

#define assume "test"
#undef assume

#define noreturn "test" // expected-warning {{noreturn is a reserved attribute identifier}}
#undef noreturn // expected-warning {{noreturn is a reserved attribute identifier}}

#define carries_dependency "test" // expected-warning {{carries_dependency is a reserved attribute identifier}}
#undef carries_dependency // expected-warning {{carries_dependency is a reserved attribute identifier}}

#define deprecated "test" // expected-warning {{deprecated is a reserved attribute identifier}}
#undef deprecated // expected-warning {{deprecated is a reserved attribute identifier}}

#define fallthrough "test" // expected-warning {{fallthrough is a reserved attribute identifier}}
#undef fallthrough // expected-warning {{fallthrough is a reserved attribute identifier}}

#define likely "test" // expected-warning {{likely is a reserved attribute identifier}}
#undef likely // expected-warning {{likely is a reserved attribute identifier}}

#define no_unique_address "test" // expected-warning {{no_unique_address is a reserved attribute identifier}}
#undef no_unique_address // expected-warning {{no_unique_address is a reserved attribute identifier}}

#define unlikely "test" // expected-warning {{unlikely is a reserved attribute identifier}}
#undef unlikely // expected-warning {{unlikely is a reserved attribute identifier}}

#define maybe_unused "test" // expected-warning {{maybe_unused is a reserved attribute identifier}}
#undef maybe_unused // expected-warning {{maybe_unused is a reserved attribute identifier}}

#define nodiscard "test" // expected-warning {{nodiscard is a reserved attribute identifier}}
#undef nodiscard // expected-warning {{nodiscard is a reserved attribute identifier}}

#elif TEST3

#define assume() "test" // expected-warning {{assume is a reserved attribute identifier}}
#define deprecated() "test" // expected-warning {{deprecated is a reserved attribute identifier}}
#define nodiscard() "test" // expected-warning {{nodiscard is a reserved attribute identifier}}
#define noreturn() "test"
#define carries_dependency() "test"
#define fallthrough() "test"
#define likely() "test"
#define no_unique_address() "test"
#define unlikely() "test"
#define maybe_unused() "test"

#elif TEST4

#define assume() // expected-warning {{assume is a reserved attribute identifier}}
#define deprecated() // expected-warning {{deprecated is a reserved attribute identifier}}
#define nodiscard() // expected-warning {{nodiscard is a reserved attribute identifier}}
#define noreturn()
#define carries_dependency()
#define fallthrough()
#define likely()
#define no_unique_address()
#define unlikely()
#define maybe_unused()

#else

#error Unknown test

#endif
Loading
Loading