Prevent devirtualization of calls to un-instantiated functions.

PR 27895

Differential Revision: https://reviews.llvm.org/D22057

llvm-svn: 305862

clang/include/clang/AST/Decl.h

@@ -1656,6 +1656,7 @@ private:
   unsigned HasImplicitReturnZero : 1;
   unsigned IsLateTemplateParsed : 1;
   unsigned IsConstexpr : 1;
+  unsigned InstantiationIsPending:1;
 
   /// \brief Indicates if the function uses __try.
   unsigned UsesSEHTry : 1;
@@ -1751,6 +1752,7 @@ protected:
         IsDeleted(false), IsTrivial(false), IsDefaulted(false),
         IsExplicitlyDefaulted(false), HasImplicitReturnZero(false),
         IsLateTemplateParsed(false), IsConstexpr(isConstexprSpecified),
+        InstantiationIsPending(false),
         UsesSEHTry(false), HasSkippedBody(false), WillHaveBody(false),
         EndRangeLoc(NameInfo.getEndLoc()), TemplateOrSpecialization(),
         DNLoc(NameInfo.getInfo()) {}
@@ -1943,6 +1945,15 @@ public:
   bool isConstexpr() const { return IsConstexpr; }
   void setConstexpr(bool IC) { IsConstexpr = IC; }
 
+  /// \brief Whether the instantiation of this function is pending.
+  /// This bit is set when the decision to instantiate this function is made
+  /// and unset if and when the function body is created. That leaves out
+  /// cases where instantiation did not happen because the template definition
+  /// was not seen in this TU. This bit remains set in those cases, under the
+  /// assumption that the instantiation will happen in some other TU.
+  bool instantiationIsPending() const { return InstantiationIsPending; }
+  void setInstantiationIsPending(bool IC) { InstantiationIsPending = IC; }
+
   /// \brief Indicates the function uses __try.
   bool usesSEHTry() const { return UsesSEHTry; }
   void setUsesSEHTry(bool UST) { UsesSEHTry = UST; }

clang/lib/CodeGen/CGClass.cpp

