Skip to content

Disallow btf_type_tag in C++ mode #107238

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
Sep 4, 2024

Conversation

AaronBallman
Copy link
Collaborator

This was always intended to be disallowed in C++ (see the definition in Attr.td), but failed to add the correct checking code in SemaType.cpp to ensure it was rejected.

Fixes #106864

This was always intended to be disallowed in C++ (see the definition in
Attr.td), but failed to add the correct checking code in SemaType.cpp
to ensure it was rejected.

Fixes llvm#106864
@AaronBallman AaronBallman added clang:frontend Language frontend issues, e.g. anything involving "Sema" false-negative labels Sep 4, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Sep 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2024

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

This was always intended to be disallowed in C++ (see the definition in Attr.td), but failed to add the correct checking code in SemaType.cpp to ensure it was rejected.

Fixes #106864


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+5)
  • (modified) clang/lib/Sema/SemaType.cpp (+9)
  • (added) clang/test/Sema/attr-btf_type_tag.cpp (+11)
  • (modified) clang/test/SemaCXX/sugar-common-types.cpp (-7)
  • (modified) clang/test/SemaCXX/type-traits.cpp (-1)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bcdbc5b702765e..6fcc56f35b15de 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -225,6 +225,11 @@ Attribute Changes in Clang
   more cases where the returned reference outlives the object.
   (#GH100567)
 
+- Now correctly diagnosing use of ``btf_type_tag`` in C++ and ignoring it; this
+  attribute is a C-only attribute, and was causing crashes with template
+  instantiation by accidentally allowing it in C++ in some circumstances.
+  (#GH106864)
+
 Improvements to Clang's diagnostics
 -----------------------------------
 
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 7df8f663da26a5..520dce870b7b78 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6490,6 +6490,15 @@ static void HandleBTFTypeTagAttribute(QualType &Type, const ParsedAttr &Attr,
                                       TypeProcessingState &State) {
   Sema &S = State.getSema();
 
+  // This attribute is only supported in C.
+  // FIXME: we should implement checkCommonAttributeFeatures() in SemaAttr.cpp
+  // such that it handles type attributes, and then call that from
+  // processTypeAttrs() instead of one-off checks like this.
+  if (!Attr.diagnoseLangOpts(S)) {
+    Attr.setInvalid();
+    return;
+  }
+
   // Check the number of attribute arguments.
   if (Attr.getNumArgs() != 1) {
     S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments)
diff --git a/clang/test/Sema/attr-btf_type_tag.cpp b/clang/test/Sema/attr-btf_type_tag.cpp
new file mode 100644
index 00000000000000..cef78fff79b963
--- /dev/null
+++ b/clang/test/Sema/attr-btf_type_tag.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c -x c %s
+
+// c-no-diagnostics
+
+// Ensure that we diagnose the attribute as ignored in C++ but not in C.
+#ifdef __cplusplus
+static_assert(__builtin_is_implicit_lifetime(int __attribute__((btf_type_tag("user"))) *)); // expected-warning {{'btf_type_tag' attribute ignored}}
+#endif
+int __attribute__((btf_type_tag("user"))) *ptr; // expected-warning {{'btf_type_tag' attribute ignored}}
+
diff --git a/clang/test/SemaCXX/sugar-common-types.cpp b/clang/test/SemaCXX/sugar-common-types.cpp
index e1c7578a66b9ca..39a762127811ff 100644
--- a/clang/test/SemaCXX/sugar-common-types.cpp
+++ b/clang/test/SemaCXX/sugar-common-types.cpp
@@ -90,13 +90,6 @@ N t19 = 0 ? (__underlying_type(EnumsX::X)){} : (__underlying_type(EnumsY::Y)){};
 N t20 = 0 ? (__underlying_type(EnumsX::X)){} : (__underlying_type(EnumsY::X)){};
 // expected-error@-1 {{rvalue of type '__underlying_type(Enums::X)' (aka 'int')}}
 
-using SBTF1 = SS1 [[clang::btf_type_tag("1")]];
-using SBTF2 = ::SS1 [[clang::btf_type_tag("1")]];
-using SBTF3 = ::SS1 [[clang::btf_type_tag("2")]];
-
-N t21 = 0 ? (SBTF1){} : (SBTF3){}; // expected-error {{from 'SS1'}}
-N t22 = 0 ? (SBTF1){} : (SBTF2){}; // expected-error {{from 'SS1 __attribute__((btf_type_tag("1")))' (aka 'SS1')}}
-
 using QX = const SB1 *;
 using QY = const ::SB1 *;
 N t23 = 0 ? (QX){} : (QY){}; // expected-error {{rvalue of type 'const SB1 *' (aka 'const SS1 *')}}
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index bf069d9bc082c3..b8a9db103782c3 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -2052,7 +2052,6 @@ void is_implicit_lifetime(int n) {
   static_assert(__builtin_is_implicit_lifetime(float4));
   static_assert(__builtin_is_implicit_lifetime(align_value_int));
   static_assert(__builtin_is_implicit_lifetime(int[[clang::annotate_type("category2")]] *));
-  static_assert(__builtin_is_implicit_lifetime(int __attribute__((btf_type_tag("user"))) *));
   static_assert(__builtin_is_implicit_lifetime(EnforceReadOnlyPlacement));
   static_assert(__builtin_is_implicit_lifetime(int __attribute__((noderef)) *));
   static_assert(__builtin_is_implicit_lifetime(TypeVisibility));

@AaronBallman AaronBallman merged commit 7e03753 into llvm:main Sep 4, 2024
6 of 8 checks passed
@AaronBallman AaronBallman deleted the aballman-gh106864 branch September 4, 2024 16:33
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 false-negative
Projects
None yet
3 participants