Fix incorrect codegen for devirtualized calls to virtual overloaded operators.

Consider this program:

    struct A {
      virtual void operator-() { printf("base\n"); }
    };
    struct B final : public A {
      virtual void operator-() override { printf("derived\n"); }
    };

    int main() {
      B* b = new B;
      -static_cast<A&>(*b);
    }

Before this patch, clang saw the virtual call to A::operator-(), figured out
that it can be devirtualized, and then just called A::operator-() directly,
without going through the vtable.  Instead, it should've looked up which
operator-() the call devirtualizes to and should've called that.

For regular virtual member calls, clang gets all this right already. So
instead of giving EmitCXXOperatorMemberCallee() all the logic that
EmitCXXMemberCallExpr() already has, cut the latter function into two pieces,
call the second piece EmitCXXMemberOrOperatorMemberCallExpr(), and use it also
to generate code for calls to virtual member operators.

This way, virtual overloaded operators automatically don't get devirtualized
if they have covariant returns (like it was done for regular calls in r218602),
etc.

This also happens to fix (or at least improve) codegen for explicit constructor
calls (`A a; a.A::A()`) in MS mode with -fsanitize-address-field-padding=1.

(This adjustment for virtual operator calls seems still wrong with the MS ABI.)

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

diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp
index 8138c6fc8d23fd7fde80daf6c681c5d7ce5e9499..3957eb76527bd190cb29efbb0c1b8d776fc9f112 100644 (file)
--- a/lib/CodeGen/CGClass.cpp
+++ b/lib/CodeGen/CGClass.cpp
@@ -2163,20 +2163,6 @@ CodeGenFunction::CanDevirtualizeMemberFunctionCall(const Expr *Base,
   return false;
 }
 
-llvm::Value *
-CodeGenFunction::EmitCXXOperatorMemberCallee(const CXXOperatorCallExpr *E,
-                                             const CXXMethodDecl *MD,
-                                             llvm::Value *This) {
-  llvm::FunctionType *fnType =
-    CGM.getTypes().GetFunctionType(
-                             CGM.getTypes().arrangeCXXMethodDeclaration(MD));
-
-  if (MD->isVirtual() && !CanDevirtualizeMemberFunctionCall(E->getArg(0), MD))
-    return CGM.getCXXABI().getVirtualFunctionPointer(*this, MD, This, fnType);
-
-  return CGM.GetAddrOfFunction(MD, fnType);
-}
-
 void CodeGenFunction::EmitForwardingCallToLambda(
                                       const CXXMethodDecl *callOperator,
                                       CallArgList &callArgs) {

diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index ff084de41c616914debfb9cdd853d8c5b1b0309f..4a8fe9a954c459815f380ab21d9c153996c68c90 100644 (file)
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -120,9 +120,23 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
                     ReturnValue);
   }
 
-  // Compute the object pointer.
+  bool HasQualifier = ME->hasQualifier();
+  NestedNameSpecifier *Qualifier = HasQualifier ? ME->getQualifier() : nullptr;
+  bool IsArrow = ME->isArrow();
   const Expr *Base = ME->getBase();
-  bool CanUseVirtualCall = MD->isVirtual() && !ME->hasQualifier();
+
+  return EmitCXXMemberOrOperatorMemberCallExpr(
+      CE, MD, ReturnValue, HasQualifier, Qualifier, IsArrow, Base);
+}
+
+RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(
+    const CallExpr *CE, const CXXMethodDecl *MD, ReturnValueSlot ReturnValue,
+    bool HasQualifier, NestedNameSpecifier *Qualifier, bool IsArrow,
+    const Expr *Base) {
+  assert(isa<CXXMemberCallExpr>(CE) || isa<CXXOperatorCallExpr>(CE));
+
+  // Compute the object pointer.
+  bool CanUseVirtualCall = MD->isVirtual() && !HasQualifier;
 
   const CXXMethodDecl *DevirtualizedMethod = nullptr;
   if (CanUseVirtualCall && CanDevirtualizeMemberFunctionCall(Base, MD)) {
@@ -153,7 +167,7 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
   }
 
   llvm::Value *This;
-  if (ME->isArrow())
+  if (IsArrow)
     This = EmitScalarExpr(Base);
   else
     This = EmitLValue(Base).getAddress();
@@ -165,23 +179,28 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
         cast<CXXConstructorDecl>(MD)->isDefaultConstructor())
       return RValue::get(nullptr);
 
