Skip to content

[Clang] Permit both gnu and clang prefixes on some attributes #125796

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 2 commits into from
Feb 5, 2025

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Feb 5, 2025

Summary:
Some attributes have gnu extensions that share names with clang
attributes. If these imply the same thing, we can specially declare this
to be an alternate but equivalent spelling. This patch enables this for
no_sanitize and provides the infrastructure for more to be added if
needed.

Discussions welcome on whether or not we want to bind ourselves to GNU
behavior, since theoretically it's possible for GNU to silently change
the semantics away from our implementation, but I'm not an expert.

Fixes: #125760

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
Some attributes have gnu extensions that share names with clang
attributes. If these imply the same thing, we can specially declare this
to be an alternate but equivalent spelling.

Discussions welcome on whether or not we want to bind ourselves to GNU
behavior, since theoretically it's possible for GNU to silently change
the semantics away from our implementation, but I'm not an expert.

Fixes: #125760


Full diff: https://github.com/llvm/llvm-project/pull/125796.diff

3 Files Affected:

  • (modified) clang/include/clang/Basic/Attr.td (+9-1)
  • (modified) clang/test/SemaCXX/attr-no-sanitize.cpp (+2-2)
  • (modified) clang/utils/TableGen/ClangAttrEmitter.cpp (+8)
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 2a3a29bd2ee1cf..bf746c2da51299 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -380,6 +380,14 @@ class Clang<string name, bit allowInC = 1, int version = 1>
   bit AllowInC = allowInC;
 }
 
+// The Clang spelling implies GNU<name>, CXX11<"clang", name>, and optionally,
+// C23<"clang", name>. This spelling should be used for any Clang-specific
+// attributes.
+class ClangGCC<string name, bit allowInC = 1, int version = 1>
+    : Spelling<name, "ClangGCC", version> {
+  bit AllowInC = allowInC;
+}
+
 // HLSL Annotation spellings
 class HLSLAnnotation<string name> : Spelling<name, "HLSLAnnotation">;
 
@@ -3677,7 +3685,7 @@ def X86ForceAlignArgPointer : InheritableAttr, TargetSpecificAttr<TargetAnyX86>
 }
 
 def NoSanitize : InheritableAttr {
-  let Spellings = [Clang<"no_sanitize">];
+  let Spellings = [ClangGCC<"no_sanitize">];
   let Args = [VariadicStringArgument<"Sanitizers">];
   let Subjects = SubjectList<[Function, ObjCMethod, GlobalVar], ErrorDiag>;
   let Documentation = [NoSanitizeDocs];
diff --git a/clang/test/SemaCXX/attr-no-sanitize.cpp b/clang/test/SemaCXX/attr-no-sanitize.cpp
index 8951f616ce0f05..cd60e71963ac30 100644
--- a/clang/test/SemaCXX/attr-no-sanitize.cpp
+++ b/clang/test/SemaCXX/attr-no-sanitize.cpp
@@ -21,8 +21,8 @@ int f3() __attribute__((no_sanitize("address")));
 
 // DUMP-LABEL: FunctionDecl {{.*}} f4
 // DUMP: NoSanitizeAttr {{.*}} hwaddress
-// PRINT: {{\[\[}}clang::no_sanitize("hwaddress")]] int f4()
-[[clang::no_sanitize("hwaddress")]] int f4();
+// PRINT: {{\[\[}}gnu::no_sanitize("hwaddress")]] int f4()
+[[gnu::no_sanitize("hwaddress")]] int f4();
 
 // DUMP-LABEL: FunctionDecl {{.*}} f5
 // DUMP: NoSanitizeAttr {{.*}} address thread hwaddress
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index de12c7062666a4..af7478b7986f92 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -108,6 +108,14 @@ GetFlattenedSpellings(const Record &Attr) {
       Ret.emplace_back("CXX11", Name, "clang", false, *Spelling);
       if (Spelling->getValueAsBit("AllowInC"))
         Ret.emplace_back("C23", Name, "clang", false, *Spelling);
+    } else if (Variety == "ClangGCC") {
+      Ret.emplace_back("GNU", Name, "", false, *Spelling);
+      Ret.emplace_back("CXX11", Name, "clang", false, *Spelling);
+      Ret.emplace_back("CXX11", Name, "gnu", false, *Spelling);
+      if (Spelling->getValueAsBit("AllowInC")) {
+        Ret.emplace_back("C23", Name, "clang", false, *Spelling);
+        Ret.emplace_back("C23", Name, "gnu", false, *Spelling);
+      }
     } else {
       Ret.push_back(FlattenedSpelling(*Spelling));
     }

Summary:
Some attributes have gnu extensions that share names with clang
attributes. If these imply the same thing, we can specially declare this
to be an alternate but equivalent spelling.

Discussions welcome on whether or not we want to bind ourselves to GNU
behavior, since theoretically it's possible for GNU to silently change
the semantics away from our implementation, but I'm not an expert.

Fixes: llvm#125760
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please be sure to add a release note to clang/docs/ReleaseNotes.rst so users know about the changes as well. Also, before landing, please change the summary -- this isn't allowing both clang and gnu on all attributes, just on no_sanitize (I was pretty confused at first!).

@jhuber6 jhuber6 changed the title [Clang] Permit both gnu and clang prefixes on attributes [Clang] Permit both gnu and clang prefixes on some attributes Feb 5, 2025
@jhuber6 jhuber6 merged commit cd754af into llvm:main Feb 5, 2025
6 of 8 checks passed
@@ -380,6 +380,13 @@ class Clang<string name, bit allowInC = 1, int version = 1>
bit AllowInC = allowInC;
}

// This spelling combines the spellings of GCC and Clang for cases where the
// spellings are equivalent for compile compatibility.
class ClangGCC<string name, bit allowInC = 1, int version = 1>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Could have probably used this for NoStackProtector, too.

NoSanitizeSpecific was almost a candidate, but the attribute has different spellings between compiler namespaces. Same for AcquireCapability.

Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…vm#125796)

Summary:
Some attributes have gnu extensions that share names with clang
attributes. If these imply the same thing, we can specially declare this
to be an alternate but equivalent spelling. This patch enables this for
`no_sanitize` and provides the infrastructure for more to be added if
needed.

Discussions welcome on whether or not we want to bind ourselves to GNU
behavior, since theoretically it's possible for GNU to silently change
the semantics away from our implementation, but I'm not an expert.

Fixes: llvm#125760
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang should accept [[gnu::no_sanitize("...")]] spelling
4 participants