Skip to content

[ItaniumMangle] Make sure class types are added to the dictionary of substitution candidates when compiling for older ABIs #138947

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.

Sign up for GitHub

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 4 commits into from
May 28, 2025
Merged

[ItaniumMangle] Make sure class types are added to the dictionary of substitution candidates when compiling for older ABIs #138947

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ece5ba2
[ItaniumMangle] Make sure class types are added to the dictionary of
ahatanak May 7, 2025
1856a60
Move test to clang-abi-compat.cpp
ahatanak May 8, 2025
56e7708
Avoid adding the record to the dictionary only in mangleCXXCtorVTable
ahatanak May 12, 2025
c7d9fde
Address review comments
ahatanak May 27, 2025
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
19 changes: 12 additions & 7 deletions clang/lib/AST/ItaniumMangle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ class CXXNameMangler {
void mangleSeqID(unsigned SeqID);
void mangleName(GlobalDecl GD);
void mangleType(QualType T);
void mangleCXXRecordDecl(const CXXRecordDecl *Record);
void mangleCXXRecordDecl(const CXXRecordDecl *Record,
bool IsManglingVTable = false);
void mangleLambdaSig(const CXXRecordDecl *Lambda);
void mangleModuleNamePrefix(StringRef Name, bool IsPartition = false);
void mangleVendorQualifier(StringRef Name);
Expand Down Expand Up @@ -3102,11 +3103,15 @@ void CXXNameMangler::mangleType(QualType T) {
addSubstitution(T);
}

void CXXNameMangler::mangleCXXRecordDecl(const CXXRecordDecl *Record) {
void CXXNameMangler::mangleCXXRecordDecl(const CXXRecordDecl *Record,
bool IsManglingVTable) {
if (mangleSubstitution(Record))
return;
mangleName(Record);
if (isCompatibleWith(LangOptions::ClangABI::Ver19))
// If we are mangling vtables, return early without adding the record to the
// dictionary of substitution candidates to maintain compatibility with older
// ABIs.
if (IsManglingVTable && isCompatibleWith(LangOptions::ClangABI::Ver19))
Copy link
Contributor

Choose a reason for hiding this comment

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

So the previous buggy behavior is a bit interesting.

You are saying we used to mangle a substitution here, but not actually consider this site a user of the type for future substitutions?

Do you have a specific test case for that?

return;
addSubstitution(Record);
}
Expand Down Expand Up @@ -7501,15 +7506,15 @@ void ItaniumMangleContextImpl::mangleCXXVTable(const CXXRecordDecl *RD,
// <special-name> ::= TV <type> # virtual table
CXXNameMangler Mangler(*this, Out);
Mangler.getStream() << "_ZTV";
Mangler.mangleCXXRecordDecl(RD);
Mangler.mangleCXXRecordDecl(RD, /*IsManglingVTable=*/true);
}

void ItaniumMangleContextImpl::mangleCXXVTT(const CXXRecordDecl *RD,
raw_ostream &Out) {
// <special-name> ::= TT <type> # VTT structure
CXXNameMangler Mangler(*this, Out);
Mangler.getStream() << "_ZTT";
Mangler.mangleCXXRecordDecl(RD);
Mangler.mangleCXXRecordDecl(RD, /*IsManglingVTable=*/true);
}

void ItaniumMangleContextImpl::mangleCXXCtorVTable(const CXXRecordDecl *RD,
Expand All @@ -7519,10 +7524,10 @@ void ItaniumMangleContextImpl::mangleCXXCtorVTable(const CXXRecordDecl *RD,
// <special-name> ::= TC <type> <offset number> _ <base type>
CXXNameMangler Mangler(*this, Out);
Mangler.getStream() << "_ZTC";
Mangler.mangleCXXRecordDecl(RD);
Mangler.mangleCXXRecordDecl(RD, /*IsManglingVTable=*/true);
Mangler.getStream() << Offset;
Mangler.getStream() << '_';
Mangler.mangleCXXRecordDecl(Type);
Mangler.mangleCXXRecordDecl(Type, /*IsManglingVTable=*/true);
}

void ItaniumMangleContextImpl::mangleCXXRTTI(QualType Ty, raw_ostream &Out) {
Expand Down
14 changes: 14 additions & 0 deletions clang/test/CodeGenCXX/mangle-itanium-ptrauth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
// RUN: %clang_cc1 -std=c++11 -fptrauth-intrinsics -fptrauth-calls -emit-llvm -o - -triple=arm64-apple-ios %s | FileCheck %s
// RUN: %clang_cc1 -std=c++11 -fptrauth-intrinsics -fptrauth-calls -emit-llvm -o - -triple=aarch64-linux-gnu %s | FileCheck %s
// RUN: %clang_cc1 -std=c++11 -fptrauth-intrinsics -fptrauth-calls -emit-llvm -o - -triple=arm64-apple-ios -fclang-abi-compat=4 %s | FileCheck %s

// clang previously emitted an incorrect discriminator for the member function
// pointer because of a bug in the mangler.

// CHECK: @_ZN17test_substitution5funcsE = global [1 x { i64, i64 }] [{ i64, i64 } { i64 ptrtoint (ptr ptrauth (ptr @_ZN17test_substitution1S1fEPvS1_, i32 0, i64 48995) to i64), i64 0 }], align 8
namespace test_substitution {
struct S { int f(void *, void *); };

typedef int (S::*s_func)(void *, void *);

s_func funcs[] = { (s_func)(&S::f) };
}


// CHECK: define {{.*}}void @_Z3fooPU9__ptrauthILj3ELb1ELj234EEPi(
void foo(int * __ptrauth(3, 1, 234) *) {}
Expand Down
19 changes: 16 additions & 3 deletions clang/test/CodeGenCXX/mangle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -fblocks -std=c++11 | FileCheck %s
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -fblocks -std=c++11 | FileCheck --check-prefixes=CHECK,CHECK-ABI-LATEST %s
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -fblocks -std=c++11 -fclang-abi-compat=4 | FileCheck %s
struct X { };
struct Y { };

Expand Down Expand Up @@ -1176,7 +1177,7 @@ namespace test56 {
namespace test57 {
struct X { template <int N> int f(); } x;
template<int N> void f(decltype(x.f<0>() + N)) {}
// CHECK-LABEL: @_ZN6test571fILi0EEEvDTplcldtL_ZNS_1xEE1fILi0EEET_E
// CHECK-ABI-LATEST: @_ZN6test571fILi0EEEvDTplcldtL_ZNS_1xEE1fILi0EEET_E
template void f<0>(int);
}

Expand Down Expand Up @@ -1217,7 +1218,7 @@ namespace test61 {
};
};
template <typename T> void f(typename T::Y::a, typename T::Y::b) {}
// CHECK-LABEL: @_ZN6test611fINS_1XEEEvNT_1Y1aENS3_1bE
// CHECK-ABI-LATEST-LABEL: @_ZN6test611fINS_1XEEEvNT_1Y1aENS3_1bE
template void f<X>(int, int);
}

Expand Down Expand Up @@ -1247,3 +1248,15 @@ namespace test63 {
// CHECK-LABEL: @_ZN6test6312_GLOBAL__N_11fIiEEvNS0_4_AndINS0_17integral_constantIivEENS0_7_OrImplIXsr17integral_constantIT_iEE5valueEEEEE
void g() { f<int>({}); }
} // namespace test63

namespace test_substitution {
struct S { int f(void *, void *); };

typedef int (S::*s_func)(void *, void *);

// clang previously emitted 'S0_' for the second 'void *' parameter type because
// of a bug in the mangler.

// CHECK-LABEL: define void @_ZN17test_substitution4foo1EMNS_1SEFiPvS1_E(
void foo1(s_func s) {}
}
Loading