From a8d99bb69939fb1bc9e8cf27dcf208aa7a48cc17 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Wed, 4 Sep 2024 09:13:22 -0400 Subject: [PATCH 1/2] Disallow btf_type_tag in C++ mode 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 --- clang/docs/ReleaseNotes.rst | 5 +++++ clang/lib/Sema/SemaType.cpp | 9 +++++++++ clang/test/Sema/attr-btf_type_tag.cpp | 11 +++++++++++ clang/test/SemaCXX/sugar-common-types.cpp | 7 ------- clang/test/SemaCXX/type-traits.cpp | 1 - 5 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 clang/test/Sema/attr-btf_type_tag.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index bcdbc5b702765..6fcc56f35b15d 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 7df8f663da26a..520dce870b7b7 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 0000000000000..cef78fff79b96 --- /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 e1c7578a66b9c..39a762127811f 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 bf069d9bc082c..b8a9db103782c 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)); From fae3a5b1b1bef001f87887a1f3284d45bd240b13 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Wed, 4 Sep 2024 11:57:03 -0400 Subject: [PATCH 2/2] Update release notes based on review feedback --- clang/docs/ReleaseNotes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 6fcc56f35b15d..1520f7a2916aa 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -225,8 +225,8 @@ 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 +- Clang now correctly diagnoses the use of ``btf_type_tag`` in C++ and ignores + it; this attribute is a C-only attribute, and caused crashes with template instantiation by accidentally allowing it in C++ in some circumstances. (#GH106864)