[CodeGen] Devirtualize calls to methods marked final in a derived class

[vsk]

Summary

If we see a virtual method call to Base::foo() but can infer that the
object is an instance of Derived, and that 'foo' is marked 'final' in
Derived, we can devirtualize the call to Derived::foo().

Note: This patch introduces a check which depends on the result of
Base->getBestDynamicClassType(). Since this API can return nullptr, I
took the opportunity to defensively group together the checks which rely
on the result. I can drop this change, or try it in a separate commit if
that would be better.

[rjmccall]

Looks great to me, thanks.

cfe/trunk/lib/CodeGen/CGClass.cpp

 bool
 CodeGenFunction::CanDevirtualizeMemberFunctionCall(const Expr *Base,
                                                    const CXXMethodDecl *MD) {
   // When building with -fapple-kext, all calls must go through the vtable since
   // the kernel linker can do runtime patching of vtables.
   if (getLangOpts().AppleKext)
     return false;
 
+  // If the member function is marked 'final', we know that it can't be
+  // overridden and can therefore devirtualize it.
+  if (MD->hasAttr<FinalAttr>())
+    return true;
+
   // If the most derived class is marked final, we know that no subclass can
   // override this member function and so we can devirtualize it. For example:
   //
   // struct A { virtual void f(); }
   // struct B final : A { };
   //
   // void f(B *b) {
   //   b->f();
   // }
   //
-  const CXXRecordDecl *MostDerivedClassDecl = Base->getBestDynamicClassType();
-  if (MostDerivedClassDecl->hasAttr<FinalAttr>())
+  if (const CXXRecordDecl *BestDynamicDecl = Base->getBestDynamicClassType()) {
+    if (BestDynamicDecl->hasAttr<FinalAttr>())
       return true;
 
-  // If the member function is marked 'final', we know that it can't be
-  // overridden and can therefore devirtualize it.
-  if (MD->hasAttr<FinalAttr>())
+    // There may be a method corresponding to MD in a derived class. If that
+    // method is marked final, we can devirtualize it.
+    const CXXMethodDecl *DevirtualizedMethod =
+        MD->getCorrespondingMethodInClass(BestDynamicDecl);
+    if (DevirtualizedMethod->hasAttr<FinalAttr>())
       return true;
+  }
 
   // Similarly, if the class itself is marked 'final' it can't be overridden
   // and we can therefore devirtualize the member function call.
   if (MD->getParent()->hasAttr<FinalAttr>())
     return true;
 
   Base = skipNoOpCastsAndParens(Base);
   if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Base)) {

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

A *fop(RC *x) {
     // 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);
   }
 }
+
+namespace Test10 {
+  struct A {
+    virtual int f();
+  };
+
+  struct B : A {
+    int f() final;
+  };
+
+  // CHECK-LABEL: define i32 @_ZN6Test101fEPNS_1BE
+  int f(B *b) {
+    // CHECK: call i32 @_ZN6Test101B1fEv
+    return static_cast<A *>(b)->f();
+  }
+}

cfe/trunk/test/CodeGenCXX/ubsan-devirtualized-calls.cpp

 struct Derived1 final : Base1 {
   void f1() override {}
 };
 
 struct Derived2 final : Base1, Base2 {
   void f1() override {}
 };
 
-// PR13127 documents some missed devirtualization opportunities, including
-// devirt for methods marked 'final'. We can enable the checks marked 'PR13127'
-// if we implement this in the frontend.
 struct Derived3 : Base1 {
   void f1() override /* nofinal */ {}
 };
 
 struct Derived4 final : Base1 {
   void f1() override final {}
 };
 
 // CHECK: [[UBSAN_TI_DERIVED1_1:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived1 to i8*
 // CHECK: [[UBSAN_TI_DERIVED2_1:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived2 to i8*
 // CHECK: [[UBSAN_TI_DERIVED2_2:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived2 to i8*
