[PATCH] D30285: [ubsan] Don't check alignment if the alignment is 1

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300371: [ubsan] Don't check alignment if the alignment is 1 
(authored by vedantk).

Changed prior to commit:
  https://reviews.llvm.org/D30285?vs=89458=95353#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30285

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp
  cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp


Index: cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp
===
--- cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp
@@ -133,7 +133,7 @@
 // CHECK: call void @__ubsan_handle_type_mismatch
 //
 // Check the result of the conversion before using it.
-// CHECK: call void @__ubsan_handle_type_mismatch
+// NULL: call void @__ubsan_handle_type_mismatch
 //
 // CHECK-NOT: call void @__ubsan_handle_type_mismatch
 B b;
Index: cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
===
--- cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
@@ -5,16 +5,32 @@
 struct A {
   // COMMON-LABEL: define linkonce_odr void @_ZN1A10do_nothingEv
   void do_nothing() {
-// ALIGN: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
-// ALIGN: and i64 %{{.*}}, 0, !nosanitize
+// ALIGN-NOT: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
  
 // NULL: icmp ne %struct.A* %{{.*}}, null, !nosanitize
  
 // OBJSIZE-NOT: call i64 @llvm.objectsize
   }
 };
 
+struct B {
+  int x;
+
+  // COMMON-LABEL: define linkonce_odr void @_ZN1B10do_nothingEv
+  void do_nothing() {
+// ALIGN: ptrtoint %struct.B* %{{.*}} to i64, !nosanitize
+// ALIGN: and i64 %{{.*}}, 3, !nosanitize
+
+// NULL: icmp ne %struct.B* %{{.*}}, null, !nosanitize
+
+// OBJSIZE-NOT: call i64 @llvm.objectsize
+  }
+};
+
 void force_irgen() {
   A a;
   a.do_nothing();
+
+  B b;
+  b.do_nothing();
 }
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -599,7 +599,7 @@
   AlignVal = getContext().getTypeAlignInChars(Ty).getQuantity();
 
 // The glvalue must be suitably aligned.
