[ItaniumMangle] Add substitutions for record types when mangling vtables

[tcwzxx]

Fix #108015

The mangleNameOrStandardSubstitution function does not add the RD type into the substitution, which causes the mangling of the <base type> to be incorrect.

[llvmbot]

@llvm/pr-subscribers-clang

Author: tcwzxx (tcwzxx)

Changes

Fix #108015

The mangleNameOrStandardSubstitution function does not add the RD type into the substitution, which causes the mangling of the &lt;base type&gt; to be incorrect.

---

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

2 Files Affected:

• (modified) clang/lib/AST/ItaniumMangle.cpp (+4-2)
• (modified) clang/test/CodeGenCXX/mangle.cpp (+24)

diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b6e1da0c3192da..962e62ef86c0ca 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -7326,11 +7326,13 @@ void ItaniumMangleContextImpl::mangleCXXCtorVTable(const CXXRecordDecl *RD,
                                                    raw_ostream &Out) {
   // <special-name> ::= TC <type> <offset number> _ <base type>
   CXXNameMangler Mangler(*this, Out);
+  QualType RDType = getASTContext().getRecordType(RD);
+  QualType TypeType = getASTContext().getRecordType(Type);
   Mangler.getStream() << "_ZTC";
-  Mangler.mangleNameOrStandardSubstitution(RD);
+  Mangler.mangleType(RDType);
   Mangler.getStream() << Offset;
   Mangler.getStream() << '_';
-  Mangler.mangleNameOrStandardSubstitution(Type);
+  Mangler.mangleType(TypeType);
 }
 
 void ItaniumMangleContextImpl::mangleCXXRTTI(QualType Ty, raw_ostream &Out) {
diff --git a/clang/test/CodeGenCXX/mangle.cpp b/clang/test/CodeGenCXX/mangle.cpp
index d0800af55c87e8..848b026028be76 100644
--- a/clang/test/CodeGenCXX/mangle.cpp
+++ b/clang/test/CodeGenCXX/mangle.cpp
@@ -11,6 +11,8 @@ struct Y { };
 //CHECK: @pr5966_i = external global
 //CHECK: @_ZL8pr5966_j = internal global
 
+//CHECK: @_ZTCN6test624InstE0_NS_1A4ImplINS1_4WrapEEE
+
 // CHECK-LABEL: define{{.*}} zeroext i1 @_ZplRK1YRA100_P1X
 bool operator+(const Y&, X* (&xs)[100]) { return false; }
 
@@ -1214,3 +1216,25 @@ namespace test61 {
   // CHECK-LABEL: @_ZN6test611fINS_1XEEEvNT_1Y1aENS3_1bE
   template void f<X>(int, int);
 }
+
+namespace test62 {
+namespace A {
+
+class VBase {
+ public:
+  virtual ~VBase() {};
+};
+
+struct Wrap {};
+
+template <typename T>
+class Impl : public virtual VBase {
+ public:
+};
+
+}  // namespace A
+
+struct Inst : public A::Impl<A::Wrap> {};
+
+void Test() { Inst a; }
+}

[mizvekov]

LGTM, Thanks!

Can you also add a release note, mentioning the potential ABI breaking change?

Please wait 24h before merging in case the other reviewers want to chime in.

[zyn0217]

Do we need an ABI flag guarding against the change as well?

[zygoloid]

Do we need an ABI flag guarding against the change as well?

Yeah, I think this change should respect -fclang-abi-compat.

[tcwzxx]

Thanks for the review. I have added the ABI version 19 and the release notes.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[zyn0217]

Some nits, otherwise LGTM as well.

[zyn0217]

Can you move these two checks directly above the corresponding codes? Thanks!

[tcwzxx]

The construction vtable is a global variable, so it cannot be moved above the corresponding code.

[zyn0217]

Suggested change

	/// - Incorrect Mangling of CXXCtorVTable
	/// - Incorrectly mangle CXX vtable substitutions in some cases...

It would be great if you could flesh out the behavior changes here.

[tcwzxx]

Thanks. Updated.