@@ -2770,10 +2770,19 @@ CodeGenFunction::CanDevirtualizeMemberFunctionCall(const Expr *Base,
 
   // We can devirtualize calls on an object accessed by a class member access
   // expression, since by C++11 [basic.life]p6 we know that it can't refer to
-  // a derived class object constructed in the same location.
+  // a derived class object constructed in the same location. However, we avoid
+  // devirtualizing a call to a template function that we could instantiate
+  // implicitly, but have not decided to do so. This is needed because if this
+  // function does not get instantiated, the devirtualization will create a
+  // direct call to a function whose body may not exist. In contrast, calls to
+  // template functions that are not defined in this TU are allowed to be 
+  // devirtualized under assumption that it is user responsibility to
+  // instantiate them in some other TU.
   if (const MemberExpr *ME = dyn_cast<MemberExpr>(Base))
     if (const ValueDecl *VD = dyn_cast<ValueDecl>(ME->getMemberDecl()))
-      return VD->getType()->isRecordType();
+      return VD->getType()->isRecordType() && 
+             (MD->instantiationIsPending() || MD->isDefined() ||
+               !MD->isImplicitlyInstantiable());
 
   // Likewise for calls on an object accessed by a (non-reference) pointer to
   // member access.

clang/lib/Sema/Sema.cpp

@@ -740,6 +740,9 @@ void Sema::ActOnEndOfTranslationUnit() {
       // Load pending instantiations from the external source.
       SmallVector<PendingImplicitInstantiation, 4> Pending;
       ExternalSource->ReadPendingInstantiations(Pending);
+      for (auto PII : Pending)
+        if (auto Func = dyn_cast<FunctionDecl>(PII.first))
+          Func->setInstantiationIsPending(true);
       PendingInstantiations.insert(PendingInstantiations.begin(),
                                    Pending.begin(), Pending.end());
     }

clang/lib/Sema/SemaExpr.cpp

@@ -13732,6 +13732,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
         // call to such a function.
         InstantiateFunctionDefinition(PointOfInstantiation, Func);
       else {
+        Func->setInstantiationIsPending(true);
         PendingInstantiations.push_back(std::make_pair(Func,
                                                        PointOfInstantiation));
         // Notify the consumer that a function was implicitly instantiated.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -3782,6 +3782,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
       // Try again at the end of the translation unit (at which point a
       // definition will be required).
       assert(!Recursive);
+      Function->setInstantiationIsPending(true);
       PendingInstantiations.push_back(
         std::make_pair(Function, PointOfInstantiation));
     } else if (TSK == TSK_ImplicitInstantiation) {
@@ -3801,6 +3802,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   // Postpone late parsed template instantiations.
   if (PatternDecl->isLateTemplateParsed() &&
       !LateTemplateParser) {
+    Function->setInstantiationIsPending(true);
     PendingInstantiations.push_back(
       std::make_pair(Function, PointOfInstantiation));
     return;
@@ -5146,6 +5148,8 @@ void Sema::PerformPendingInstantiations(bool LocalOnly) {
                                 TSK_ExplicitInstantiationDefinition;
       InstantiateFunctionDefinition(/*FIXME:*/Inst.second, Function, true,
                                     DefinitionRequired, true);
+      if (Function->isDefined())
+        Function->setInstantiationIsPending(false);
       continue;
     }
 

clang/test/CodeGen/no-devirt.cpp

@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 %s -DUSEIT -triple %itanium_abi_triple -emit-llvm -o - |  FileCheck %s
+
+// Test with decls and template defs in pch, and just use in .cpp
+// RUN:  %clang_cc1 %s -DTMPL_DEF_IN_HEADER -triple %itanium_abi_triple -emit-pch -o %t
+// RUN:  %clang_cc1 %s -DTMPL_DEF_IN_HEADER -DUSEIT -triple %itanium_abi_triple -include-pch %t -emit-llvm -o - | FileCheck %s
+
+// Test with A in pch, and B and C in main
+// Test with just decls in pch, and template defs and use in .cpp
+// RUN:  %clang_cc1 %s -triple %itanium_abi_triple -emit-pch -o %t
+// RUN:  %clang_cc1 %s -DUSEIT -triple %itanium_abi_triple -include-pch %t -emit-llvm -o - | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+template < typename T, int N = 0 > class TmplWithArray {
+public:
+  virtual T& operator [] (int idx);
+  virtual T& func1 (int idx);
+  virtual T& func2 (int idx);
+  T ar[N+1];
+};
+struct Wrapper {
+  TmplWithArray<bool, 10> data;
+  bool indexIt(int a) {
+    if (a > 6) return data[a] ;      // Should not devirtualize
+    if (a > 4) return data.func1(a); // Should devirtualize
+    return data.func2(a);            // Should devirtualize
+  }
+};
+
+#ifdef TMPL_DEF_IN_HEADER
+template <typename T, int N> T& TmplWithArray<T, N >::operator[](int idx) {
+  return ar[idx];
+}
+template <typename T, int N> T& TmplWithArray<T, N >::func1(int idx) {
+  return ar[idx];
+}
+#endif // TMPL_DEF_IN_HEADER
+#endif // HEADER
+
+#ifdef USEIT
+#ifndef TMPL_DEF_IN_HEADER
+template <typename T, int N> T& TmplWithArray<T, N >::operator[](int idx) {
+  return ar[idx];
+}
+template <typename T, int N> T& TmplWithArray<T, N >::func1(int idx) {
+  return ar[idx];
+}
+#endif // !TMPL_DEF_IN_HEADER
+extern Wrapper ew;
+bool stuff(int p)
+{
+  return ew.indexIt(p);
+}
+#endif
+
+// CHECK-NOT: call {{.*}} @_ZN13TmplWithArrayIbLi10EEixEi
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func1Ei
+// CHECK-DAG: call {{.*}} @_ZN13TmplWithArrayIbLi10EE5func2Ei
+