llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Chuanqi Xu (ChuanqiXu9) <details> <summary>Changes</summary> Close https://github.com/llvm/llvm-project/issues/71196 The root cause of the problem is, ThinLTO may localize linkonce vtable but there is an optimization to assume the address of vtable is unique to optimize dynamic_cast. The patch tries to solve the problem by not marking linkonce vtable as unique --- Full diff: https://github.com/llvm/llvm-project/pull/197855.diff 6 Files Affected: - (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+9-1) - (added) clang/test/CodeGenCXX/GH71196.cpp (+17) - (modified) clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp (-1) - (added) clang/test/CodeGenCXX/dynamic-cast-exact-weak-vtable.cpp (+21) - (modified) clang/test/CodeGenCXX/dynamic-cast-exact.cpp (+15-6) - (modified) clang/test/CodeGenCXX/ptrauth-dynamic-cast-exact.cpp (+12-3) ``````````diff diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 6069d39f520ef..b6bab4a6a927b 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -201,6 +201,7 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { /// practice in some cases due to language extensions. bool hasUniqueVTablePointer(QualType RecordTy) { const CXXRecordDecl *RD = RecordTy->getAsCXXRecordDecl(); + llvm::GlobalVariable::LinkageTypes Linkage = CGM.getVTableLinkage(RD); // Under -fapple-kext, multiple definitions of the same vtable may be // emitted. @@ -215,9 +216,16 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { // If there's only one definition of the vtable in the program, it has a // unique address. - if (!llvm::GlobalValue::isWeakForLinker(CGM.getVTableLinkage(RD))) + if (!llvm::GlobalValue::isWeakForLinker(Linkage)) return true; + // Header-defined classes can have a linkonce_odr vtable. ThinLTO can + // localize those weak definitions in another linkage unit, so the vtable + // address is not a reliable exact dynamic_cast discriminator. + // See https://github.com/llvm/llvm-project/issues/71196 for example. + if (Linkage == llvm::GlobalValue::LinkOnceODRLinkage) + return false; + // Even if there are multiple definitions of the vtable, they are required // by the ABI to use the same symbol name, so should be merged at load // time. However, if the class has hidden visibility, there can be diff --git a/clang/test/CodeGenCXX/GH71196.cpp b/clang/test/CodeGenCXX/GH71196.cpp new file mode 100644 index 0000000000000..245fd8a99849d --- /dev/null +++ b/clang/test/CodeGenCXX/GH71196.cpp @@ -0,0 +1,17 @@ +// RUN: %clangxx --target=x86_64-unknown-linux-gnu -O1 -fPIC -emit-llvm -S -std=c++11 -o - %s | FileCheck %s --check-prefixes=CHECK + +struct Foo { + virtual ~Foo(); +}; + +struct Bar final : Foo {}; + +Bar *make() { return new Bar(); } + +// CHECK: @_ZTV3Bar = linkonce_odr{{.*}}constant + +// CHECK-LABEL: define {{.*}} @_Z4castP3Foo +Bar *cast(Foo *f) { + // CHECK: call {{.*}} @__dynamic_cast(ptr nonnull %[[ARG:.*]], ptr nonnull @_ZTI3Foo, ptr nonnull @_ZTI3Bar, i64 0) + return dynamic_cast<Bar *>(f); +} diff --git a/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp b/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp index bf202d14c3398..8c841247c8032 100644 --- a/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp +++ b/clang/test/CodeGenCXX/dynamic-cast-exact-disabled.cpp @@ -1,4 +1,3 @@ -// RUN: %clang_cc1 -I%S %s -triple x86_64-apple-darwin10 -O1 -emit-llvm -std=c++11 -o - | FileCheck %s --check-prefixes=CHECK,EXACT // RUN: %clang_cc1 -I%S %s -triple x86_64-apple-darwin10 -O0 -emit-llvm -std=c++11 -o - | FileCheck %s --check-prefixes=CHECK,INEXACT // RUN: %clang_cc1 -I%S %s -triple x86_64-apple-darwin10 -O1 -fvisibility=hidden -emit-llvm -std=c++11 -o - | FileCheck %s --check-prefixes=CHECK,INEXACT // RUN: %clang_cc1 -I%S %s -triple x86_64-apple-darwin10 -O1 -fapple-kext -emit-llvm -std=c++11 -o - | FileCheck %s --check-prefixes=CHECK,INEXACT diff --git a/clang/test/CodeGenCXX/dynamic-cast-exact-weak-vtable.cpp b/clang/test/CodeGenCXX/dynamic-cast-exact-weak-vtable.cpp new file mode 100644 index 0000000000000..2ae3a0dcdaf50 --- /dev/null +++ b/clang/test/CodeGenCXX/dynamic-cast-exact-weak-vtable.cpp @@ -0,0 +1,21 @@ +// RUN: %clangxx --target=x86_64-unknown-linux-gnu -O1 -fPIC -emit-llvm -S -std=c++11 -o - %s | FileCheck %s --check-prefixes=CHECK,INEXACT + +struct Foo { + virtual ~Foo(); +}; + +// Header-defined final classes have a weak_odr/linkonce_odr vtable. In the +// ThinLTO shared-library reproducer for issue #71196, using that vtable as a +// unique identity for exact dynamic_cast is wrong because another linkage unit +// can end up with a different local copy. +struct Bar final : Foo {}; + +Bar *make() { return new Bar(); } + +// CHECK: @_ZTV3Bar = linkonce_odr{{.*}}constant + +// CHECK-LABEL: define {{.*}} @_Z4castP3Foo +Bar *cast(Foo *f) { + // INEXACT: call {{.*}} @__dynamic_cast(ptr nonnull %[[ARG:.*]], ptr nonnull @_ZTI3Foo, ptr nonnull @_ZTI3Bar, i64 0) + return dynamic_cast<Bar *>(f); +} diff --git a/clang/test/CodeGenCXX/dynamic-cast-exact.cpp b/clang/test/CodeGenCXX/dynamic-cast-exact.cpp index 588d80844a2fa..afddb22b74f93 100644 --- a/clang/test/CodeGenCXX/dynamic-cast-exact.cpp +++ b/clang/test/CodeGenCXX/dynamic-cast-exact.cpp @@ -1,15 +1,21 @@ // RUN: %clang_cc1 -I%S %s -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions -std=c++11 -o - -O1 -disable-llvm-passes | FileCheck %s --implicit-check-not='call {{.*}} @__dynamic_cast' struct Offset { virtual ~Offset(); }; +Offset::~Offset() { } struct A { virtual ~A(); }; -struct B final : Offset, A { }; +A::~A() { } +struct B final : Offset, A { virtual ~B(); }; +B::~B() { } struct C { virtual ~C(); int c; }; +C::~C() { } struct D : A { int d; }; struct E : A { int e; }; struct F : virtual A { int f; }; struct G : virtual A { int g; }; -struct H final : C, D, E, F, G { int h; }; -struct H1 final: C, private D { int h1; }; +struct H final : C, D, E, F, G { int h; virtual ~H(); }; +H::~H() { } +struct H1 final: C, private D { int h1; virtual ~H1(); }; +H1::~H1() { } // CHECK-LABEL: @_Z7inexactP1A C *inexact(A *a) { @@ -103,7 +109,8 @@ H1 &exact_invalid_multi(D& d) { namespace GH137518 { class base { virtual void fn() = 0; }; - class test final : base { virtual void fn() { } }; + class test final : base { virtual void fn(); }; + void test::fn() {} test* new_test() { return new test(); } // CHECK-LABEL: @_ZN8GH1375184castEPNS_4baseE( @@ -120,8 +127,10 @@ namespace GH137518 { namespace GH64088 { // Ensure we mark the B vtable as used here, because we're going to emit a // reference to it. - // CHECK: define {{.*}} void @_ZN7GH640881BD0Ev( + // CHECK: define {{.*}}void @_ZN7GH640881BD0Ev( struct A { virtual ~A(); }; - struct B final : A { virtual ~B() = default; }; + struct B final : A { virtual ~B(); }; + B::~B() { } + B *cast(A *p) { return dynamic_cast<B*>(p); } } diff --git a/clang/test/CodeGenCXX/ptrauth-dynamic-cast-exact.cpp b/clang/test/CodeGenCXX/ptrauth-dynamic-cast-exact.cpp index 1710ca5563380..8390f5f8e9f45 100644 --- a/clang/test/CodeGenCXX/ptrauth-dynamic-cast-exact.cpp +++ b/clang/test/CodeGenCXX/ptrauth-dynamic-cast-exact.cpp @@ -3,29 +3,35 @@ struct A { virtual ~A(); }; +A::~A() {} struct B { int foo; virtual ~B(); }; struct C final : A, B { - virtual void f(){}; + virtual void f(); }; +void C::f() {} struct D final : B, A { - virtual void f(){}; + virtual void f(); }; +void D::f() {} struct Offset { virtual ~Offset(); }; +Offset::~Offset() {} struct E { virtual ~E(); }; +E::~E() {} struct F final : Offset, E { }; struct G { virtual ~G(); int g; }; +G::~G() {} struct H : E { int h; }; @@ -40,8 +46,11 @@ struct K : virtual E { }; struct L final : G, H, I, J, K { int l; + virtual ~L(); }; -struct M final: G, private H { int m; }; +L::~L() {} +struct M final: G, private H { int m; ~M(); }; +M::~M() {} // CHECK-LABEL: @_Z10exact_to_CP1A C *exact_to_C(A *a) { `````````` </details> https://github.com/llvm/llvm-project/pull/197855 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
