From: Timur Iskhodzhanov <timurrrr@google.com>
Date: Thu, 20 Mar 2014 20:38:34 +0000 (+0000)
Subject: Fix PR19172 - wrong this adjustment calculated for virtual destructor in a class... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dad9d115806048b723a08155cfa495f063710ee0;p=clang

Fix PR19172 - wrong this adjustment calculated for virtual destructor in a class with complex inheritance

Reviewed at http://llvm-reviews.chandlerc.com/D3128

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@204394 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp
index 0137234117..12c5b8f687 100644
--- a/lib/AST/VTableBuilder.cpp
+++ b/lib/AST/VTableBuilder.cpp
@@ -2414,6 +2414,9 @@ public:
   typedef llvm::DenseMap<GlobalDecl, MethodVFTableLocation>
     MethodVFTableLocationsTy;
 
+  typedef llvm::iterator_range<MethodVFTableLocationsTy::const_iterator>
+    method_locations_range;
+
 private:
   /// VTables - Global vtable information.
   MicrosoftVTableContext &VTables;
@@ -2576,12 +2579,9 @@ public:
 
   ThunksMapTy::const_iterator thunks_end() const { return Thunks.end(); }
 
-  MethodVFTableLocationsTy::const_iterator vtable_indices_begin() const {
-    return MethodVFTableLocations.begin();
-  }
-
-  MethodVFTableLocationsTy::const_iterator vtable_indices_end() const {
-    return MethodVFTableLocations.end();
+  method_locations_range vtable_locations() const {
+    return method_locations_range(MethodVFTableLocations.begin(),
+                                  MethodVFTableLocations.end());
   }
 
   uint64_t getNumVTableComponents() const { return Components.size(); }
@@ -3291,9 +3291,15 @@ void MicrosoftVTableContext::computeVTableRelatedInformation(
     VFTableLayouts[id] = new VTableLayout(
         Builder.getNumVTableComponents(), Builder.vtable_component_begin(),
         VTableThunks.size(), VTableThunks.data(), EmptyAddressPointsMap, true);
-    NewMethodLocations.insert(Builder.vtable_indices_begin(),
-                              Builder.vtable_indices_end());
     Thunks.insert(Builder.thunks_begin(), Builder.thunks_end());
+
+    for (const auto &I : Builder.vtable_locations()) {
+      GlobalDecl GD = I.first;
+      MethodVFTableLocation NewLoc = I.second;
+      auto M = NewMethodLocations.find(GD);
+      if (M == NewMethodLocations.end() || NewLoc < M->second)
+        NewMethodLocations[GD] = NewLoc;
+    }
   }
 
   MethodVFTableLocations.insert(NewMethodLocations.begin(),
diff --git a/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp b/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
index 22e1fa71ec..23cc70994b 100644
--- a/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ b/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -377,3 +377,45 @@ void D::bar() {
   // CHECK: ret
 }
 }
+
+namespace test4{
+// PR19172: We used to merge method vftable locations wrong.
+
+struct A {
+  virtual ~A() {}
+};
+
+struct B {
+  virtual ~B() {}
+};
+
+struct C : virtual A, B {
+  virtual ~C();
+};
+
+void foo(void*);
+
+C::~C() {
+  // CHECK-LABEL: define x86_thiscallcc void @"\01??1C@test4@@UAE@XZ"(%"struct.test4::C"* %this)
+
+  // In this case "this" points to the most derived class, so no GEPs needed.
+  // CHECK-NOT: getelementptr
+  // CHECK-NOT: bitcast
+  // CHECK: %[[VFPTR_i8:.*]] = bitcast %"struct.test4::C"* %{{.*}} to [1 x i8*]**
+  // CHECK: store [1 x i8*]* @"\01??_7C@test4@@6BB@1@@", [1 x i8*]** %[[VFPTR_i8]]
+
+  foo(this);
+}
+
+void destroy(C *obj) {
+  // CHECK-LABEL: define void @"\01?destroy@test4@@YAXPAUC@1@@Z"(%"struct.test4::C"* %obj)
+
+  delete obj;
+  // CHECK: %[[VPTR:.*]] = bitcast %"struct.test4::C"* %[[OBJ:.*]] to void (%"struct.test4::C"*, i32)***
+  // CHECK: %[[VFTABLE:.*]] = load void (%"struct.test4::C"*, i32)*** %[[VPTR]]
+  // CHECK: %[[VFTENTRY:.*]] = getelementptr inbounds void (%"struct.test4::C"*, i32)** %[[VFTABLE]], i64 0
+  // CHECK: %[[VFUN:.*]] = load void (%"struct.test4::C"*, i32)** %[[VFTENTRY]]
+  // CHECK: call x86_thiscallcc void %[[VFUN]](%"struct.test4::C"* %[[OBJ]], i32 1)
+}
+
+}
diff --git a/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp b/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
index 82eac8a047..5e05d15136 100644
--- a/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
+++ b/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
@@ -21,6 +21,7 @@
 // RUN: FileCheck --check-prefix=VDTORS-U %s < %t
 // RUN: FileCheck --check-prefix=VDTORS-V %s < %t
 // RUN: FileCheck --check-prefix=VDTORS-P %s < %t
+// RUN: FileCheck --check-prefix=VDTORS-R %s < %t
 // RUN: FileCheck --check-prefix=RET-W %s < %t
 // RUN: FileCheck --check-prefix=RET-T %s < %t
 // RUN: FileCheck --check-prefix=RET-V %s < %t
@@ -543,6 +544,30 @@ struct P : T, Y {
 
 P p;
 
+struct Q {
+  virtual ~Q();
+};
+
+// PR19172: Yet another diamond we miscompiled.
+struct R : virtual Q, X {
+  // VDTORS-R: VFTable for 'vdtors::Q' in 'vdtors::R' (1 entry).
+  // VDTORS-R-NEXT: 0 | vdtors::R::~R() [scalar deleting]
+  // VDTORS-R-NEXT:     [this adjustment: -8 non-virtual]
+
+  // VDTORS-R: Thunks for 'vdtors::R::~R()' (1 entry).
+  // VDTORS-R-NEXT: 0 | [this adjustment: -8 non-virtual]
+
+  // VDTORS-R: VFTable for 'vdtors::X' in 'vdtors::R' (2 entries).
+  // VDTORS-R-NEXT: 0 | vdtors::R::~R() [scalar deleting]
+  // VDTORS-R-NEXT: 1 | void vdtors::X::zzz()
+
+  // VDTORS-R: VFTable indices for 'vdtors::R' (1 entry).
+  // VDTORS-R-NEXT: 0 | vdtors::R::~R() [scalar deleting]
+  virtual ~R();
+};
+
+R r;
+
 }
 
 namespace return_adjustment {