vsk created this revision.
This patch teaches ubsan to insert an alignment check for the 'this'
pointer at the start of each method/lambda. This allows clang to emit
significantly fewer alignment checks overall, because if 'this' is
aligned, so are its fields.
This is essentially the same thing r295515 does, but for the alignment
check instead of the null check.
Testing: check-clang, check-ubsan, and a stage2 ubsan build.
I also compiled X86FastISel.cpp with -fsanitize=alignment using
patched/unpatched clangs based on r295686. Here are the number of
alignment checks emitted:
| Setup | # of alignment checks |
| unpatched, -O0 | 24918 |
| patched, -O0 | 14307 |
There are a few possible follow-ups:
- Don't add the per method/lambda check in delegating constructors.
- Don't instrument accesses to fields with alignment = 1.
https://reviews.llvm.org/D30283
Files:
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprCXX.cpp
lib/CodeGen/CodeGenFunction.cpp
test/CodeGen/catch-undef-behavior.c
test/CodeGen/sanitize-recover.c
test/CodeGenCXX/ubsan-suppress-checks.cpp
test/CodeGenCXX/ubsan-suppress-null-checks.cpp
test/CodeGenCXX/ubsan-type-checks.cpp
Index: test/CodeGenCXX/ubsan-type-checks.cpp
===================================================================
--- test/CodeGenCXX/ubsan-type-checks.cpp
+++ test/CodeGenCXX/ubsan-type-checks.cpp
@@ -5,8 +5,8 @@
struct A {
// COMMON-LABEL: define linkonce_odr void @_ZN1A10do_nothingEv
void do_nothing() {
- // ALIGN-NOT: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
- // ALIGN-NOT: and i64 %{{.*}}, 7, !nosanitize
+ // ALIGN: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %{{.*}}, 0, !nosanitize
// NULL: icmp ne %struct.A* %{{.*}}, null, !nosanitize
Index: test/CodeGenCXX/ubsan-suppress-checks.cpp
===================================================================
--- test/CodeGenCXX/ubsan-suppress-checks.cpp
+++ test/CodeGenCXX/ubsan-suppress-checks.cpp
@@ -1,24 +1,28 @@
-// 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
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=alignment | FileCheck %s --check-prefixes=CHECK,ALIGN
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=null | FileCheck %s --check-prefixes=CHECK,NULL
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=alignment,null -DCHECK_LAMBDA | FileCheck %s --check-prefixes=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
+ // ALIGN: %[[THISINT1:[0-9]+]] = ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT1]], 3, !nosanitize
+ // NULL: icmp ne %struct.A* %[[THIS1:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.A* %[[THIS1]] to i64, !nosanitize
+ // CHECK: 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
+ // LAMBDA: %[[THISINT2:[0-9]+]] = ptrtoint %struct.A* %[[THIS2]] to i64, !nosanitize
+ // LAMBDA: and i64 %[[THISINT2]], 3, !nosanitize
+ // LAMBDA: call void @__ubsan_handle_type_mismatch
auto f = [&] {
foo = 0;
@@ -38,49 +42,59 @@
// 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
+ // ALIGN: %[[THISINT3:[0-9]+]] = ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT3]], 3, !nosanitize
+ // NULL: icmp ne %struct.A* %[[THIS3:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.A* %[[THIS3]] to i64, !nosanitize
+ // CHECK: 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
+ // ALIGN: %[[THISINT4:[0-9]+]] = ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT4]], 3, !nosanitize
+ // NULL: icmp ne %struct.A* %[[THIS4:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.A* %[[THIS4]] to i64, !nosanitize
+ // CHECK: 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
+ // ALIGN: %[[THISINT5:[0-9]+]] = ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT5]], 3, !nosanitize
+ // NULL: icmp ne %struct.A* %[[THIS5:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.A* %[[THIS5]] to i64, !nosanitize
+ // CHECK: 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
+ // ALIGN: %[[THISINT6:[0-9]+]] = ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT6]], 3, !nosanitize
+ // NULL: icmp ne %struct.A* %[[THIS6:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.A* %[[THIS6]] to i64, !nosanitize
+ // CHECK: 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
+ // ALIGN: %[[THISINT7:[0-9]+]] = ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT7]], 3, !nosanitize
+ // NULL: icmp ne %struct.A* %[[THIS7:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.A* %[[THIS7]] to i64, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
const_cast<A *>(this)->foo = 0;
// CHECK: ret void
@@ -106,10 +120,10 @@
// CHECK-LABEL: define linkonce_odr i32 @_ZN1B11load_memberEv
static int load_member() {
- // Null-check &b before converting it to an A*.
+ // 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 the result of the conversion before using it.
// CHECK: call void @__ubsan_handle_type_mismatch
//
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
@@ -130,11 +144,14 @@
// 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
+ // ALIGN: %[[THISINT8:[0-9]+]] = ptrtoint %struct.Derived* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT8]], 7, !nosanitize
+ // ALIGN: call void @__ubsan_handle_type_mismatch
+ // NULL: icmp ne %struct.Derived* %[[THIS8:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.Derived* %[[THIS8]] to i64, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch
//
- // Null-check the result of the cast before using it.
+ // Check the result of the cast before using it.
// CHECK: call void @__ubsan_handle_type_mismatch
//
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
@@ -144,19 +161,26 @@
// 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
+ // ALIGN: %[[THISINT9:[0-9]+]] = ptrtoint %struct.Derived* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT9]], 7, !nosanitize
+ // ALIGN: call void @__ubsan_handle_type_mismatch
+ // ALIGN: call void @__ubsan_handle_type_mismatch
+ // NULL: icmp ne %struct.Derived* %[[THIS9:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.Derived* %[[THIS9]] to i64, !nosanitize
+ // CHECK: 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
+ // ALIGN: %[[THISINT10:[0-9]+]] = ptrtoint %struct.Derived* %{{.*}} to i64, !nosanitize
+ // ALIGN: and i64 %[[THISINT10]], 7, !nosanitize
+ // ALIGN: call void @__ubsan_handle_type_mismatch
+ // NULL: icmp ne %struct.Derived* %[[THIS10:[a-z0-9]+]], null, !nosanitize
+ // NULL: ptrtoint %struct.Derived* %[[THIS10]] to i64, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch
// CHECK-NOT: call void @__ubsan_handle_type_mismatch
return foo + bar;
// CHECK: ret i32
Index: test/CodeGen/sanitize-recover.c
===================================================================
--- test/CodeGen/sanitize-recover.c
+++ test/CodeGen/sanitize-recover.c
@@ -22,15 +22,7 @@
// PARTIAL: %[[SIZE:.*]] = call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false)
// PARTIAL-NEXT: %[[CHECK0:.*]] = icmp uge i64 %[[SIZE]], 4
- // PARTIAL: %[[MISALIGN:.*]] = and i64 {{.*}}, 3
- // PARTIAL-NEXT: %[[CHECK1:.*]] = icmp eq i64 %[[MISALIGN]], 0
+ // PARTIAL: br i1 %[[CHECK0]], {{.*}} !nosanitize
- // PARTIAL: %[[CHECK01:.*]] = and i1 %[[CHECK1]], %[[CHECK0]]
-
- // PARTIAL: br i1 %[[CHECK01]], {{.*}} !nosanitize
- // PARTIAL: br i1 %[[CHECK1]], {{.*}} !nosanitize
-
- // 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
@@ -5,7 +5,7 @@
// CHECK-UBSAN: @[[INT:.*]] = private unnamed_addr constant { i16, i16, [6 x i8] } { i16 0, i16 11, [6 x i8] c"'int'\00" }
// FIXME: When we only emit each type once, use [[INT]] more below.
-// CHECK-UBSAN: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}} @[[INT]], i8 2, i8 1
+// CHECK-UBSAN: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}} @[[INT]], i8 1, i8 1
// CHECK-UBSAN: @[[LINE_200:.*]] = {{.*}}, i32 200, i32 10 {{.*}}, i8 2, i8 0
// CHECK-UBSAN: @[[LINE_300:.*]] = {{.*}}, i32 300, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}}
// CHECK-UBSAN: @[[LINE_400:.*]] = {{.*}}, i32 400, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}}
@@ -38,14 +38,7 @@
// 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: %[[PTRTOINT:.*]] = ptrtoint {{.*}}* %[[PTR]] to i64
- // CHECK-COMMON-NEXT: %[[MISALIGN:.*]] = and i64 %[[PTRTOINT]], 3
- // CHECK-COMMON-NEXT: %[[CHECK1:.*]] = icmp eq i64 %[[MISALIGN]], 0
-
- // CHECK-COMMON: %[[OK:.*]] = and i1 %[[CHECK0]], %[[CHECK1]]
-
- // CHECK-UBSAN: br i1 %[[OK]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
- // CHECK-TRAP: br i1 %[[OK]], {{.*}}
+ // CHECK-UBSAN: br i1 %[[CHECK0]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
// CHECK-UBSAN: %[[ARG:.*]] = ptrtoint {{.*}} %[[PTR]] to i64
// CHECK-UBSAN-NEXT: call void @__ubsan_handle_type_mismatch_v1(i8* bitcast ({{.*}} @[[LINE_100]] to i8*), i64 %[[ARG]])
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -949,13 +949,14 @@
CXXThisValue = CXXABIThisValue;
}
- // Null-check the 'this' pointer once per function, if it's available.
+ // Check the 'this' pointer once per function, if it's available.
if (CXXThisValue) {
SanitizerSet SkippedChecks;
- SkippedChecks.set(SanitizerKind::Alignment, true);
SkippedChecks.set(SanitizerKind::ObjectSize, true);
- EmitTypeCheck(TCK_Load, Loc, CXXThisValue, MD->getThisType(getContext()),
- /*Alignment=*/CharUnits::Zero(), SkippedChecks);
+ QualType ThisTy = MD->getThisType(getContext());
+ EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
+ getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
+ SkippedChecks);
}
}
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -291,9 +291,12 @@
CallLoc = CE->getExprLoc();
SanitizerSet SkippedChecks;
- if (const auto *CMCE = dyn_cast<CXXMemberCallExpr>(CE))
- if (IsDeclRefOrWrappedCXXThis(CMCE->getImplicitObjectArgument()))
+ if (const auto *CMCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+ if (IsDeclRefOrWrappedCXXThis(CMCE->getImplicitObjectArgument())) {
+ SkippedChecks.set(SanitizerKind::Alignment, true);
SkippedChecks.set(SanitizerKind::Null, true);
+ }
+ }
EmitTypeCheck(
isa<CXXConstructorDecl>(CalleeDecl) ? CodeGenFunction::TCK_ConstructorCall
: CodeGenFunction::TCK_MemberCall,
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -986,9 +986,12 @@
LV = EmitLValue(E);
if (!isa<DeclRefExpr>(E) && !LV.isBitField() && LV.isSimple()) {
SanitizerSet SkippedChecks;
- if (const auto *ME = dyn_cast<MemberExpr>(E))
- if (IsDeclRefOrWrappedCXXThis(ME->getBase()))
+ if (const auto *ME = dyn_cast<MemberExpr>(E)) {
+ if (IsDeclRefOrWrappedCXXThis(ME->getBase())) {
+ SkippedChecks.set(SanitizerKind::Alignment, true);
SkippedChecks.set(SanitizerKind::Null, true);
+ }
+ }
EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(),
E->getType(), LV.getAlignment(), SkippedChecks);
}
@@ -3372,8 +3375,10 @@
Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource);
QualType PtrTy = BaseExpr->getType()->getPointeeType();
SanitizerSet SkippedChecks;
- if (IsDeclRefOrWrappedCXXThis(BaseExpr))
+ if (IsDeclRefOrWrappedCXXThis(BaseExpr)) {
+ SkippedChecks.set(SanitizerKind::Alignment, true);
SkippedChecks.set(SanitizerKind::Null, true);
+ }
EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy,
/*Alignment=*/CharUnits::Zero(), SkippedChecks);
BaseLV = MakeAddrLValue(Addr, PtrTy, AlignSource);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
