On Wed, Nov 05, 2014 at 11:08:27PM +0100, Jakub Jelinek wrote:
> I wonder if for the case where you don't remove, gimple_bb (g) == gimple_bb
> (stmt) and tree_int_cst_compare (cur_align, align) == 0 it wouldn't be
> worthwhile to v.pop (); so that the vector doesn't grow up unnecessarily too
Good idea. I've added that.
> much. Maybe also bump the 30 limit to 100-200? Now that you don't walk the
> whole vector each time, it is only a matter of memory consumption (perhaps
> instead of having a constant limit for every vector it might be better to
> just have a limit on the total size of all the sanopt vectors (say a few
> megabytes)? And count towards that limit the allocated () size of the
> vector, once you have some size allocated, not pushing stuff in there
> doesn't make you consume more memory. Or just don't have a limit at all.
I removed the limit altogether. I believe the vectors won't grow too much
anyway.
Thanks for review.
ubsan.exp passed, bootstrap-ubsan running - ok if it passes?
2014-11-06 Marek Polacek <pola...@redhat.com>
* sanopt.c (sanopt_optimize_walker): Limit removal of the checks.
Remove vector limit.
testsuite/
* c-c++-common/ubsan/align-2.c: Add dg-output.
* c-c++-common/ubsan/align-4.c: Likewise.
* c-c++-common/ubsan/align-6.c: New test.
* c-c++-common/ubsan/align-7.c: New test.
* c-c++-common/ubsan/align-8.c: New test.
* g++.dg/ubsan/null-1.C: Add dg-output.
* g++.dg/ubsan/null-2.C: Likewise.
* g++.dg/ubsan/null-3.C: New test.
* g++.dg/ubsan/null-4.C: New test.
* g++.dg/ubsan/null-5.C: New test.
diff --git gcc/sanopt.c gcc/sanopt.c
index 4483ff7..0fc032a 100644
--- gcc/sanopt.c
+++ gcc/sanopt.c
@@ -130,7 +130,30 @@ sanopt_optimize_walker (basic_block bb, struct sanopt_ctx
*ctx)
/* At this point we shouldn't have any statements
that aren't dominating the current BB. */
tree align = gimple_call_arg (g, 2);
- remove = tree_int_cst_le (cur_align, align);
+ int kind = tree_to_shwi (gimple_call_arg (g, 1));
+ /* If this is a NULL pointer check where we had segv
+ anyway, we can remove it. */
+ if (integer_zerop (align)
+ && (kind == UBSAN_LOAD_OF
+ || kind == UBSAN_STORE_OF
+ || kind == UBSAN_MEMBER_ACCESS))
+ remove = true;
+ /* Otherwise remove the check in non-recovering
+ mode, or if the stmts have same location. */
+ else if (integer_zerop (align))
+ remove = !(flag_sanitize_recover & SANITIZE_NULL)
+ || flag_sanitize_undefined_trap_on_error
+ || gimple_location (g)
+ == gimple_location (stmt);
+ else if (tree_int_cst_le (cur_align, align))
+ remove = !(flag_sanitize_recover
+ & SANITIZE_ALIGNMENT)
+ || flag_sanitize_undefined_trap_on_error
+ || gimple_location (g)
+ == gimple_location (stmt);
+ if (!remove && gimple_bb (g) == gimple_bb (stmt)
+ && tree_int_cst_compare (cur_align, align) == 0)
+ v.pop ();
break;
}
}
@@ -147,7 +170,7 @@ sanopt_optimize_walker (basic_block bb, struct sanopt_ctx
*ctx)
}
gsi_remove (&gsi, true);
}
- else if (v.length () < 30)
+ else
v.safe_push (stmt);
}
}
diff --git gcc/testsuite/c-c++-common/ubsan/align-2.c
gcc/testsuite/c-c++-common/ubsan/align-2.c
index 02a26e2..071de8c 100644
--- gcc/testsuite/c-c++-common/ubsan/align-2.c
+++ gcc/testsuite/c-c++-common/ubsan/align-2.c
@@ -51,4 +51,6 @@ main ()
/* { dg-output "\.c:(13|16):\[0-9]*: \[^\n\r]*store to misaligned address
0x\[0-9a-fA-F]* for type 'int', which requires 4 byte alignment.*" } */
/* { dg-output "\.c:23:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
/* { dg-output "\.c:(29|30):\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
+/* { dg-output "\.c:30:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
+/* { dg-output "\.c:31:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
/* { dg-output "\.c:37:\[0-9]*: \[^\n\r]*load of misaligned address
0x\[0-9a-fA-F]* for type 'long long int', which requires \[48] byte alignment"
} */
diff --git gcc/testsuite/c-c++-common/ubsan/align-4.c
gcc/testsuite/c-c++-common/ubsan/align-4.c
index f010589..3252595 100644
--- gcc/testsuite/c-c++-common/ubsan/align-4.c
+++ gcc/testsuite/c-c++-common/ubsan/align-4.c
@@ -9,4 +9,6 @@
/* { dg-output "\[^\n\r]*\.c:(13|16):\[0-9]*: \[^\n\r]*store to misaligned
address 0x\[0-9a-fA-F]* for type 'int', which requires 4 byte alignment.*" } */
/* { dg-output "\[^\n\r]*\.c:23:\[0-9]*: \[^\n\r]*member access within
misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48]
byte alignment.*" } */
/* { dg-output "\[^\n\r]*\.c:(29|30):\[0-9]*: \[^\n\r]*member access within
misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48]
byte alignment.*" } */
+/* { dg-output "\[^\n\r]*\.c:30:\[0-9]*: \[^\n\r]*member access within
misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48]
byte alignment.*" } */
+/* { dg-output "\[^\n\r]*\.c:31:\[0-9]*: \[^\n\r]*member access within
misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48]
byte alignment.*" } */
/* { dg-output "\[^\n\r]*\.c:37:\[0-9]*: \[^\n\r]*load of misaligned address
0x\[0-9a-fA-F]* for type 'long long int', which requires \[48] byte alignment"
} */
diff --git gcc/testsuite/c-c++-common/ubsan/align-6.c
gcc/testsuite/c-c++-common/ubsan/align-6.c
index e69de29..5521292 100644
--- gcc/testsuite/c-c++-common/ubsan/align-6.c
+++ gcc/testsuite/c-c++-common/ubsan/align-6.c
@@ -0,0 +1,33 @@
+/* Limit this to known non-strict alignment targets. */
+/* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */
+/* { dg-options "-O -fsanitize=alignment -fsanitize-recover=alignment" } */
+
+struct S { int a; char b; long long c; short d[10]; };
+struct T { char a; long long b; };
+struct U { char a; int b; int c; long long d; struct S e; struct T f; }
__attribute__((packed));
+struct V { long long a; struct S b; struct T c; struct U u; } v;
+
+__attribute__((noinline, noclone)) int
+foo (struct S *p)
+{
+ volatile int i;
+ i = p->a;
+ i = p->a;
+ i = p->a;
+ i = p->a;
+ return p->a;
+}
+
+int
+main ()
+{
+ if (foo (&v.u.e))
+ __builtin_abort ();
+ return 0;
+}
+
+/* { dg-output "\.c:14:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
+/* { dg-output "\.c:15:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
+/* { dg-output "\.c:16:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
+/* { dg-output "\.c:17:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
+/* { dg-output "\.c:18:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
diff --git gcc/testsuite/c-c++-common/ubsan/align-7.c
gcc/testsuite/c-c++-common/ubsan/align-7.c
index e69de29..4a18d8d 100644
--- gcc/testsuite/c-c++-common/ubsan/align-7.c
+++ gcc/testsuite/c-c++-common/ubsan/align-7.c
@@ -0,0 +1,32 @@
+/* Limit this to known non-strict alignment targets. */
+/* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */
+/* { dg-options "-O -fsanitize=alignment -fno-sanitize-recover=alignment
-fdump-tree-sanopt-details" } */
+/* { dg-shouldfail "ubsan" } */
+
+struct S { int a; char b; long long c; short d[10]; };
+struct T { char a; long long b; };
+struct U { char a; int b; int c; long long d; struct S e; struct T f; }
__attribute__((packed));
+struct V { long long a; struct S b; struct T c; struct U u; } v;
+
+__attribute__((noinline, noclone)) int
+foo (struct S *p)
+{
+ volatile int i;
+ i = p->a;
+ i = p->a;
+ i = p->a;
+ i = p->a;
+ return p->a;
+}
+
+int
+main ()
+{
+ if (foo (&v.u.e))
+ __builtin_abort ();
+ return 0;
+}
+
+/* { dg-output "\.c:15:\[0-9]*: \[^\n\r]*member access within misaligned
address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte
alignment.*" } */
+/* { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} } */
+/* { dg-final { cleanup-tree-dump "sanopt" } } */
diff --git gcc/testsuite/c-c++-common/ubsan/align-8.c
gcc/testsuite/c-c++-common/ubsan/align-8.c
index e69de29..b930162 100644
--- gcc/testsuite/c-c++-common/ubsan/align-8.c
+++ gcc/testsuite/c-c++-common/ubsan/align-8.c
@@ -0,0 +1,31 @@
+/* Limit this to known non-strict alignment targets. */
+/* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */
+/* { dg-options "-O -fsanitize=alignment -fsanitize-undefined-trap-on-error
-fdump-tree-sanopt-details" } */
+/* { dg-shouldfail "ubsan" } */
+
+struct S { int a; char b; long long c; short d[10]; };
+struct T { char a; long long b; };
+struct U { char a; int b; int c; long long d; struct S e; struct T f; }
__attribute__((packed));
+struct V { long long a; struct S b; struct T c; struct U u; } v;
+
+__attribute__((noinline, noclone)) int
+foo (struct S *p)
+{
+ volatile int i;
+ i = p->a;
+ i = p->a;
+ i = p->a;
+ i = p->a;
+ return p->a;
+}
+
+int
+main ()
+{
+ if (foo (&v.u.e))
+ __builtin_abort ();
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} } */
+/* { dg-final { cleanup-tree-dump "sanopt" } } */
diff --git gcc/testsuite/g++.dg/ubsan/null-1.C
gcc/testsuite/g++.dg/ubsan/null-1.C
index 83b3033..e1524b1 100644
--- gcc/testsuite/g++.dg/ubsan/null-1.C
+++ gcc/testsuite/g++.dg/ubsan/null-1.C
@@ -25,4 +25,6 @@ main (void)
}
// { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*reference binding to null pointer of type
'int'(\n|\r\n|\r)" }
// { dg-output "\[^\n\r]*reference binding to null pointer of type 'const
L'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*reference binding to null pointer of type
'int'(\n|\r\n|\r)" }
diff --git gcc/testsuite/g++.dg/ubsan/null-2.C
gcc/testsuite/g++.dg/ubsan/null-2.C
index 0230c7c..88f387e 100644
--- gcc/testsuite/g++.dg/ubsan/null-2.C
+++ gcc/testsuite/g++.dg/ubsan/null-2.C
@@ -35,3 +35,5 @@ main (void)
// { dg-output "\.C:26:\[0-9]*:\[\^\n\r]*member call on null pointer of type
'struct U'.*" }
// { dg-output "\.C:29:\[0-9]*:\[\^\n\r]*member call on null pointer of type
'struct V'.*" }
+// { dg-output "\.C:31:\[0-9]*:\[\^\n\r]*member call on null pointer of type
'struct V'.*" }
+// { dg-output "\.C:33:\[0-9]*:\[\^\n\r]*member call on null pointer of type
'struct V'" }
diff --git gcc/testsuite/g++.dg/ubsan/null-3.C
gcc/testsuite/g++.dg/ubsan/null-3.C
index e69de29..cd3716b 100644
--- gcc/testsuite/g++.dg/ubsan/null-3.C
+++ gcc/testsuite/g++.dg/ubsan/null-3.C
@@ -0,0 +1,20 @@
+// { dg-do run }
+// { dg-options "-fsanitize=null" }
+
+int
+main (void)
+{
+ int *p = 0;
+
+ int &r1 = *p;
+ int &r2 = *p;
+ int &r3 = *p;
+ int &r4 = *p;
+ int &r5 = *p;
+}
+
+// { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*reference binding to null pointer of type
'int'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*reference binding to null pointer of type
'int'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*reference binding to null pointer of type
'int'(\n|\r\n|\r)" }
+// { dg-output "\[^\n\r]*reference binding to null pointer of type
'int'(\n|\r\n|\r)" }
diff --git gcc/testsuite/g++.dg/ubsan/null-4.C
gcc/testsuite/g++.dg/ubsan/null-4.C
index e69de29..9cb04ef 100644
--- gcc/testsuite/g++.dg/ubsan/null-4.C
+++ gcc/testsuite/g++.dg/ubsan/null-4.C
@@ -0,0 +1,19 @@
+// { dg-do run }
+// { dg-options "-O -fsanitize=null -fno-sanitize-recover=null
-fdump-tree-sanopt-details" }
+// { dg-shouldfail "ubsan" }
+
+int
+main (void)
+{
+ int *p = 0;
+
+ int &r1 = *p;
+ int &r2 = *p;
+ int &r3 = *p;
+ int &r4 = *p;
+ int &r5 = *p;
+}
+
+// { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" }
+// { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} }
+// { dg-final { cleanup-tree-dump "sanopt" } }
diff --git gcc/testsuite/g++.dg/ubsan/null-5.C
gcc/testsuite/g++.dg/ubsan/null-5.C
index e69de29..d8e4a68 100644
--- gcc/testsuite/g++.dg/ubsan/null-5.C
+++ gcc/testsuite/g++.dg/ubsan/null-5.C
@@ -0,0 +1,18 @@
+// { dg-do run }
+// { dg-options "-O -fsanitize=null -fsanitize-undefined-trap-on-error
-fdump-tree-sanopt-details" }
+// { dg-shouldfail "ubsan" }
+
+int
+main (void)
+{
+ int *p = 0;
+
+ int &r1 = *p;
+ int &r2 = *p;
+ int &r3 = *p;
+ int &r4 = *p;
+ int &r5 = *p;
+}
+
+// { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} }
+// { dg-final { cleanup-tree-dump "sanopt" } }
Marek
