For the annals of subtle but terrible bugs: fix a longstanding bug

in vtable layout where virtual methods inherited from virtual bases
could be assigned the same vcall adjustment slot if they shared
a name and parameter signature but differed in their
cv-qualification.  The code was already trying to handle this
case, but unfortunately used the ordinary type qualifiers
(which are always empty here) instead of the method qualifiers.
This seems like something that the API should discourage, but
I don't know how to carry that principle out in this instance.

Eliminate this function's need for an ASTContext while we're at it.

This bug affects the ABI, and fixing it brings us into accord with
the Itanium ABI (and GCC's implementation of it), but, obviously,
technically breaks full compatibility with previous releases of Clang.
Just letting you know.

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

diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp
index 35f84ff4834cdd108f64cd9f4e21ea70260a6569..7a459726493c61a2ae31dee0e15e3b4a28bb41a5 100644 (file)
--- a/lib/AST/VTableBuilder.cpp
+++ b/lib/AST/VTableBuilder.cpp
@@ -466,10 +466,10 @@ public:
 
 static bool HasSameVirtualSignature(const CXXMethodDecl *LHS,
                                     const CXXMethodDecl *RHS) {
-  ASTContext &C = LHS->getASTContext(); // TODO: thread this down
-  CanQual<FunctionProtoType>
-    LT = C.getCanonicalType(LHS->getType()).getAs<FunctionProtoType>(),
-    RT = C.getCanonicalType(RHS->getType()).getAs<FunctionProtoType>();
+  const FunctionProtoType *LT =
+    cast<FunctionProtoType>(LHS->getType().getCanonicalType());
+  const FunctionProtoType *RT =
+    cast<FunctionProtoType>(RHS->getType().getCanonicalType());
 
   // Fast-path matches in the canonical types.
   if (LT == RT) return true;
@@ -477,7 +477,7 @@ static bool HasSameVirtualSignature(const CXXMethodDecl *LHS,
   // Force the signatures to match.  We can't rely on the overrides
   // list here because there isn't necessarily an inheritance
   // relationship between the two methods.
-  if (LT.getQualifiers() != RT.getQualifiers() ||
+  if (LT->getTypeQuals() != RT->getTypeQuals() ||
       LT->getNumArgs() != RT->getNumArgs())
     return false;
   for (unsigned I = 0, E = LT->getNumArgs(); I != E; ++I)

diff --git a/test/CodeGenCXX/vtable-layout.cpp b/test/CodeGenCXX/vtable-layout.cpp
index bd696813c8141d52a8db935011ad494cf082e8fd..d7644b98ae09477ff4e488c601326d77098763d4 100644 (file)
--- a/test/CodeGenCXX/vtable-layout.cpp
+++ b/test/CodeGenCXX/vtable-layout.cpp
@@ -42,6 +42,7 @@
 // RUN: FileCheck --check-prefix=CHECK-41 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-42 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-43 %s < %t
+// RUN: FileCheck --check-prefix=CHECK-44 %s < %t
 
 // For now, just verify this doesn't crash.
 namespace test0 {
@@ -1701,3 +1702,28 @@ struct C : B {
 C* C::f() { return 0; }
 
 }
+
+// rdar://problem/10959710
+namespace Test38 {
+  struct A {
+    virtual void *foo();
+    virtual const void *foo() const;
+  };
+
+  // CHECK-44:      Vtable for 'Test38::B' (7 entries).
+  // CHECK-44-NEXT:    0 | vbase_offset (0)
+  // CHECK-44-NEXT:    1 | vcall_offset (0)
+  // CHECK-44-NEXT:    2 | vcall_offset (0)
+  // CHECK-44-NEXT:    3 | offset_to_top (0)
+  // CHECK-44-NEXT:    4 | Test38::B RTTI
+  // CHECK-44-NEXT:        -- (Test38::A, 0) vtable address --
+  // CHECK-44-NEXT:        -- (Test38::B, 0) vtable address --
+  // CHECK-44-NEXT:    5 | void *Test38::B::foo()
+  // CHECK-44-NEXT:    6 | const void *Test38::B::foo() const
+  class B : virtual public A {
+    void *foo();
+    const void *foo() const;
+  };
+
+  void *B::foo() { return 0; }
+}