vsk created this revision.
Given a load of a member variable from an instance method ('this->x'),
ubsan inserts a null check for 'this', and another null check for
'&this->x', before allowing the load to occur. Both of these checks are
redundant, because 'this' must have been null-checked before the method
is called.
Similarly, given a call to a method from another method bound to the
same instance ('this->foo()'), ubsan inserts 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. I'm not sure I've gone about this in the way suggested in PR27581,
and would appreciate any advice/corrections.
Testing: check-clang and check-ubsan. I also compiled X86FastISel.cpp
with -fsanitize=null using patched/unpatched clangs based on r293572.
Here are the number of null checks emitted in various setups:
| Setup | # of null checks |
| unpatched, -O0 | 21767 |
| unpatched, -O3 | 11862 |
| patched, -O0 | 13178 |
| patched, -O3 | 5547 |
https://reviews.llvm.org/D29530
Files:
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprCXX.cpp
test/CodeGenCXX/ubsan-suppress-null-checks.cpp
Index: test/CodeGenCXX/ubsan-suppress-null-checks.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/ubsan-suppress-null-checks.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s
-fsanitize=null | FileCheck %s
+
+struct A {
+ int foo;
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func1Ev
+ int func1() {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ return foo;
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func2Ev
+ int func2() {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ return func1();
+ }
+
+ // CHECK-LABEL: define linkonce_odr void @_ZN1A5func3Ev
+ void func3() {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ foo = 0;
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func4ERS_
+ static int func4(A &a) {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ return a.func1();
+ }
+};
+
+// Force clang to emit IR for A's methods.
+void bar() {
+ A *a;
+ a->func1();
+ a->func2();
+ a->func3();
+ A::func4(*a);
+}
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -290,10 +290,16 @@
if (CE)
CallLoc = CE->getExprLoc();
- EmitTypeCheck(isa<CXXConstructorDecl>(CalleeDecl)
- ? CodeGenFunction::TCK_ConstructorCall
- : CodeGenFunction::TCK_MemberCall,
- CallLoc, This.getPointer(),
C.getRecordType(CalleeDecl->getParent()));
+ bool SkipNullCheck = false;
+ if (const auto *CMCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+ Expr *IOA = CMCE->getImplicitObjectArgument();
+ SkipNullCheck = isa<CXXThisExpr>(IOA) || isa<DeclRefExpr>(IOA);
+ }
+ EmitTypeCheck(
+ isa<CXXConstructorDecl>(CalleeDecl) ?
CodeGenFunction::TCK_ConstructorCall
+ : CodeGenFunction::TCK_MemberCall,
+ CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()),
+ /*Alignment=*/CharUnits::Zero(), SkipNullCheck);
// FIXME: Uses of 'MD' past this point need to be audited. We may need to use
// 'CalleeDecl' instead.
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -953,9 +953,13 @@
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()) {
+ bool SkipNullCheck = false;
+ if (const auto *ME = dyn_cast<MemberExpr>(E))
+ SkipNullCheck = isa<CXXThisExpr>(ME->getBase());
EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(),
- E->getType(), LV.getAlignment());
+ E->getType(), LV.getAlignment(), SkipNullCheck);
+ }
return LV;
}
@@ -3335,7 +3339,9 @@
AlignmentSource AlignSource;
Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource);
QualType PtrTy = BaseExpr->getType()->getPointeeType();
- EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy);
+ bool SkipNullCheck = isa<CXXThisExpr>(BaseExpr);
+ EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy,
+ /*Alignment=*/CharUnits::Zero(), SkipNullCheck);
BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource);
} else
BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess);
Index: test/CodeGenCXX/ubsan-suppress-null-checks.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/ubsan-suppress-null-checks.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=null | FileCheck %s
+
+struct A {
+ int foo;
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func1Ev
+ int func1() {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ return foo;
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func2Ev
+ int func2() {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ return func1();
+ }
+
+ // CHECK-LABEL: define linkonce_odr void @_ZN1A5func3Ev
+ void func3() {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ foo = 0;
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A5func4ERS_
+ static int func4(A &a) {
+ // CHECK-NOT: ubsan_handler_type_mismatch
+ return a.func1();
+ }
+};
+
+// Force clang to emit IR for A's methods.
+void bar() {
+ A *a;
+ a->func1();
+ a->func2();
+ a->func3();
+ A::func4(*a);
+}
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -290,10 +290,16 @@
if (CE)
CallLoc = CE->getExprLoc();
- EmitTypeCheck(isa<CXXConstructorDecl>(CalleeDecl)
- ? CodeGenFunction::TCK_ConstructorCall
- : CodeGenFunction::TCK_MemberCall,
- CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()));
+ bool SkipNullCheck = false;
+ if (const auto *CMCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+ Expr *IOA = CMCE->getImplicitObjectArgument();
+ SkipNullCheck = isa<CXXThisExpr>(IOA) || isa<DeclRefExpr>(IOA);
+ }
+ EmitTypeCheck(
+ isa<CXXConstructorDecl>(CalleeDecl) ? CodeGenFunction::TCK_ConstructorCall
+ : CodeGenFunction::TCK_MemberCall,
+ CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()),
+ /*Alignment=*/CharUnits::Zero(), SkipNullCheck);
// FIXME: Uses of 'MD' past this point need to be audited. We may need to use
// 'CalleeDecl' instead.
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -953,9 +953,13 @@
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()) {
+ bool SkipNullCheck = false;
+ if (const auto *ME = dyn_cast<MemberExpr>(E))
+ SkipNullCheck = isa<CXXThisExpr>(ME->getBase());
EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(),
- E->getType(), LV.getAlignment());
+ E->getType(), LV.getAlignment(), SkipNullCheck);
+ }
return LV;
}
@@ -3335,7 +3339,9 @@
AlignmentSource AlignSource;
Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource);
QualType PtrTy = BaseExpr->getType()->getPointeeType();
- EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy);
+ bool SkipNullCheck = isa<CXXThisExpr>(BaseExpr);
+ EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy,
+ /*Alignment=*/CharUnits::Zero(), SkipNullCheck);
BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource);
} else
BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
