Author: vedantk Date: Fri Feb 17 14:59:40 2017 New Revision: 295494 URL: http://llvm.org/viewvc/llvm-project?rev=295494&view=rev Log: Revert "Retry: [ubsan] Reduce null checking of C++ object pointers (PR27581)"
This reverts commit r295401. It breaks the ubsan self-host. It inserts object size checks once per C++ method which fire when the structure is empty. Removed: cfe/trunk/test/CodeGenCXX/ubsan-suppress-null-checks.cpp Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp cfe/trunk/lib/CodeGen/CGExprCXX.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/test/CodeGen/catch-undef-behavior.c cfe/trunk/test/CodeGen/sanitize-recover.c Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=295494&r1=295493&r2=295494&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Feb 17 14:59:40 2017 @@ -947,45 +947,15 @@ LValue CodeGenFunction::EmitUnsupportedL 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()) { - bool SkipNullCheck = false; - if (const auto *ME = dyn_cast<MemberExpr>(E)) - SkipNullCheck = CanElideObjectPointerNullCheck(ME->getBase()); + if (!isa<DeclRefExpr>(E) && !LV.isBitField() && LV.isSimple()) EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(), - E->getType(), LV.getAlignment(), SkipNullCheck); - } + E->getType(), LV.getAlignment()); return LV; } @@ -3365,9 +3335,7 @@ LValue CodeGenFunction::EmitMemberExpr(c AlignmentSource AlignSource; Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource); QualType PtrTy = BaseExpr->getType()->getPointeeType(); - bool SkipNullCheck = CanElideObjectPointerNullCheck(BaseExpr); - EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy, - /*Alignment=*/CharUnits::Zero(), SkipNullCheck); + EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy); BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource); } else BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess); Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=295494&r1=295493&r2=295494&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Fri Feb 17 14:59:40 2017 @@ -290,15 +290,10 @@ RValue CodeGenFunction::EmitCXXMemberOrO if (CE) CallLoc = CE->getExprLoc(); - 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); + EmitTypeCheck(isa<CXXConstructorDecl>(CalleeDecl) + ? CodeGenFunction::TCK_ConstructorCall + : CodeGenFunction::TCK_MemberCall, + CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=295494&r1=295493&r2=295494&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Feb 17 14:59:40 2017 @@ -948,11 +948,6 @@ void CodeGenFunction::StartFunction(Glob // fast register allocator would be happier... CXXThisValue = CXXABIThisValue; } - - // Sanitize the 'this' pointer once per function, if it's available. - if (CXXThisValue) - EmitTypeCheck(TCK_MemberAccess, Loc, CXXThisValue, - MD->getThisType(getContext())); } // If any of the arguments have a variably modified type, make sure to Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=295494&r1=295493&r2=295494&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Feb 17 14:59:40 2017 @@ -2030,9 +2030,6 @@ public: 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. Modified: cfe/trunk/test/CodeGen/catch-undef-behavior.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/catch-undef-behavior.c?rev=295494&r1=295493&r2=295494&view=diff ============================================================================== --- cfe/trunk/test/CodeGen/catch-undef-behavior.c (original) +++ cfe/trunk/test/CodeGen/catch-undef-behavior.c Fri Feb 17 14:59:40 2017 @@ -1,5 +1,6 @@ // 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" } @@ -29,20 +30,25 @@ // 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: %[[CHECK0:.*]] = icmp uge i64 %[[SIZE]], 4 + // CHECK-COMMON-NEXT: %[[CHECK1:.*]] = icmp uge i64 %[[SIZE]], 4 // CHECK-COMMON: %[[PTRTOINT:.*]] = ptrtoint {{.*}}* %[[PTR]] to i64 // CHECK-COMMON-NEXT: %[[MISALIGN:.*]] = and i64 %[[PTRTOINT]], 3 - // CHECK-COMMON-NEXT: %[[CHECK1:.*]] = icmp eq i64 %[[MISALIGN]], 0 + // CHECK-COMMON-NEXT: %[[CHECK2:.*]] = icmp eq i64 %[[MISALIGN]], 0 - // CHECK-COMMON: %[[OK:.*]] = and i1 %[[CHECK0]], %[[CHECK1]] + // CHECK-COMMON: %[[CHECK01:.*]] = and i1 %[[CHECK0]], %[[CHECK1]] + // CHECK-COMMON-NEXT: %[[OK:.*]] = and i1 %[[CHECK01]], %[[CHECK2]] // CHECK-UBSAN: br i1 %[[OK]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize // CHECK-TRAP: br i1 %[[OK]], {{.*}} @@ -52,6 +58,11 @@ void foo() { // 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; } Modified: cfe/trunk/test/CodeGen/sanitize-recover.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sanitize-recover.c?rev=295494&r1=295493&r2=295494&view=diff ============================================================================== --- cfe/trunk/test/CodeGen/sanitize-recover.c (original) +++ cfe/trunk/test/CodeGen/sanitize-recover.c Fri Feb 17 14:59:40 2017 @@ -19,17 +19,20 @@ void test() { 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: %[[CHECK0:.*]] = icmp uge i64 %[[SIZE]], 4 + // PARTIAL-NEXT: %[[CHECK1:.*]] = icmp uge i64 %[[SIZE]], 4 // PARTIAL: %[[MISALIGN:.*]] = and i64 {{.*}}, 3 - // PARTIAL-NEXT: %[[CHECK1:.*]] = icmp eq i64 %[[MISALIGN]], 0 + // PARTIAL-NEXT: %[[CHECK2:.*]] = icmp eq i64 %[[MISALIGN]], 0 - // PARTIAL: %[[CHECK01:.*]] = and i1 %[[CHECK1]], %[[CHECK0]] + // PARTIAL: %[[CHECK02:.*]] = and i1 %[[CHECK0]], %[[CHECK2]] + // PARTIAL-NEXT: %[[CHECK012:.*]] = and i1 %[[CHECK02]], %[[CHECK1]] - // PARTIAL: br i1 %[[CHECK01]], {{.*}} !nosanitize - // PARTIAL: br i1 %[[CHECK1]], {{.*}} !nosanitize + // PARTIAL: br i1 %[[CHECK012]], {{.*}} !prof ![[WEIGHT_MD:.*]], !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( Removed: cfe/trunk/test/CodeGenCXX/ubsan-suppress-null-checks.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-suppress-null-checks.cpp?rev=295493&view=auto ============================================================================== --- cfe/trunk/test/CodeGenCXX/ubsan-suppress-null-checks.cpp (original) +++ cfe/trunk/test/CodeGenCXX/ubsan-suppress-null-checks.cpp (removed) @@ -1,188 +0,0 @@ -// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=null | FileCheck %s -// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=null -DCHECK_LAMBDA | FileCheck %s --check-prefix=LAMBDA - -struct A { - int foo; - - // CHECK-LABEL: define linkonce_odr void @_ZN1A10do_nothingEv - void do_nothing() { - // CHECK: icmp ne %struct.A* %[[THIS1:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.A* %[[THIS1]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // CHECK-NOT: call void @__ubsan_handle_type_mismatch - // CHECK: ret void - } - -#ifdef CHECK_LAMBDA - // LAMBDA-LABEL: define linkonce_odr void @_ZN1A22do_nothing_with_lambdaEv - void do_nothing_with_lambda() { - // LAMBDA: icmp ne %struct.A* %[[THIS2:[a-z0-9]+]], null, !nosanitize - // LAMBDA: ptrtoint %struct.A* %[[THIS2]] to i64, !nosanitize - // LAMBDA-NEXT: call void @__ubsan_handle_type_mismatch - - auto f = [&] { - foo = 0; - }; - f(); - - // LAMBDA: ret void - } - -// Check the IR for the lambda: -// -// LAMBDA-LABEL: define linkonce_odr void @_ZZN1A22do_nothing_with_lambdaEvENKUlvE_clEv -// LAMBDA: call void @__ubsan_handle_type_mismatch -// LAMBDA-NOT: call void @__ubsan_handle_type_mismatch -// LAMBDA: ret void -#endif - - // CHECK-LABEL: define linkonce_odr i32 @_ZN1A11load_memberEv - int load_member() { - // CHECK: icmp ne %struct.A* %[[THIS3:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.A* %[[THIS3]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // 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: icmp ne %struct.A* %[[THIS4:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.A* %[[THIS4]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // 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: icmp ne %struct.A* %[[THIS5:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.A* %[[THIS5]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // 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: icmp ne %struct.A* %[[THIS6:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.A* %[[THIS6]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // CHECK-NOT: call void @__ubsan_handle_type_mismatch - (__extension__ (this))->foo = 0; - // CHECK: ret void - } - - // CHECK-LABEL: define linkonce_odr void @_ZNK1A15assign_member_3Ev - void assign_member_3() const { - // CHECK: icmp ne %struct.A* %[[THIS7:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.A* %[[THIS7]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // 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() { - // CHECK: icmp ne %struct.Derived* %[[THIS8:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.Derived* %[[THIS8]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // - // 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: icmp ne %struct.Derived* %[[THIS9:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.Derived* %[[THIS9]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // 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: icmp ne %struct.Derived* %[[THIS10:[a-z0-9]+]], null, !nosanitize - // CHECK: ptrtoint %struct.Derived* %[[THIS10]] to i64, !nosanitize - // CHECK-NEXT: call void @__ubsan_handle_type_mismatch - // CHECK-NOT: call void @__ubsan_handle_type_mismatch - return foo + bar; - // CHECK: ret i32 - } -}; - -void force_irgen() { - A *a; - a->do_nothing(); -#ifdef CHECK_LAMBDA - a->do_nothing_with_lambda(); -#endif - 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(); -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits