r225761 - Mark vtable used on explicit destructor definitions.

[Nico Weber]

Author: nico
Date: Mon Jan 12 21:52:11 2015
New Revision: 225761

URL: http://llvm.org/viewvc/llvm-project?rev=225761&view=rev
Log:
Mark vtable used on explicit destructor definitions.

There are two things in a C++ program that need to read the vtable pointer:
Constructors and destructors.  (A few other operations -- virtual calls,
dynamic cast, rtti -- read the vtable pointer off a this pointer, but for
this they don't need the vtable symbol.)  Implicit constructors and destructors
and explicit constructors already marked the vtable as used, but explicit
destructors didn't.

Note that the only thing sema's "mark a class's vtable used" does is to mark all 
final overriders of the class as referenced, it does _not_ cause emission of
the vtable itself.  This is done on demand by codegen, independent of sema,
since sema might emit functions that are not referenced.  (The exception are
vtables that are forced via key functions -- these are forced onto codegen
by sema.)

This bug went unnoticed for years because it doesn't have observable effects
(yet -- I want to change this in PR20337, which is why I noticed this).

r213109 made it so that _calls_ to constructors don't mark the vtable used.
Currently, _calls_ to destructors still mark the vtable used.  If that
wasn't the case, this program would tickle the problem:

  test.h:
    template <typename T>
    struct B {
      int* p;
      virtual ~B() { delete p; }
      virtual void f() {}
    };

    struct __attribute__((visibility("default"))) C {
      C();
      B<int> m;
    };

  test2.cc:
    #include "test.h"
    int main() {
      C* c = new C;
      delete c;
    }

  test3.cc:
    #include "test.h"
    C::C() {}

  # This bin/clang++ binary doesn't MarkVTableUsed() for virtual dtor calls:
  $ bin/clang++ -shared test3.cc -std=c++11 -O2  -fvisibility=hidden \
        -fvisibility-inlines-hidden  -o libtest3.dylib
  $ bin/clang++ test2.cc -std=c++11 -O2  -fvisibility=hidden \
        -fvisibility-inlines-hidden  libtest3.dylib 
  Undefined symbols for architecture x86_64:
    "B<int>::f()", referenced from:
        vtable for B<int> in test2-af8f4f.o
  ld: symbol(s) not found for architecture x86_64

What's happening here is that there's a copy of B's vtable hidden in
libtest3.dylib, because C's constructor caused an implicit instantiation of that
(and implicit constructors generate vtables).
test2.cc calls C's destructDr, which destroys the B<int> member,
which wants to overwrite the vtable back to B (think of B as the base of a class
hierarchy, and of hierarchical destruction -- maybe we shouldn't do the vtable
writing in destructors of final classes), but there's nothing in test2.cc that
marks B's vtable used.  So codegen writes out the vtable, but since it wasn't
marked used, sema didn't mark all the virtual functions (in particular f())
as used.

Note that this change makes us reject programs we didn't reject before (see
the included Sema test case), but both gcc and cl also reject this code, and
clang used to reject it before r213109.


Modified:
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=225761&r1=225760&r2=225761&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Jan 12 21:52:11 2015
@@ -10481,9 +10481,11 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
       DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(), FD->param_end(),
                                              FD->getReturnType(), FD);
 
-      // If this is a constructor, we need a vtable.
+      // If this is a structor, we need a vtable.
       if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(FD))
         MarkVTableUsed(FD->getLocation(), Constructor->getParent());
+      else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(FD))
+        MarkVTableUsed(FD->getLocation(), Destructor->getParent());
       
       // Try to apply the named return value optimization. We have to check
       // if we can do this here because lambdas keep return statements around

Modified: cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp?rev=225761&r1=225760&r2=225761&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp (original)
+++ cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp Mon Jan 12 21:52:11 2015
@@ -25,6 +25,24 @@ template<>
 void X<int>::f() { }
 }
 
+// Like PR5557, but with a defined destructor instead of a defined constructor.
+namespace PR5557_dtor {
+template <class T> struct A {
+  A(); // Don't have an implicit constructor.
+  ~A(); // expected-note{{instantiation}}
+  virtual int a(T x);
+};
+template<class T> A<T>::~A() {}
+
+template<class T> int A<T>::a(T x) { 
+  return *x; // expected-error{{requires pointer operand}}
+}
+
+void f() {
+  A<int> x; // expected-note{{instantiation}}
+}
+}
+
 template<typename T>
 struct Base {
   virtual ~Base() {