-// PR13127: [[UBSAN_TI_DERIVED3:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived3 to i8*
-// PR13127: [[UBSAN_TI_BASE1:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI5Base1 to i8*
-// PR13127: [[UBSAN_TI_DERIVED4_1:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived4 to i8*
-// PR13127: [[UBSAN_TI_DERIVED4_2:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived4 to i8*
+// CHECK: [[UBSAN_TI_DERIVED3:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived3 to i8*
+// CHECK: [[UBSAN_TI_BASE1:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI5Base1 to i8*
+// CHECK: [[UBSAN_TI_DERIVED4_1:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived4 to i8*
+// CHECK: [[UBSAN_TI_DERIVED4_2:@[0-9]+]] = private unnamed_addr global {{.*}} i8* bitcast {{.*}} @_ZTI8Derived4 to i8*
 
 // CHECK-LABEL: define void @_Z2t1v
 void t1() {
   Derived1 d1;
   static_cast<Base1 *>(&d1)->f1(); //< Devirt Base1::f1 to Derived1::f1.
   // CHECK: %[[D1:[0-9]+]] = ptrtoint %struct.Derived1* %d1 to i{{[0-9]+}}, !nosanitize
   // CHECK-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED1_1]] {{.*}}, i{{[0-9]+}} %[[D1]]
 }

 // CHECK-LABEL: define void @_Z2t3v
 void t3() {
   Derived2 d2;
   static_cast<Base2 *>(&d2)->f1(); //< Devirt Base2::f1 to Derived2::f1.
   // CHECK: %[[D2_2:[0-9]+]] = ptrtoint %struct.Derived2* %d2 to i{{[0-9]+}}, !nosanitize
   // CHECK-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED2_2]] {{.*}}, i{{[0-9]+}} %[[D2_2]]
 }
 
-// PR13127-LABEL: define void @_Z2t4v
+// CHECK-LABEL: define void @_Z2t4v
 void t4() {
   Base1 p;
   Derived3 *badp = static_cast<Derived3 *>(&p); //< Check that &p isa Derived3.
-  // PR13127: %[[P1:[0-9]+]] = ptrtoint %struct.Derived3* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
-  // PR13127-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED3]] {{.*}}, i{{[0-9]+}} %[[P1]]
+  // CHECK: %[[P1:[0-9]+]] = ptrtoint %struct.Derived3* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
+  // CHECK-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED3]] {{.*}}, i{{[0-9]+}} %[[P1]]
 
   static_cast<Base1 *>(badp)->f1(); //< No devirt, test 'badp isa Base1'.
-  // PR13127: %[[BADP1:[0-9]+]] = ptrtoint %struct.Base1* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
-  // PR13127-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_BASE1]] {{.*}}, i{{[0-9]+}} %[[BADP1]]
+  // CHECK: %[[BADP1:[0-9]+]] = ptrtoint %struct.Base1* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
+  // CHECK-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_BASE1]] {{.*}}, i{{[0-9]+}} %[[BADP1]]
 }
 
-// PR13127-LABEL: define void @_Z2t5v
+// CHECK-LABEL: define void @_Z2t5v
 void t5() {
   Base1 p;
   Derived4 *badp = static_cast<Derived4 *>(&p); //< Check that &p isa Derived4.
-  // PR13127: %[[P1:[0-9]+]] = ptrtoint %struct.Derived4* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
-  // PR13127-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED4_1]] {{.*}}, i{{[0-9]+}} %[[P1]]
+  // CHECK: %[[P1:[0-9]+]] = ptrtoint %struct.Derived4* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
+  // CHECK-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED4_1]] {{.*}}, i{{[0-9]+}} %[[P1]]
 
   static_cast<Base1 *>(badp)->f1(); //< Devirt Base1::f1 to Derived4::f1.
-  // PR13127: %[[BADP1:[0-9]+]] = ptrtoint %struct.Derived4* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
-  // PR13127-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED4_2]] {{.*}}, i{{[0-9]+}} %[[BADP1]]
+  // CHECK: %[[BADP1:[0-9]+]] = ptrtoint %struct.Derived4* {{%[0-9]+}} to i{{[0-9]+}}, !nosanitize
+  // CHECK-NEXT: call void @__ubsan_handle_dynamic_type_cache{{[_a-z]*}}({{.*}} [[UBSAN_TI_DERIVED4_2]] {{.*}}, i{{[0-9]+}} %[[BADP1]]
 }