-    if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()) {
-      // We don't like to generate the trivial copy/move assignment operator
-      // when it isn't necessary; just produce the proper effect here.
-      llvm::Value *RHS = EmitLValue(*CE->arg_begin()).getAddress();
-      EmitAggregateAssign(This, RHS, CE->getType());
-      return RValue::get(This);
-    }
+    if (!MD->getParent()->mayInsertExtraPadding()) {
+      if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()) {
+        // We don't like to generate the trivial copy/move assignment operator
+        // when it isn't necessary; just produce the proper effect here.
+        // Special case: skip first argument of CXXOperatorCall (it is "this").
+        unsigned ArgsToSkip = isa<CXXOperatorCallExpr>(CE) ? 1 : 0;
+        llvm::Value *RHS =
+            EmitLValue(*(CE->arg_begin() + ArgsToSkip)).getAddress();
+        EmitAggregateAssign(This, RHS, CE->getType());
+        return RValue::get(This);
+      }
 
-    if (isa<CXXConstructorDecl>(MD) &&
-        cast<CXXConstructorDecl>(MD)->isCopyOrMoveConstructor()) {
-      // Trivial move and copy ctor are the same.
-      assert(CE->getNumArgs() == 1 && "unexpected argcount for trivial ctor");
-      llvm::Value *RHS = EmitLValue(*CE->arg_begin()).getAddress();
-      EmitAggregateCopy(This, RHS, CE->arg_begin()->getType());
-      return RValue::get(This);
+      if (isa<CXXConstructorDecl>(MD) &&
+          cast<CXXConstructorDecl>(MD)->isCopyOrMoveConstructor()) {
+        // Trivial move and copy ctor are the same.
+        assert(CE->getNumArgs() == 1 && "unexpected argcount for trivial ctor");
+        llvm::Value *RHS = EmitLValue(*CE->arg_begin()).getAddress();
+        EmitAggregateCopy(This, RHS, CE->arg_begin()->getType());
+        return RValue::get(This);
+      }
+      llvm_unreachable("unknown trivial member function");
     }
-    llvm_unreachable("unknown trivial member function");
   }
 
   // Compute the function type we're calling.