-if (AlignVal) {
+if (AlignVal > 1) {
   llvm::Value *Align =
   Builder.CreateAnd(Builder.CreatePtrToInt(Ptr, IntPtrTy),
 llvm::ConstantInt::get(IntPtrTy, AlignVal - 1));


Index: cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp
===
--- cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp
@@ -133,7 +133,7 @@
 // CHECK: call void @__ubsan_handle_type_mismatch
 //
 // Check the result of the conversion before using it.
-// CHECK: call void @__ubsan_handle_type_mismatch
+// NULL: call void @__ubsan_handle_type_mismatch
 //
 // CHECK-NOT: call void @__ubsan_handle_type_mismatch
 B b;
Index: cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
===
--- cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
@@ -5,16 +5,32 @@
 struct A {
   // COMMON-LABEL: define linkonce_odr void @_ZN1A10do_nothingEv
   void do_nothing() {
-// ALIGN: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
-// ALIGN: and i64 %{{.*}}, 0, !nosanitize
+// ALIGN-NOT: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
  
 // NULL: icmp ne %struct.A* %{{.*}}, null, !nosanitize
  
 // OBJSIZE-NOT: call i64 @llvm.objectsize
   }
 };
 
+struct B {
+  int x;
+
+  // COMMON-LABEL: define linkonce_odr void @_ZN1B10do_nothingEv
+  void do_nothing() {
+// ALIGN: ptrtoint %struct.B* %{{.*}} to i64, !nosanitize
+// ALIGN: and i64 %{{.*}}, 3, !nosanitize
+
+// NULL: icmp ne %struct.B* %{{.*}}, null, !nosanitize
+
+// OBJSIZE-NOT: call i64 @llvm.objectsize
+  }
+};
+
 void force_irgen() {
   A a;
   a.do_nothing();
+
+  B b;
+  b.do_nothing();
 }
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -599,7 +599,7 @@
   AlignVal = getContext().getTypeAlignInChars(Ty).getQuantity();
 
 // The glvalue must be suitably aligned.
-if (AlignVal) {
+if (AlignVal > 1) {
   llvm::Value *Align =
   Builder.CreateAnd(Builder.CreatePtrToInt(Ptr, IntPtrTy),
 llvm::ConstantInt::get(IntPtrTy, AlignVal - 1));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30285: [ubsan] Don't check alignment if the alignment is 1

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab accepted this revision.
filcab added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D30285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30285: [ubsan] Don't check alignment if the alignment is 1

[Vedant Kumar via Phabricator via cfe-commits]

vsk created this revision.

If a pointer is 1-byte aligned, there's no use in checking its
alignment. Somewhat surprisingly, ubsan can spend a significant amount
of time doing just that!

This loosely depends on https://reviews.llvm.org/D30283.

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 with https://reviews.llvm.org/D30283 
applied. Here are
the number of alignment checks emitted:

| Setup  | # of alignment checks |
| unpatched + https://reviews.llvm.org/D30283, -O0 | 14307 |
| patched + https://reviews.llvm.org/D30283, -O0   | 12515 |


https://reviews.llvm.org/D30285

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/ubsan-suppress-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,16 +5,32 @@
 struct A {
   // COMMON-LABEL: define linkonce_odr void @_ZN1A10do_nothingEv
   void do_nothing() {
-// ALIGN: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
-// ALIGN: and i64 %{{.*}}, 0, !nosanitize
+// ALIGN-NOT: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
  
 // NULL: icmp ne %struct.A* %{{.*}}, null, !nosanitize
  
 // OBJSIZE-NOT: call i64 @llvm.objectsize
   }
 };
 
+struct B {
+  int x;
+
+  // COMMON-LABEL: define linkonce_odr void @_ZN1B10do_nothingEv
+  void do_nothing() {
+// ALIGN: ptrtoint %struct.B* %{{.*}} to i64, !nosanitize
+// ALIGN: and i64 %{{.*}}, 3, !nosanitize
+
+// NULL: icmp ne %struct.B* %{{.*}}, null, !nosanitize
+
+// OBJSIZE-NOT: call i64 @llvm.objectsize
+  }
+};
+
 void force_irgen() {
   A a;
   a.do_nothing();
+
+  B b;
+  b.do_nothing();
 }
Index: test/CodeGenCXX/ubsan-suppress-checks.cpp
===
--- test/CodeGenCXX/ubsan-suppress-checks.cpp
+++ test/CodeGenCXX/ubsan-suppress-checks.cpp
@@ -124,7 +124,7 @@
 // CHECK: call void @__ubsan_handle_type_mismatch
 //
 // Check the result of the conversion before using it.
-// CHECK: call void @__ubsan_handle_type_mismatch
+// NULL: call void @__ubsan_handle_type_mismatch
 //
 // CHECK-NOT: call void @__ubsan_handle_type_mismatch
 B b;
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -597,7 +597,7 @@
   AlignVal = getContext().getTypeAlignInChars(Ty).getQuantity();
 
 // The glvalue must be suitably aligned.
-if (AlignVal) {
+if (AlignVal > 1) {
   llvm::Value *Align =
   Builder.CreateAnd(Builder.CreatePtrToInt(Ptr, IntPtrTy),
 llvm::ConstantInt::get(IntPtrTy, AlignVal - 1));


Index: test/CodeGenCXX/ubsan-type-checks.cpp
===
--- test/CodeGenCXX/ubsan-type-checks.cpp
+++ test/CodeGenCXX/ubsan-type-checks.cpp
@@ -5,16 +5,32 @@
 struct A {
   // COMMON-LABEL: define linkonce_odr void @_ZN1A10do_nothingEv
   void do_nothing() {
-// ALIGN: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
-// ALIGN: and i64 %{{.*}}, 0, !nosanitize
+// ALIGN-NOT: ptrtoint %struct.A* %{{.*}} to i64, !nosanitize
  
 // NULL: icmp ne %struct.A* %{{.*}}, null, !nosanitize
  
 // OBJSIZE-NOT: call i64 @llvm.objectsize
   }
 };
 
+struct B {
+  int x;
+
+  // COMMON-LABEL: define linkonce_odr void @_ZN1B10do_nothingEv
+  void do_nothing() {
+// ALIGN: ptrtoint %struct.B* %{{.*}} to i64, !nosanitize
+// ALIGN: and i64 %{{.*}}, 3, !nosanitize
+
+// NULL: icmp ne %struct.B* %{{.*}}, null, !nosanitize
+
+// OBJSIZE-NOT: call i64 @llvm.objectsize
+  }
+};
+
 void force_irgen() {
   A a;
   a.do_nothing();
+
+  B b;
+  b.do_nothing();
 }
Index: test/CodeGenCXX/ubsan-suppress-checks.cpp
===
--- test/CodeGenCXX/ubsan-suppress-checks.cpp
+++ test/CodeGenCXX/ubsan-suppress-checks.cpp
@@ -124,7 +124,7 @@
 // CHECK: call void @__ubsan_handle_type_mismatch
 //
 // Check the result of the conversion before using it.
-// CHECK: call void @__ubsan_handle_type_mismatch
+// NULL: call void @__ubsan_handle_type_mismatch
 //
 // CHECK-NOT: call void @__ubsan_handle_type_mismatch
 B b;
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -597,7 +597,7 @@
   AlignVal = getContext().getTypeAlignInChars(Ty).getQuantity();
 
 // The glvalue must be suitably aligned.
-if (AlignVal) {
+if (AlignVal > 1) {
   llvm::Value *Align =
   Builder.CreateAnd(Builder.CreatePtrToInt(Ptr, IntPtrTy),