vsk updated this revision to Diff 87334.
vsk edited the summary of this revision.
vsk added a comment.
@arphaman thank you for the feedback!
Per Alex's comments:
- Add test for explicit use of the 'this' pointer.
- Handle cases where the 'this' pointer may be casted (implicitly, or
otherwise).
We're able to eliminate more null checks with this version of the patch. See
the updated patch summary for the X86FastISel.cpp numbers.
https://reviews.llvm.org/D29530
Files:
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprCXX.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGen/catch-undef-behavior.c
test/CodeGen/sanitize-recover.c
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,126 @@
+// 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 @_ZN1A11load_memberEv
+ int load_member() {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ return foo;
+ // CHECK: ret i32
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A11call_methodEv
+ int call_method() {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ return load_member();
+ // CHECK: ret i32
+ }
+
+ // CHECK-LABEL: define linkonce_odr void @_ZN1A15assign_member_1Ev
+ void assign_member_1() {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ foo = 0;
+ // CHECK: ret void
+ }
+
+ // CHECK-LABEL: define linkonce_odr void @_ZN1A15assign_member_2Ev
+ void assign_member_2() {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ this->foo = 0;
+ // CHECK: ret void
+ }
+
+ // CHECK-LABEL: define linkonce_odr void @_ZNK1A15assign_member_3Ev
+ void assign_member_3() const {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ const_cast<A *>(this)->foo = 0;
+ // CHECK: ret void
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A22call_through_referenceERS_
+ static int call_through_reference(A &a) {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ return a.load_member();
+ // CHECK: ret i32
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1A20call_through_pointerEPS_
+ static int call_through_pointer(A *a) {
+ // CHECK: call void @__ubsan_handle_type_mismatch
+ return a->load_member();
+ // CHECK: ret i32
+ }
+};
+
+struct B {
+ operator A*() const { return nullptr; }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN1B11load_memberEv
+ static int load_member() {
+ // Null-check &b before converting it to an A*.
+ // CHECK: call void @__ubsan_handle_type_mismatch
+ //
+ // Null-check the result of the conversion before using it.
+ // CHECK: call void @__ubsan_handle_type_mismatch
+ //
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ B b;
+ return static_cast<A *>(b)->load_member();
+ // CHECK: ret i32
+ }
+};
+
+struct Base {
+ int foo;
+
+ virtual int load_member_1() = 0;
+};
+
+struct Derived : public Base {
+ int bar;
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN7Derived13load_member_2Ev
+ int load_member_2() {
+ // Null-check the result of the cast before using it.
+ // CHECK: call void @__ubsan_handle_type_mismatch
+ //
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ return dynamic_cast<Base *>(this)->load_member_1();
+ // CHECK: ret i32
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN7Derived13load_member_3Ev
+ int load_member_3() {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ return reinterpret_cast<Derived *>(static_cast<Base *>(this))->foo;
+ // CHECK: ret i32
+ }
+
+ // CHECK-LABEL: define linkonce_odr i32 @_ZN7Derived13load_member_1Ev
+ int load_member_1() override {
+ // CHECK-NOT: call void @__ubsan_handle_type_mismatch
+ return foo + bar;
+ // CHECK: ret i32
+ }
+};
+
+void force_irgen() {
+ A *a;
+ a->load_member();
+ a->call_method();
+ a->assign_member_1();
+ a->assign_member_2();
+ a->assign_member_3();
+ A::call_through_reference(*a);
+ A::call_through_pointer(a);
+
+ B::load_member();
+
+ Base *b = new Derived;
+ b->load_member_1();
+
+ Derived *d;
+ d->load_member_2();
+ d->load_member_3();
+}
Index: test/CodeGen/sanitize-recover.c
===================================================================
--- test/CodeGen/sanitize-recover.c
+++ test/CodeGen/sanitize-recover.c
@@ -19,20 +19,17 @@
void foo() {
union { int i; } u;
u.i=1;
- // PARTIAL: %[[CHECK0:.*]] = icmp ne {{.*}}* %[[PTR:.*]], null
-
// PARTIAL: %[[SIZE:.*]] = call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false)
- // PARTIAL-NEXT: %[[CHECK1:.*]] = icmp uge i64 %[[SIZE]], 4
+ // PARTIAL-NEXT: %[[CHECK0:.*]] = icmp uge i64 %[[SIZE]], 4
// PARTIAL: %[[MISALIGN:.*]] = and i64 {{.*}}, 3
- // PARTIAL-NEXT: %[[CHECK2:.*]] = icmp eq i64 %[[MISALIGN]], 0
+ // PARTIAL-NEXT: %[[CHECK1:.*]] = icmp eq i64 %[[MISALIGN]], 0
- // PARTIAL: %[[CHECK02:.*]] = and i1 %[[CHECK0]], %[[CHECK2]]
- // PARTIAL-NEXT: %[[CHECK012:.*]] = and i1 %[[CHECK02]], %[[CHECK1]]
+ // PARTIAL: %[[CHECK01:.*]] = and i1 %[[CHECK1]], %[[CHECK0]]
- // PARTIAL: br i1 %[[CHECK012]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
+ // PARTIAL: br i1 %[[CHECK01]], {{.*}} !nosanitize
+ // PARTIAL: br i1 %[[CHECK1]], {{.*}} !nosanitize
- // PARTIAL: br i1 %[[CHECK02]], {{.*}}
// PARTIAL: call void @__ubsan_handle_type_mismatch_v1_abort(
// PARTIAL-NEXT: unreachable
// PARTIAL: call void @__ubsan_handle_type_mismatch_v1(
Index: test/CodeGen/catch-undef-behavior.c
===================================================================
--- test/CodeGen/catch-undef-behavior.c
+++ test/CodeGen/catch-undef-behavior.c
@@ -1,6 +1,5 @@
// RUN: %clang_cc1 -fsanitize=alignment,null,object-size,shift-base,shift-exponent,return,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute,nonnull-attribute -fsanitize-recover=alignment,null,object-size,shift-base,shift-exponent,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute,nonnull-attribute -emit-llvm %s -o - -triple x86_64-linux-gnu | opt -instnamer -S | FileCheck %s --check-prefix=CHECK-COMMON --check-prefix=CHECK-UBSAN
// RUN: %clang_cc1 -fsanitize-trap=alignment,null,object-size,shift-base,shift-exponent,return,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute,nonnull-attribute -fsanitize-recover=alignment,null,object-size,shift-base,shift-exponent,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute,nonnull-attribute -fsanitize=alignment,null,object-size,shift-base,shift-exponent,return,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute,nonnull-attribute -fsanitize-recover=alignment,null,object-size,shift-base,shift-exponent,signed-integer-overflow,vla-bound,float-cast-overflow,integer-divide-by-zero,bool,returns-nonnull-attribute,nonnull-attribute -emit-llvm %s -o - -triple x86_64-linux-gnu | opt -instnamer -S | FileCheck %s --check-prefix=CHECK-COMMON --check-prefix=CHECK-TRAP
-// RUN: %clang_cc1 -fsanitize=null -fsanitize-recover=null -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-NULL
// RUN: %clang_cc1 -fsanitize=signed-integer-overflow -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-OVERFLOW
// CHECK-UBSAN: @[[INT:.*]] = private unnamed_addr constant { i16, i16, [6 x i8] } { i16 0, i16 11, [6 x i8] c"'int'\00" }
@@ -30,25 +29,20 @@
// CHECK-UBSAN: @[[LINE_1500:.*]] = {{.*}}, i32 1500, i32 10 {{.*}} @[[FP16]], {{.*}} }
// CHECK-UBSAN: @[[LINE_1600:.*]] = {{.*}}, i32 1600, i32 10 {{.*}} @{{.*}} }
-// CHECK-NULL: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}}
-
// PR6805
// CHECK-COMMON-LABEL: @foo
-// CHECK-NULL-LABEL: @foo
void foo() {
union { int i; } u;
- // CHECK-COMMON: %[[CHECK0:.*]] = icmp ne {{.*}}* %[[PTR:.*]], null
- // CHECK-COMMON: %[[I8PTR:.*]] = bitcast i32* %[[PTR]] to i8*
+ // CHECK-COMMON: %[[I8PTR:.*]] = bitcast i32* %[[PTR:.*]] to i8*
// CHECK-COMMON-NEXT: %[[SIZE:.*]] = call i64 @llvm.objectsize.i64.p0i8(i8* %[[I8PTR]], i1 false)
- // CHECK-COMMON-NEXT: %[[CHECK1:.*]] = icmp uge i64 %[[SIZE]], 4
+ // CHECK-COMMON-NEXT: %[[CHECK0:.*]] = icmp uge i64 %[[SIZE]], 4
// CHECK-COMMON: %[[PTRTOINT:.*]] = ptrtoint {{.*}}* %[[PTR]] to i64
// CHECK-COMMON-NEXT: %[[MISALIGN:.*]] = and i64 %[[PTRTOINT]], 3
- // CHECK-COMMON-NEXT: %[[CHECK2:.*]] = icmp eq i64 %[[MISALIGN]], 0
+ // CHECK-COMMON-NEXT: %[[CHECK1:.*]] = icmp eq i64 %[[MISALIGN]], 0
- // CHECK-COMMON: %[[CHECK01:.*]] = and i1 %[[CHECK0]], %[[CHECK1]]
- // CHECK-COMMON-NEXT: %[[OK:.*]] = and i1 %[[CHECK01]], %[[CHECK2]]
+ // CHECK-COMMON: %[[OK:.*]] = and i1 %[[CHECK0]], %[[CHECK1]]
// CHECK-UBSAN: br i1 %[[OK]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
// CHECK-TRAP: br i1 %[[OK]], {{.*}}
@@ -58,11 +52,6 @@
// CHECK-TRAP: call void @llvm.trap() [[NR_NUW:#[0-9]+]]
// CHECK-TRAP-NEXT: unreachable
-
- // With -fsanitize=null, only perform the null check.
- // CHECK-NULL: %[[NULL:.*]] = icmp ne {{.*}}, null
- // CHECK-NULL: br i1 %[[NULL]]
- // CHECK-NULL: call void @__ubsan_handle_type_mismatch_v1(i8* bitcast ({{.*}} @[[LINE_100]] to i8*), i64 %{{.*}})
#line 100
u.i=1;
}
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2030,6 +2030,9 @@
llvm::BlockAddress *GetAddrOfLabel(const LabelDecl *L);
llvm::BasicBlock *GetIndirectGotoBlock();
+ /// Check if the null check for \p ObjectPointer can be skipped.
+ static bool CanElideObjectPointerNullCheck(const Expr *ObjectPointer);
+
/// EmitNullInitialization - Generate code to set a value of the given type to
/// null, If the type contains data member pointers, they will be initialized
/// to -1 in accordance with the Itanium C++ ABI.
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)) {
+ SkipNullCheck =
+ CanElideObjectPointerNullCheck(CMCE->getImplicitObjectArgument());
+ }
+ 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
@@ -947,15 +947,37 @@
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
+ 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()) {
+ bool SkipNullCheck = false;
+ if (const auto *ME = dyn_cast<MemberExpr>(E))
+ SkipNullCheck = CanElideObjectPointerNullCheck(ME->getBase());
EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(),
- E->getType(), LV.getAlignment());
+ E->getType(), LV.getAlignment(), SkipNullCheck);
+ }
return LV;
}
@@ -3335,7 +3357,9 @@
AlignmentSource AlignSource;
Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource);
QualType PtrTy = BaseExpr->getType()->getPointeeType();
- EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy);
+ bool SkipNullCheck = CanElideObjectPointerNullCheck(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
