Revert "[ubsan] Reduce null checking of C++ object pointers (PR27581)"

This reverts commit r295391. It breaks this bot:

http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/1898

I need to not rely on labels in the IR test.

llvm-svn: 295396
This commit is contained in:
Vedant Kumar 2017-02-17 01:42:36 +00:00
parent 95ed51dcfe
commit 4f94a94bea
7 changed files with 30 additions and 248 deletions

View File

@ -947,45 +947,15 @@ LValue CodeGenFunction::EmitUnsupportedLValue(const Expr *E,
E->getType()); 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 CodeGenFunction::EmitCheckedLValue(const Expr *E, TypeCheckKind TCK) {
LValue LV; LValue LV;
if (SanOpts.has(SanitizerKind::ArrayBounds) && isa<ArraySubscriptExpr>(E)) if (SanOpts.has(SanitizerKind::ArrayBounds) && isa<ArraySubscriptExpr>(E))
LV = EmitArraySubscriptExpr(cast<ArraySubscriptExpr>(E), /*Accessed*/true); LV = EmitArraySubscriptExpr(cast<ArraySubscriptExpr>(E), /*Accessed*/true);
else else
LV = EmitLValue(E); 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(), EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(),
E->getType(), LV.getAlignment(), SkipNullCheck); E->getType(), LV.getAlignment());
}
return LV; return LV;
} }
@ -3365,9 +3335,7 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) {
AlignmentSource AlignSource; AlignmentSource AlignSource;
Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource); Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource);
QualType PtrTy = BaseExpr->getType()->getPointeeType(); QualType PtrTy = BaseExpr->getType()->getPointeeType();
bool SkipNullCheck = CanElideObjectPointerNullCheck(BaseExpr); EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy);
EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy,
/*Alignment=*/CharUnits::Zero(), SkipNullCheck);
BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource); BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource);
} else } else
BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess); BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess);

View File

@ -290,15 +290,10 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(
if (CE) if (CE)
CallLoc = CE->getExprLoc(); CallLoc = CE->getExprLoc();
bool SkipNullCheck = false; EmitTypeCheck(isa<CXXConstructorDecl>(CalleeDecl)
if (const auto *CMCE = dyn_cast<CXXMemberCallExpr>(CE)) ? CodeGenFunction::TCK_ConstructorCall
SkipNullCheck =
CanElideObjectPointerNullCheck(CMCE->getImplicitObjectArgument());
EmitTypeCheck(
isa<CXXConstructorDecl>(CalleeDecl) ? CodeGenFunction::TCK_ConstructorCall
: CodeGenFunction::TCK_MemberCall, : CodeGenFunction::TCK_MemberCall,
CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()), 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 // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
// 'CalleeDecl' instead. // 'CalleeDecl' instead.

View File

@ -948,11 +948,6 @@ void CodeGenFunction::StartFunction(GlobalDecl GD,
// fast register allocator would be happier... // fast register allocator would be happier...
CXXThisValue = CXXABIThisValue; 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 // If any of the arguments have a variably modified type, make sure to

View File

@ -2030,9 +2030,6 @@ public:
llvm::BlockAddress *GetAddrOfLabel(const LabelDecl *L); llvm::BlockAddress *GetAddrOfLabel(const LabelDecl *L);
llvm::BasicBlock *GetIndirectGotoBlock(); 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 /// EmitNullInitialization - Generate code to set a value of the given type to
/// null, If the type contains data member pointers, they will be initialized /// null, If the type contains data member pointers, they will be initialized
/// to -1 in accordance with the Itanium C++ ABI. /// to -1 in accordance with the Itanium C++ ABI.

View File

@ -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=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-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 // 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" } // 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_1500:.*]] = {{.*}}, i32 1500, i32 10 {{.*}} @[[FP16]], {{.*}} }
// CHECK-UBSAN: @[[LINE_1600:.*]] = {{.*}}, i32 1600, i32 10 {{.*}} @{{.*}} } // CHECK-UBSAN: @[[LINE_1600:.*]] = {{.*}}, i32 1600, i32 10 {{.*}} @{{.*}} }
// CHECK-NULL: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}}
// PR6805 // PR6805
// CHECK-COMMON-LABEL: @foo // CHECK-COMMON-LABEL: @foo
// CHECK-NULL-LABEL: @foo
void foo() { void foo() {
union { int i; } u; 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: %[[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: %[[PTRTOINT:.*]] = ptrtoint {{.*}}* %[[PTR]] to i64
// CHECK-COMMON-NEXT: %[[MISALIGN:.*]] = and i64 %[[PTRTOINT]], 3 // 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-UBSAN: br i1 %[[OK]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
// CHECK-TRAP: br i1 %[[OK]], {{.*}} // CHECK-TRAP: br i1 %[[OK]], {{.*}}
@ -52,6 +58,11 @@ void foo() {
// CHECK-TRAP: call void @llvm.trap() [[NR_NUW:#[0-9]+]] // CHECK-TRAP: call void @llvm.trap() [[NR_NUW:#[0-9]+]]
// CHECK-TRAP-NEXT: unreachable // 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 #line 100
u.i=1; u.i=1;
} }

View File

@ -19,17 +19,20 @@ void test() {
void foo() { void foo() {
union { int i; } u; union { int i; } u;
u.i=1; u.i=1;
// PARTIAL: %[[CHECK0:.*]] = icmp ne {{.*}}* %[[PTR:.*]], null
// PARTIAL: %[[SIZE:.*]] = call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false) // 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: %[[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 %[[CHECK012]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
// PARTIAL: br i1 %[[CHECK1]], {{.*}} !nosanitize
// PARTIAL: br i1 %[[CHECK02]], {{.*}}
// PARTIAL: call void @__ubsan_handle_type_mismatch_v1_abort( // PARTIAL: call void @__ubsan_handle_type_mismatch_v1_abort(
// PARTIAL-NEXT: unreachable // PARTIAL-NEXT: unreachable
// PARTIAL: call void @__ubsan_handle_type_mismatch_v1( // PARTIAL: call void @__ubsan_handle_type_mismatch_v1(

View File

@ -1,187 +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: call void @__ubsan_handle_type_mismatch
// CHECK: L1
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
L1:
return foo;
// CHECK: ret i32
}
// CHECK-LABEL: define linkonce_odr i32 @_ZN1A11call_methodEv
int call_method() {
// CHECK: call void @__ubsan_handle_type_mismatch
// CHECK: L2
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
L2:
return load_member();
// CHECK: ret i32
}
// CHECK-LABEL: define linkonce_odr void @_ZN1A15assign_member_1Ev
void assign_member_1() {
// CHECK: call void @__ubsan_handle_type_mismatch
// CHECK: L3
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
L3:
foo = 0;
// CHECK: ret void
}
// CHECK-LABEL: define linkonce_odr void @_ZN1A15assign_member_2Ev
void assign_member_2() {
// CHECK: call void @__ubsan_handle_type_mismatch
// CHECK: L4
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
L4:
(__extension__ (this))->foo = 0;
// CHECK: ret void
}
// CHECK-LABEL: define linkonce_odr void @_ZNK1A15assign_member_3Ev
void assign_member_3() const {
// CHECK: call void @__ubsan_handle_type_mismatch
// CHECK: L5
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
L5:
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: call void @__ubsan_handle_type_mismatch
// CHECK: L6
L6:
// 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: call void @__ubsan_handle_type_mismatch
// CHECK: L7
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
L7:
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: call void @__ubsan_handle_type_mismatch
// CHECK: L8
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
L8:
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();
}