vsk created this revision.
UBSan's alignment check does not detect unaligned loads and stores from
extern struct declarations. Example:
struct S { int i; };
extern struct S g_S; // &g_S may not be aligned properly.
int main() {
return g_S.i; // No alignment check for &g_S.i is emitted.
}
The frontend skips the alignment check for &g_S.i because the check is
constant-folded to 'NOT NULL' by the IR builder.
If we drop the alignment information from extern struct declarations,
the IR builder would no longer be able to constant-fold the checks away.
That would make the alignment check more useful.
Note: it would still not be able to catch the following case --
extern int i; // &i is not aligned properly.
PR: https://bugs.llvm.org/show_bug.cgi?id=32630
Testing: check-clang, check-ubsan.
https://reviews.llvm.org/D32456
Files:
lib/CodeGen/CodeGenModule.cpp
test/CodeGenCXX/ubsan-global-alignment.cpp
Index: test/CodeGenCXX/ubsan-global-alignment.cpp
===================================================================
--- test/CodeGenCXX/ubsan-global-alignment.cpp
+++ test/CodeGenCXX/ubsan-global-alignment.cpp
@@ -5,18 +5,27 @@
};
extern S g_S;
+__private_extern__ S pe_S;
extern S array_S[];
-// CHECK-LABEL: define i32 @_Z18load_extern_global
-int load_extern_global() {
- // FIXME: The IR builder constant-folds the alignment check away to 'true'
- // here, so we never call the diagnostic. This is PR32630.
- // CHECK-NOT: ptrtoint i32* {{.*}} to i32, !nosanitize
+// CHECK-LABEL: define i32 @_Z19load_extern_global1
+int load_extern_global1() {
+ // CHECK: br i1 icmp eq (i64 and (i64 ptrtoint (%struct.S* @g_S to i64), i64
3), i64 0), {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch
// CHECK: [[I:%.*]] = load i32, i32* getelementptr inbounds (%struct.S,
%struct.S* @g_S, i32 0, i32 0), align 4
// CHECK-NEXT: ret i32 [[I]]
return g_S.I;
}
+// CHECK-LABEL: define i32 @_Z19load_extern_global2
+int load_extern_global2() {
+ // CHECK: br i1 icmp eq (i64 and (i64 ptrtoint (%struct.S* @pe_S to i64),
i64 3), i64 0), {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch
+ // CHECK: [[I:%.*]] = load i32, i32* getelementptr inbounds (%struct.S,
%struct.S* @pe_S, i32 0, i32 0), align 4
+ // CHECK-NEXT: ret i32 [[I]]
+ return pe_S.I;
+}
+
// CHECK-LABEL: define i32 @_Z22load_from_extern_array
int load_from_extern_array(int I) {
// CHECK: [[I:%.*]] = getelementptr inbounds %struct.S, %struct.S* {{.*}},
i32 0, i32 0
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2312,7 +2312,14 @@
// handling.
GV->setConstant(isTypeConstant(D->getType(), false));
- GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
+ if (LangOpts.Sanitize.has(SanitizerKind::Alignment) && !IsForDefinition &&
+ (D->getStorageClass() == SC_Extern ||
+ D->getStorageClass() == SC_PrivateExtern)) {
+ // The alignment sanitizer can catch more bugs if we don't attach
+ // alignment info to extern globals.
+ } else {
+ GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
+ }
setLinkageAndVisibilityForGV(GV, D);
Index: test/CodeGenCXX/ubsan-global-alignment.cpp
===================================================================
--- test/CodeGenCXX/ubsan-global-alignment.cpp
+++ test/CodeGenCXX/ubsan-global-alignment.cpp
@@ -5,18 +5,27 @@
};
extern S g_S;
+__private_extern__ S pe_S;
extern S array_S[];
-// CHECK-LABEL: define i32 @_Z18load_extern_global
-int load_extern_global() {
- // FIXME: The IR builder constant-folds the alignment check away to 'true'
- // here, so we never call the diagnostic. This is PR32630.
- // CHECK-NOT: ptrtoint i32* {{.*}} to i32, !nosanitize
+// CHECK-LABEL: define i32 @_Z19load_extern_global1
+int load_extern_global1() {
+ // CHECK: br i1 icmp eq (i64 and (i64 ptrtoint (%struct.S* @g_S to i64), i64 3), i64 0), {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch
// CHECK: [[I:%.*]] = load i32, i32* getelementptr inbounds (%struct.S, %struct.S* @g_S, i32 0, i32 0), align 4
// CHECK-NEXT: ret i32 [[I]]
return g_S.I;
}
+// CHECK-LABEL: define i32 @_Z19load_extern_global2
+int load_extern_global2() {
+ // CHECK: br i1 icmp eq (i64 and (i64 ptrtoint (%struct.S* @pe_S to i64), i64 3), i64 0), {{.*}}, !nosanitize
+ // CHECK: call void @__ubsan_handle_type_mismatch
+ // CHECK: [[I:%.*]] = load i32, i32* getelementptr inbounds (%struct.S, %struct.S* @pe_S, i32 0, i32 0), align 4
+ // CHECK-NEXT: ret i32 [[I]]
+ return pe_S.I;
+}
+
// CHECK-LABEL: define i32 @_Z22load_from_extern_array
int load_from_extern_array(int I) {
// CHECK: [[I:%.*]] = getelementptr inbounds %struct.S, %struct.S* {{.*}}, i32 0, i32 0
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2312,7 +2312,14 @@
// handling.
GV->setConstant(isTypeConstant(D->getType(), false));
- GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
+ if (LangOpts.Sanitize.has(SanitizerKind::Alignment) && !IsForDefinition &&
+ (D->getStorageClass() == SC_Extern ||
+ D->getStorageClass() == SC_PrivateExtern)) {
+ // The alignment sanitizer can catch more bugs if we don't attach
+ // alignment info to extern globals.
+ } else {
+ GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
+ }
setLinkageAndVisibilityForGV(GV, D);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
