https://github.com/asavonic created https://github.com/llvm/llvm-project/pull/136689
This patchset includes #136128, and adds a fix for regressions that were reported by @jplehr and @DKLoehr: [clang] Fix computeTypeLinkageInfo for non-record member pointers MemberPointerType can point to records, functions, or types from template parameters. computeTypeLinkageInfo used to expect only records, and crash for anything else. It seems that the compiler never executed this code path before patch https://github.com/llvm/llvm-project/pull/136128 where the issue was reported. Function member (test74): MemberPointerType 'type-parameter-0-0 (type-parameter-0-1::*)(void)' dependent |-TemplateTypeParmType 'type-parameter-0-1' dependent depth 0 index 1 `-FunctionProtoType 'type-parameter-0-0 (void)' dependent cdecl `-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0 Template parameter (test75): MemberPointerType 'type-parameter-0-1 type-parameter-0-0::*' dependent |-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0 `-TemplateTypeParmType 'type-parameter-0-1' dependent depth 0 index 1 For non-record types, it should be enough to look at a pointee type to determine linkage and visibility. LIT tests 74 and 75 are reduced from Chromium and LLVM libc test harness as reported in https://github.com/llvm/llvm-project/pull/136128. >From 677653091bf4ca585c27b1ad30edfa94fe5d0b90 Mon Sep 17 00:00:00 2001 From: Andrew Savonichev <andrew.savonic...@gmail.com> Date: Mon, 21 Apr 2025 19:45:05 +0900 Subject: [PATCH 1/2] [clang] Fix computeTypeLinkageInfo for non-record member pointers MemberPointerType can point to records, functions, or types from template parameters. computeTypeLinkageInfo used to expect only records, and crash for anything else. It seems that the compiler never executed this code path before patch #136128 where the issue was reported. Function member (test74): MemberPointerType 'type-parameter-0-0 (type-parameter-0-1::*)(void)' dependent |-TemplateTypeParmType 'type-parameter-0-1' dependent depth 0 index 1 `-FunctionProtoType 'type-parameter-0-0 (void)' dependent cdecl `-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0 Template parameter (test75): MemberPointerType 'type-parameter-0-1 type-parameter-0-0::*' dependent |-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0 `-TemplateTypeParmType 'type-parameter-0-1' dependent depth 0 index 1 For non-record types, it should be enough to look at a pointee type to determine linkage and visibility. LIT tests 74 and 75 are reduced from Chromium and LLVM libc test harness as reported in #136128. --- clang/lib/AST/Type.cpp | 6 +++-- clang/test/CodeGenCXX/visibility.cpp | 37 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index fe1dc7e2fe786..5783f87012731 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -4787,8 +4787,10 @@ LinkageInfo LinkageComputer::computeTypeLinkageInfo(const Type *T) { return computeTypeLinkageInfo(cast<ReferenceType>(T)->getPointeeType()); case Type::MemberPointer: { const auto *MPT = cast<MemberPointerType>(T); - LinkageInfo LV = - getDeclLinkageAndVisibility(MPT->getMostRecentCXXRecordDecl()); + LinkageInfo LV; + if (CXXRecordDecl *D = MPT->getMostRecentCXXRecordDecl()) { + LV.merge(getDeclLinkageAndVisibility(D)); + } LV.merge(computeTypeLinkageInfo(MPT->getPointeeType())); return LV; } diff --git a/clang/test/CodeGenCXX/visibility.cpp b/clang/test/CodeGenCXX/visibility.cpp index e1061f3dbd18f..b4c6fdcbcdf33 100644 --- a/clang/test/CodeGenCXX/visibility.cpp +++ b/clang/test/CodeGenCXX/visibility.cpp @@ -1463,3 +1463,40 @@ namespace test71 { // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i64 @_ZN6test713fooIlE3zedEv( // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIlE3barIiEET_v( } + +namespace test74 { + template <typename> struct T; + template <typename R> + struct T<void (R::*)()> { + template <typename M> + static __attribute__((__visibility__("hidden"))) void Invoke(M) { + } + }; + + struct C; + void (C::*MM)(); + + void Fun() { + T<decltype(MM)>::Invoke(0); + } + // CHECK-LABEL: define linkonce_odr void @_ZN6test741TIMNS_1CEFvvEE6InvokeIiEEvT_( + // CHECK-HIDDEN-LABEL: define linkonce_odr hidden void @_ZN6test741TIMNS_1CEFvvEE6InvokeIiEEvT_( +} + +namespace test75 { + template <class> struct T; + template <class C, class Ret> + struct T<Ret C::*> { + template <class M> + static __attribute__((__visibility__("hidden"))) + void Invoke(M) { + } + }; + + struct A; + void Fun() { + T<void (A::*)()>::Invoke(0); + } + // CHECK-LABEL: define linkonce_odr void @_ZN6test751TIMNS_1AEFvvEE6InvokeIiEEvT_( + // CHECK-HIDDEN-LABEL: define linkonce_odr hidden void @_ZN6test751TIMNS_1AEFvvEE6InvokeIiEEvT_( +} >From e0118064d9935dad6b3232c1c9503be603d92f87 Mon Sep 17 00:00:00 2001 From: Andrew Savonichev <andrew.savonic...@gmail.com> Date: Thu, 17 Apr 2025 20:18:52 +0900 Subject: [PATCH 2/2] Reland [clang] Handle instantiated members to determine visibility (#136128) As reported in issue #103477, visibility of instantiated member functions used to be ignored when calculating visibility of a specialization. This patch modifies `getLVForClassMember` to look up for a source template for an instantiated member, and changes `mergeTemplateLV` to apply it. A similar issue was reported in #31462, but it seems that `extern` declaration with visibility prevents the function from being emitted as hidden. This behavior seems correct, even though GCC emits it as with default visibility instead. Both tests from #103477 and #31462 are added as LIT tests `test72` and `test73` respectively. --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/AST/Decl.cpp | 13 +++++++-- clang/test/CodeGenCXX/visibility.cpp | 42 ++++++++++++++++++++++++++-- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 86d37f5616356..9ecd78c13d4e5 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -458,6 +458,7 @@ Bug Fixes in This Version - Fixed a crash when ``#embed`` appears as a part of a failed constant evaluation. The crashes were happening during diagnostics emission due to unimplemented statement printer. (#GH132641) +- Fixed visibility calculation for template functions. (#GH103477) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 1d9208f0e1c72..61d497999b669 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -400,9 +400,9 @@ void LinkageComputer::mergeTemplateLV( FunctionTemplateDecl *temp = specInfo->getTemplate(); // Merge information from the template declaration. LinkageInfo tempLV = getLVForDecl(temp, computation); - // The linkage of the specialization should be consistent with the - // template declaration. - LV.setLinkage(tempLV.getLinkage()); + // The linkage and visibility of the specialization should be + // consistent with the template declaration. + LV.mergeMaybeWithVisibility(tempLV, considerVisibility); // Merge information from the template parameters. LinkageInfo paramsLV = @@ -1051,6 +1051,13 @@ LinkageComputer::getLVForClassMember(const NamedDecl *D, if (const auto *redeclTemp = dyn_cast<RedeclarableTemplateDecl>(temp)) { if (isExplicitMemberSpecialization(redeclTemp)) { explicitSpecSuppressor = temp->getTemplatedDecl(); + } else if (const RedeclarableTemplateDecl *from = + redeclTemp->getInstantiatedFromMemberTemplate()) { + // If no explicit visibility is specified yet, and this is an + // instantiated member of a template, look up visibility there + // as well. + LinkageInfo fromLV = from->getLinkageAndVisibility(); + LV.mergeMaybeWithVisibility(fromLV, considerVisibility); } } } diff --git a/clang/test/CodeGenCXX/visibility.cpp b/clang/test/CodeGenCXX/visibility.cpp index b4c6fdcbcdf33..442e2a5aaa2b3 100644 --- a/clang/test/CodeGenCXX/visibility.cpp +++ b/clang/test/CodeGenCXX/visibility.cpp @@ -1457,13 +1457,49 @@ namespace test71 { // CHECK-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv( // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v( // CHECK-LABEL: define linkonce_odr hidden noundef i64 @_ZN6test713fooIlE3zedEv( - // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIlE3barIiEET_v( + // CHECK-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIlE3barIiEET_v( // CHECK-HIDDEN-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv( // CHECK-HIDDEN-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v( // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i64 @_ZN6test713fooIlE3zedEv( // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIlE3barIiEET_v( } +// https://github.com/llvm/llvm-project/issues/103477 +namespace test72 { + template <class a> + struct t { + template <int> + static HIDDEN void bar() {} + }; + + void test() { + t<char>::bar<1>(); + } + // CHECK-LABEL: define linkonce_odr hidden void @_ZN6test721tIcE3barILi1EEEvv( + // CHECK-HIDDEN-LABEL: define linkonce_odr hidden void @_ZN6test721tIcE3barILi1EEEvv( +} + +// https://github.com/llvm/llvm-project/issues/31462 +namespace test73 { + template <class T> struct s { + template <class U> + __attribute__((__visibility__("hidden"))) U should_not_be_exported(); + }; + + template <class T> template <class U> U s<T>::should_not_be_exported() { + return U(); + } + + extern template struct __attribute__((__visibility__("default"))) s<int>; + + int f() { + s<int> o; + return o.should_not_be_exported<int>(); + } + // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test731sIiE22should_not_be_exportedIiEET_v( + // CHECK-HIDDEN-LABEL: define linkonce_odr noundef i32 @_ZN6test731sIiE22should_not_be_exportedIiEET_v( +} + namespace test74 { template <typename> struct T; template <typename R> @@ -1479,7 +1515,7 @@ namespace test74 { void Fun() { T<decltype(MM)>::Invoke(0); } - // CHECK-LABEL: define linkonce_odr void @_ZN6test741TIMNS_1CEFvvEE6InvokeIiEEvT_( + // CHECK-LABEL: define linkonce_odr hidden void @_ZN6test741TIMNS_1CEFvvEE6InvokeIiEEvT_( // CHECK-HIDDEN-LABEL: define linkonce_odr hidden void @_ZN6test741TIMNS_1CEFvvEE6InvokeIiEEvT_( } @@ -1497,6 +1533,6 @@ namespace test75 { void Fun() { T<void (A::*)()>::Invoke(0); } - // CHECK-LABEL: define linkonce_odr void @_ZN6test751TIMNS_1AEFvvEE6InvokeIiEEvT_( + // CHECK-LABEL: define linkonce_odr hidden void @_ZN6test751TIMNS_1AEFvvEE6InvokeIiEEvT_( // CHECK-HIDDEN-LABEL: define linkonce_odr hidden void @_ZN6test751TIMNS_1AEFvvEE6InvokeIiEEvT_( } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
