Fix bug 20116 - Miscompilation when devirtualization is aborted due to covariant return types

[rsmith]

LGTM with a more detailed testcase.

test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp

181–184 ↗

(On Diff #13610)

Can you tweak this test so that it checks the right argument is passed to the function?

[ABataev]

Richard, Ok, I'll do it

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

27.09.2014 5:40, Richard Smith пишет:

LGTM with a more detailed testcase.

Comment at: test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp:181-184
@@ -180,6 +180,6 @@

// not implemented yet.

+ // CHECK: bitcast

// CHECK: getelementptr
// CHECK-NEXT: %[[FUNC:.*]] = load

• CHECK-NEXT: bitcast CHECK-NEXT: = call {{.*}} %[[FUNC]] return static_cast<RA*>(x)->f(); ----------------

Can you tweak this test so that it checks the right argument is passed to the function?

http://reviews.llvm.org/D5321

[ABataev]

Closed by commit rL218602 (authored by @ABataev).

cfe/trunk/lib/CodeGen/CGExprCXX.cpp

RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
 
   const CXXMethodDecl *DevirtualizedMethod = nullptr;
   if (CanUseVirtualCall && CanDevirtualizeMemberFunctionCall(Base, MD)) {
     const CXXRecordDecl *BestDynamicDecl = Base->getBestDynamicClassType();
     DevirtualizedMethod = MD->getCorrespondingMethodInClass(BestDynamicDecl);
     assert(DevirtualizedMethod);
     const CXXRecordDecl *DevirtualizedClass = DevirtualizedMethod->getParent();
     const Expr *Inner = Base->ignoreParenBaseCasts();
-    if (getCXXRecord(Inner) == DevirtualizedClass)
+    if (DevirtualizedMethod->getReturnType().getCanonicalType() !=
+        MD->getReturnType().getCanonicalType())
+      // If the return types are not the same, this might be a case where more
+      // code needs to run to compensate for it. For example, the derived
+      // method might return a type that inherits form from the return
+      // type of MD and has a prefix.
+      // For now we just avoid devirtualizing these covariant cases.
+      DevirtualizedMethod = nullptr;
+    else if (getCXXRecord(Inner) == DevirtualizedClass)
       // If the class of the Inner expression is where the dynamic method
       // is defined, build the this pointer from it.
       Base = Inner;
     else if (getCXXRecord(Base) != DevirtualizedClass) {
       // If the method is defined in a class that is not the best dynamic
       // one or the one of the full expression, we would have to build
       // a derived-to-base cast to compute the correct this pointer, but
       // we don't have support for that yet, so do a virtual call.
       DevirtualizedMethod = nullptr;
     }
-    // If the return types are not the same, this might be a case where more
-    // code needs to run to compensate for it. For example, the derived
-    // method might return a type that inherits form from the return
-    // type of MD and has a prefix.
-    // For now we just avoid devirtualizing these covariant cases.
-    if (DevirtualizedMethod &&
-        DevirtualizedMethod->getReturnType().getCanonicalType() !=
-            MD->getReturnType().getCanonicalType())
-      DevirtualizedMethod = nullptr;
   }
 
   llvm::Value *This;
   if (ME->isArrow())
     This = EmitScalarExpr(Base);
   else
     This = EmitLValue(Base).getAddress();
 

cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp

virtual C *f() {
       x->b = 2;
       return x;
     }
   };
   // CHECK: define {{.*}} @_ZN5Test91fEPNS_2RCE
   A *f(RC *x) {
     // FIXME: It should be possible to devirtualize this case, but that is
     // not implemented yet.
-    // CHECK: getelementptr
-    // CHECK-NEXT: %[[FUNC:.*]] = load
-    // CHECK-NEXT: bitcast
+    // CHECK: load
+    // CHECK: bitcast
+    // CHECK: [[F_PTR_RA:%.+]] = bitcast
+    // CHECK: [[VTABLE:%.+]] = load {{.+}} [[F_PTR_RA]]
+    // CHECK: [[VFN:%.+]] = getelementptr inbounds {{.+}} [[VTABLE]], i{{[0-9]+}} 0
+    // CHECK-NEXT: %[[FUNC:.*]] = load {{.+}} [[VFN]]
     // CHECK-NEXT: = call {{.*}} %[[FUNC]]
     return static_cast<RA*>(x)->f();
   }
 }