@@ -213,13 +232,11 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
            "Destructor shouldn't have explicit parameters");
     assert(ReturnValue.isNull() && "Destructor shouldn't have return value");
     if (UseVirtualCall) {
-      CGM.getCXXABI().EmitVirtualDestructorCall(*this, Dtor, Dtor_Complete,
-                                                This, CE);
+      CGM.getCXXABI().EmitVirtualDestructorCall(
+          *this, Dtor, Dtor_Complete, This, cast<CXXMemberCallExpr>(CE));
     } else {
-      if (getLangOpts().AppleKext &&
-          MD->isVirtual() &&
-          ME->hasQualifier())
-        Callee = BuildAppleKextVirtualCall(MD, ME->getQualifier(), Ty);
+      if (getLangOpts().AppleKext && MD->isVirtual() && HasQualifier)
+        Callee = BuildAppleKextVirtualCall(MD, Qualifier, Ty);
       else if (!DevirtualizedMethod)
         Callee =
             CGM.getAddrOfCXXStructor(Dtor, StructorType::Complete, FInfo, Ty);
@@ -239,10 +256,8 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
   } else if (UseVirtualCall) {
     Callee = CGM.getCXXABI().getVirtualFunctionPointer(*this, MD, This, Ty);
   } else {
-    if (getLangOpts().AppleKext &&
-        MD->isVirtual() &&
-        ME->hasQualifier())
-      Callee = BuildAppleKextVirtualCall(MD, ME->getQualifier(), Ty);
+    if (getLangOpts().AppleKext && MD->isVirtual() && HasQualifier)
+      Callee = BuildAppleKextVirtualCall(MD, Qualifier, Ty);
     else if (!DevirtualizedMethod)
       Callee = CGM.GetAddrOfFunction(MD, Ty);
     else {
@@ -315,20 +330,9 @@ CodeGenFunction::EmitCXXOperatorMemberCallExpr(const CXXOperatorCallExpr *E,
                                                ReturnValueSlot ReturnValue) {
   assert(MD->isInstance() &&
          "Trying to emit a member call expr on a static method!");
-  LValue LV = EmitLValue(E->getArg(0));
-  llvm::Value *This = LV.getAddress();
-
-  if ((MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()) &&
-      MD->isTrivial() && !MD->getParent()->mayInsertExtraPadding()) {
-    llvm::Value *Src = EmitLValue(E->getArg(1)).getAddress();
-    QualType Ty = E->getType();
-    EmitAggregateAssign(This, Src, Ty);
-    return RValue::get(This);
-  }
-
-  llvm::Value *Callee = EmitCXXOperatorMemberCallee(E, MD, This);
-  return EmitCXXMemberOrOperatorCall(MD, Callee, ReturnValue, This,
-                                     /*ImplicitParam=*/nullptr, QualType(), E);
+  return EmitCXXMemberOrOperatorMemberCallExpr(
+      E, MD, ReturnValue, /*HasQualifier=*/false, /*Qualifier=*/nullptr,
+      /*IsArrow=*/false, E->getArg(0));
 }
 
 RValue CodeGenFunction::EmitCUDAKernelCallExpr(const CUDAKernelCallExpr *E,

diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 4575e31dbd2fc1e98f7e8bdc49d8ae5be7a8bd9b..e74abc9be6c1f70ec6bffa8ec2ac95844a089f8f 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -2329,12 +2329,16 @@ public:
                              StructorType Type);
   RValue EmitCXXMemberCallExpr(const CXXMemberCallExpr *E,
                                ReturnValueSlot ReturnValue);
+  RValue EmitCXXMemberOrOperatorMemberCallExpr(const CallExpr *CE,
+                                               const CXXMethodDecl *MD,
+                                               ReturnValueSlot ReturnValue,
+                                               bool HasQualifier,
+                                               NestedNameSpecifier *Qualifier,
+                                               bool IsArrow, const Expr *Base);
+  // Compute the object pointer.
   RValue EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E,
                                       ReturnValueSlot ReturnValue);
 
-  llvm::Value *EmitCXXOperatorMemberCallee(const CXXOperatorCallExpr *E,
-                                           const CXXMethodDecl *MD,
-                                           llvm::Value *This);
   RValue EmitCXXOperatorMemberCallExpr(const CXXOperatorCallExpr *E,
                                        const CXXMethodDecl *MD,
                                        ReturnValueSlot ReturnValue);

diff --git a/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp b/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
index 89c9a61b36ec1c76dda5ea0e876f498f40b44f50..193ee5f804060d66da5502595484fa109ff6e4f3 100644 (file)
--- a/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
+++ b/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
@@ -53,26 +53,32 @@ namespace Test3 {
 namespace Test4 {
   struct A {
     virtual void f();
+    virtual int operator-();
   };
 
   struct B final : A {
     virtual void f();
+    virtual int operator-();
   };
 
   // CHECK-LABEL: define void @_ZN5Test41fEPNS_1BE
   void f(B* d) {
     // CHECK: call void @_ZN5Test41B1fEv
     static_cast<A*>(d)->f();
+    // CHECK: call i32 @_ZN5Test41BngEv
+    -static_cast<A&>(*d);
   }
 }
 
 namespace Test5 {
   struct A {
     virtual void f();
+    virtual int operator-();
   };
 
   struct B : A {
     virtual void f();
+    virtual int operator-();
   };
 
   struct C final : B {
@@ -87,6 +93,15 @@ namespace Test5 {
     // CHECK-NEXT: call void %[[FUNC]]
     static_cast<A*>(d)->f();
   }
+  // CHECK-LABEL: define void @_ZN5Test53fopEPNS_1CE
+  void fop(C* d) {
+    // FIXME: It should be possible to devirtualize this case, but that is
+    // not implemented yet.
+    // CHECK: getelementptr
+    // CHECK-NEXT: %[[FUNC:.*]] = load
+    // CHECK-NEXT: call i32 %[[FUNC]]
+    -static_cast<A&>(*d);
+  }
 }
 
 namespace Test6 {
@@ -165,6 +180,9 @@ namespace Test9 {
     virtual A *f() {
       return 0;
     }
+    virtual A *operator-() {
+      return 0;
+    }
   };
   struct RC final : public RA {
     virtual C *f() {
@@ -173,6 +191,12 @@ namespace Test9 {
       x->b = 2;
       return x;
     }
+    virtual C *operator-() {
+      C *x = new C();
+      x->a = 1;
+      x->b = 2;
+      return x;
+    }
   };
   // CHECK: define {{.*}} @_ZN5Test91fEPNS_2RCE
   A *f(RC *x) {
@@ -187,4 +211,17 @@ namespace Test9 {
     // CHECK-NEXT: = call {{.*}} %[[FUNC]]
     return static_cast<RA*>(x)->f();
   }
+  // CHECK: define {{.*}} @_ZN5Test93fopEPNS_2RCE
+  A *fop(RC *x) {
+    // FIXME: It should be possible to devirtualize this case, but that is
+    // not implemented yet.
+    // CHECK: load
+    // CHECK: bitcast
+    // CHECK: [[F_PTR_RA:%.+]] = bitcast
+    // CHECK: [[VTABLE:%.+]] = load {{.+}} [[F_PTR_RA]]
+    // CHECK: [[VFN:%.+]] = getelementptr inbounds {{.+}} [[VTABLE]], i{{[0-9]+}} 1
+    // CHECK-NEXT: %[[FUNC:.*]] = load {{.+}} [[VFN]]
+    // CHECK-NEXT: = call {{.*}} %[[FUNC]]
+    return -static_cast<RA&>(*x);
+  }
 }