Retry^2: [ubsan] Reduce null checking of C++ object pointers (PR27581)
This patch teaches ubsan to insert exactly one null check for the 'this'
pointer per method/lambda.
Previously, given a load of a member variable from an instance method
('this->x'), ubsan would insert a null check for 'this', and another
null check for '&this->x', before allowing the load to occur.
Similarly, given a call to a method from another method bound to the
same instance ('this->foo()'), ubsan would a redundant null check for
'this'. There is also a redundant null check in the case where the
object pointer is a reference ('Ref.foo()').
This patch teaches ubsan to remove the redundant null checks identified
above.
Testing: check-clang, check-ubsan, and a stage2 ubsan build.
I also compiled X86FastISel.cpp with -fsanitize=null using
patched/unpatched clangs based on r293572. Here are the number of null
checks emitted:
-------------------------------------
| Setup | # of null checks |
-------------------------------------
| unpatched, -O0 | 21767 |
| patched, -O0 | 10758 |
-------------------------------------
Changes since the initial commit:
- Don't introduce any unintentional object-size or alignment checks.
- Don't rely on IRGen of C labels in the test.
Differential Revision: https://reviews.llvm.org/D29530
llvm-svn: 295515
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 8661251..276716f 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -952,15 +952,46 @@
E->getType());
}
+bool CodeGenFunction::CanElideObjectPointerNullCheck(const Expr *Obj) {
+ if (isa<DeclRefExpr>(Obj))
+ return true;
+
+ const Expr *Base = Obj;
+ while (!isa<CXXThisExpr>(Base)) {
+ // The result of a dynamic_cast can be null.
+ if (isa<CXXDynamicCastExpr>(Base))
+ return false;
+
+ if (const auto *CE = dyn_cast<CastExpr>(Base)) {
+ Base = CE->getSubExpr();
+ } else if (const auto *PE = dyn_cast<ParenExpr>(Base)) {
+ Base = PE->getSubExpr();
+ } else if (const auto *UO = dyn_cast<UnaryOperator>(Base)) {
+ if (UO->getOpcode() == UO_Extension)
+ Base = UO->getSubExpr();
+ else
+ return false;
+ } else {
+ return false;
+ }
+ }
+ return true;
+}
+
LValue CodeGenFunction::EmitCheckedLValue(const Expr *E, TypeCheckKind TCK) {
LValue LV;
if (SanOpts.has(SanitizerKind::ArrayBounds) && isa<ArraySubscriptExpr>(E))
LV = EmitArraySubscriptExpr(cast<ArraySubscriptExpr>(E), /*Accessed*/true);
else
LV = EmitLValue(E);
- if (!isa<DeclRefExpr>(E) && !LV.isBitField() && LV.isSimple())
+ if (!isa<DeclRefExpr>(E) && !LV.isBitField() && LV.isSimple()) {
+ SanitizerSet SkippedChecks;
+ if (const auto *ME = dyn_cast<MemberExpr>(E))
+ if (CanElideObjectPointerNullCheck(ME->getBase()))
+ SkippedChecks.set(SanitizerKind::Null, true);
EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(),
- E->getType(), LV.getAlignment());
+ E->getType(), LV.getAlignment(), SkippedChecks);
+ }
return LV;
}
@@ -3340,7 +3371,11 @@
AlignmentSource AlignSource;
Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource);
QualType PtrTy = BaseExpr->getType()->getPointeeType();
- EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy);
+ SanitizerSet SkippedChecks;
+ if (CanElideObjectPointerNullCheck(BaseExpr))
+ SkippedChecks.set(SanitizerKind::Null, true);
+ EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy,
+ /*Alignment=*/CharUnits::Zero(), SkippedChecks);
BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource);
} else
BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess);