Page MenuHomePhabricator

[ubsan] nullability-arg: Fix crash on C++ member function pointers
ClosedPublic

Authored by vsk on Sep 25 2020, 1:12 PM.

Details

Summary

Extend -fsanitize=nullability-arg to handle call sites which accept C++
member function pointers.

rdar://62476022

Diff Detail

Event Timeline

vsk created this revision.Sep 25 2020, 1:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2020, 1:12 PM
Herald added a subscriber: dexonsmith. · View Herald Transcript
vsk requested review of this revision.Sep 25 2020, 1:12 PM

It looks like this still doesn't check null correctly (i.e., compare to -1) for data member pointers. Is that correct?

clang/lib/CodeGen/CGCall.cpp
3752

I think it's better to make it clear in the comment that we are checking for pointers to member function, not pointers to data members.

Also, I wonder whether there is a better way to do this only for Itanium ABI. Maybe just check ArgType->isMemberFunctionPointerType() and add a virtual function to CGCXXABI which extracts the pointer field and call it here?

vsk updated this revision to Diff 294457.Sep 25 2020, 5:06 PM

Simplify, per Akira's comment.

vsk added a comment.Sep 25 2020, 5:08 PM

It looks like this still doesn't check null correctly (i.e., compare to -1) for data member pointers. Is that correct?

Thanks for catching this. The new revision takes advantage of CXXABI::EmitMemberPointerIsNotNull, so null data member pointers are now diagnosed.

ahatanak accepted this revision.Sep 25 2020, 6:18 PM

LGTM

clang/lib/CodeGen/CGExpr.cpp
1181

You can fold T->getAs<MemberPointerType>() into the condition of the if statement.

This revision is now accepted and ready to land.Sep 25 2020, 6:18 PM
vsk added a comment.Sep 28 2020, 9:40 AM

Thanks for the review!

clang/lib/CodeGen/CGExpr.cpp
1181

I'll fix this before committing.

This revision was landed with ongoing or failed builds.Sep 28 2020, 9:42 AM
This revision was automatically updated to reflect the committed changes.