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

[Timur Iskhodzhanov]

  Addressed Alexey's comments.

  Also, decided to commit the `Out.dump()` calls separately.

Hi rnk, majnemer,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D3128?vs=7977&id=7980#toc

Files:
  lib/AST/VTableBuilder.cpp
  test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
  test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp

Index: lib/AST/VTableBuilder.cpp
===================================================================
--- lib/AST/VTableBuilder.cpp
+++ lib/AST/VTableBuilder.cpp
@@ -2414,6 +2414,9 @@
   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 @@
 
   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 @@
     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(),
Index: test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -377,3 +377,45 @@
   // 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 at test4@@UAE at 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 at test4@@6BB at 1@@", [1 x i8*]** %[[VFPTR_i8]]
+
+  foo(this);
+}
+
+void destroy(C *obj) {
+  // CHECK-LABEL: define void @"\01?destroy at test4@@YAXPAUC at 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)
+}
+
+}
Index: test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
+++ 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 @@
 
 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 {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3128.2.patch
Type: text/x-patch
Size: 4856 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140320/2edfce02/attachment.bin>