r286534 - PR30937: don't devirtualize if we find that the callee is a pure virtual
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 10 20:33:58 PST 2016
On Thu, Nov 10, 2016 at 6:12 PM, Mehdi Amini via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Thinking again, the method “can” be implemented I think. Forget it…
>
It can be, but the context is a virtual dispatch, so an overrider won't
actually be used, and you do get undefined behavior.
Emitting a direct call to __cxa_pure_virtual (or the MS ABI equivalent)
would likely be a more "polite" option than going straight to
@llvm.unreachable (after all, that's what would be in the vtable slot)...
but see PR30937: doing so would cause a Clang-compiled Firefox to stop
working. It sounds like they're fixing the problem, though, so maybe we can
do this at some point. If we go that way (and probably even if we don't),
we should teach Sema to detect this case and issue a warning.
> —
> Mehdi
>
> > On Nov 10, 2016, at 6:11 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> >
> > Could we emit llvm.unreachable in this case?
> >
> > —
> > Mehdi
> >
> >> On Nov 10, 2016, at 5:01 PM, Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> >>
> >> Author: rsmith
> >> Date: Thu Nov 10 19:01:31 2016
> >> New Revision: 286534
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=286534&view=rev
> >> Log:
> >> PR30937: don't devirtualize if we find that the callee is a pure virtual
> >> function. In that case, there is no requirement that the callee is
> actually
> >> defined, and the code may in fact be valid and have defined behavior if
> the
> >> virtual call is unreachable.
> >>
> >> Modified:
> >> cfe/trunk/lib/CodeGen/CGClass.cpp
> >> cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
> >>
> >> Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CGClass.cpp?rev=286534&r1=286533&r2=286534&view=diff
> >> ============================================================
> ==================
> >> --- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
> >> +++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Nov 10 19:01:31 2016
> >> @@ -2851,9 +2851,9 @@ CodeGenFunction::CanDevirtualizeMemberFu
> >> return false;
> >>
> >> // If the member function is marked 'final', we know that it can't be
> >> - // overridden and can therefore devirtualize it.
> >> + // overridden and can therefore devirtualize it unless it's pure
> virtual.
> >> if (MD->hasAttr<FinalAttr>())
> >> - return true;
> >> + return !MD->isPure();
> >>
> >> // If the base expression (after skipping derived-to-base conversions)
> is a
> >> // class prvalue, then we can devirtualize.
> >> @@ -2861,31 +2861,28 @@ CodeGenFunction::CanDevirtualizeMemberFu
> >> if (Base->isRValue() && Base->getType()->isRecordType())
> >> 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();
> >> - // }
> >> - //
> >> - if (const CXXRecordDecl *BestDynamicDecl =
> Base->getBestDynamicClassType()) {
> >> - if (BestDynamicDecl->hasAttr<FinalAttr>())
> >> - return true;
> >> -
> >> - // 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;
> >> - }
> >> + // If we don't even know what we would call, we can't devirtualize.
> >> + const CXXRecordDecl *BestDynamicDecl = Base->getBestDynamicClassType(
> );
> >> + if (!BestDynamicDecl)
> >> + return false;
> >> +
> >> + // There may be a method corresponding to MD in a derived class.
> >> + const CXXMethodDecl *DevirtualizedMethod =
> >> + MD->getCorrespondingMethodInClass(BestDynamicDecl);
> >> +
> >> + // If that method is pure virtual, we can't devirtualize. If this
> code is
> >> + // reached, the result would be UB, not a direct call to the derived
> class
> >> + // function, and we can't assume the derived class function is
> defined.
> >> + if (DevirtualizedMethod->isPure())
> >> + return false;
> >> +
> >> + // If that method is marked final, we can devirtualize it.
> >> + 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>())
> >> + if (BestDynamicDecl->hasAttr<FinalAttr>())
> >> return true;
> >>
> >> if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Base)) {
> >>
> >> Modified: cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-
> calls.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> CodeGenCXX/devirtualize-virtual-function-calls.cpp?
> rev=286534&r1=286533&r2=286534&view=diff
> >> ============================================================
> ==================
> >> --- cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
> (original)
> >> +++ cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
> Thu Nov 10 19:01:31 2016
> >> @@ -161,3 +161,37 @@ namespace test4 {
> >> p->fish.eat();
> >> }
> >> }
> >> +
> >> +// Do not devirtualize to pure virtual function calls.
> >> +namespace test5 {
> >> + struct X {
> >> + virtual void f() = 0;
> >> + };
> >> + struct Y {};
> >> + // CHECK-LABEL: define {{.*}} @_ZN5test51f
> >> + void f(Y &y, X Y::*p) {
> >> + // CHECK-NOT: call {{.*}} @_ZN5test51X1fEv
> >> + // CHECK: call void %
> >> + (y.*p).f();
> >> + };
> >> +
> >> + struct Z final {
> >> + virtual void f() = 0;
> >> + };
> >> + // CHECK-LABEL: define {{.*}} @_ZN5test51g
> >> + void g(Z &z) {
> >> + // CHECK-NOT: call {{.*}} @_ZN5test51Z1fEv
> >> + // CHECK: call void %
> >> + z.f();
> >> + }
> >> +
> >> + struct Q {
> >> + virtual void f() final = 0;
> >> + };
> >> + // CHECK-LABEL: define {{.*}} @_ZN5test51h
> >> + void h(Q &q) {
> >> + // CHECK-NOT: call {{.*}} @_ZN5test51Q1fEv
> >> + // CHECK: call void %
> >> + q.f();
> >> + }
> >> +}
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161110/27707f8d/attachment-0001.html>
More information about the cfe-commits
mailing list