* [PATCH] Optimize UBSAN_NULL checks
@ 2014-10-30 19:15 Marek Polacek
2014-10-30 19:46 ` Marek Polacek
2014-10-31 9:35 ` Marek Polacek
0 siblings, 2 replies; 6+ messages in thread
From: Marek Polacek @ 2014-10-30 19:15 UTC (permalink / raw)
To: GCC Patches, Jakub Jelinek
This patch tries to optimize away redundant UBSAN_NULL checks.
It walks the statements, looks for UBSAN_NULL calls and keeps
track of pointers and statements checking that pointer in a
hash map. Now, if we can prove that some UBSAN_NULL stmt is
dominated by other one which requires the same or less strict
alignment, there's no point in keeping this check around and
expanding it.
optimize_checks should be enhanced to handle other {ub,a,t}san
checks as well - which is what I'm going to work on next.
Bootstrapped(-ubsan)/regtested on x86_64-linux, ok for trunk?
2014-10-30 Marek Polacek <polacek@redhat.com>
* asan.c (optimize_checks): New function.
(pass_sanopt::execute): Call it.
testsuite/
* c-c++-common/ubsan/align-2.c: Remove dg-output.
* c-c++-common/ubsan/align-4.c: Likewise.
* g++.dg/ubsan/null-1.C: Likewise.
* g++.dg/ubsan/null-2.C: Likewise.
diff --git gcc/asan.c gcc/asan.c
index b4b0822..c06bb49 100644
--- gcc/asan.c
+++ gcc/asan.c
@@ -2707,6 +2707,99 @@ asan_expand_check_ifn (gimple_stmt_iterator *iter, bool use_calls)
return true;
}
+/* Try to remove redundant sanitizer checks in function FUN. */
+
+void
+optimize_checks (function *fun)
+{
+ basic_block bb;
+
+ /* This map maps a pointer (the first argument of UBSAN_NULL) to
+ a vector of UBSAN_NULL call statements that check this pointer. */
+ hash_map<tree, auto_vec<gimple> > *null_check_map = NULL;
+ calculate_dominance_info (CDI_DOMINATORS);
+
+ FOR_EACH_BB_FN (bb, fun)
+ {
+ gimple_stmt_iterator gsi;
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
+ {
+ gimple stmt = gsi_stmt (gsi);
+ bool remove = false;
+
+ if (is_gimple_call (stmt)
+ && gimple_call_internal_p (stmt))
+ switch (gimple_call_internal_fn (stmt))
+ {
+ case IFN_UBSAN_NULL:
+ {
+ gcc_assert (gimple_call_num_args (stmt) == 3);
+ tree ptr = gimple_call_arg (stmt, 0);
+ tree cur_align = gimple_call_arg (stmt, 2);
+ gcc_assert (TREE_CODE (cur_align) == INTEGER_CST);
+
+ if (null_check_map == NULL)
+ null_check_map = new hash_map<tree, auto_vec<gimple> >;
+
+ auto_vec<gimple> &v = null_check_map->get_or_insert (ptr);
+ if (v.is_empty ())
+ /* For this PTR we don't have any UBSAN_NULL stmts
+ recorded, so there's nothing to optimize yet. */
+ v.safe_push (stmt);
+ else
+ {
+ /* We already have recorded a UBSAN_NULL check
+ for this pointer. Perhaps we can drop this one.
+ But only if this check doesn't specify stricter
+ alignment, and is dominated by an earlier stmt. */
+ int i;
+ gimple g;
+
+ FOR_EACH_VEC_ELT (v, i, g)
+ {
+ tree align = gimple_call_arg (g, 2);
+ basic_block gbb = gimple_bb (g);
+ if (tree_int_cst_le (cur_align, align)
+ && dominated_by_p (CDI_DOMINATORS, bb, gbb))
+ {
+ remove = true;
+ break;
+ }
+ }
+
+ if (remove)
+ {
+ /* Drop this check. */
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "Optimizing out\n ");
+ print_gimple_stmt (dump_file, stmt, 0,
+ dump_flags);
+ fprintf (dump_file, "\n");
+ }
+ gsi_remove (&gsi, true);
+ }
+ else if (v.length () < 30)
+ v.safe_push (stmt);
+ }
+ break;
+ }
+ default:
+ break;
+ }
+
+ /* If we were able to remove current statement, gsi_remove
+ already pointed us to the next statement. */
+ if (!remove)
+ gsi_next (&gsi);
+ }
+ }
+
+ delete null_check_map;
+ null_check_map = NULL;
+ free_dominance_info (CDI_DOMINATORS);
+}
+
/* Instrument the current function. */
static unsigned int
@@ -2834,6 +2927,10 @@ pass_sanopt::execute (function *fun)
{
basic_block bb;
+ /* Try to remove redundant checks. */
+ if (optimize && (flag_sanitize & (SANITIZE_NULL | SANITIZE_ALIGNMENT)))
+ optimize_checks (fun);
+
int asan_num_accesses = 0;
if (flag_sanitize & SANITIZE_ADDRESS)
{
@@ -2889,7 +2986,7 @@ pass_sanopt::execute (function *fun)
if (dump_file && (dump_flags & TDF_DETAILS))
{
- fprintf (dump_file, "Optimized\n ");
+ fprintf (dump_file, "Expanded\n ");
print_gimple_stmt (dump_file, stmt, 0, dump_flags);
fprintf (dump_file, "\n");
}
diff --git gcc/testsuite/c-c++-common/ubsan/align-2.c gcc/testsuite/c-c++-common/ubsan/align-2.c
index 071de8c..02a26e2 100644
--- gcc/testsuite/c-c++-common/ubsan/align-2.c
+++ gcc/testsuite/c-c++-common/ubsan/align-2.c
@@ -51,6 +51,4 @@ 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 3252595..f010589 100644
--- gcc/testsuite/c-c++-common/ubsan/align-4.c
+++ gcc/testsuite/c-c++-common/ubsan/align-4.c
@@ -9,6 +9,4 @@
/* { 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/g++.dg/ubsan/null-1.C gcc/testsuite/g++.dg/ubsan/null-1.C
index e1524b1..83b3033 100644
--- gcc/testsuite/g++.dg/ubsan/null-1.C
+++ gcc/testsuite/g++.dg/ubsan/null-1.C
@@ -25,6 +25,4 @@ 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 88f387e..0230c7c 100644
--- gcc/testsuite/g++.dg/ubsan/null-2.C
+++ gcc/testsuite/g++.dg/ubsan/null-2.C
@@ -35,5 +35,3 @@ 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'" }
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Optimize UBSAN_NULL checks
2014-10-30 19:15 [PATCH] Optimize UBSAN_NULL checks Marek Polacek
@ 2014-10-30 19:46 ` Marek Polacek
2014-10-30 21:41 ` ygribov
2014-10-31 9:35 ` Marek Polacek
1 sibling, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2014-10-30 19:46 UTC (permalink / raw)
To: GCC Patches, Jakub Jelinek
And I wonder whether it'd be worth it to create sanopt.c - and move
.sanopt related stuff there (it'd mean making asan_expand_check_ifn
non-static, but that's pretty much it).
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Optimize UBSAN_NULL checks
2014-10-30 19:46 ` Marek Polacek
@ 2014-10-30 21:41 ` ygribov
0 siblings, 0 replies; 6+ messages in thread
From: ygribov @ 2014-10-30 21:41 UTC (permalink / raw)
To: gcc-patches
> And I wonder whether it'd be worth it to create sanopt.c -
> and move sanopt related stuff there
+1
--
View this message in context: http://gcc.1065356.n5.nabble.com/PATCH-Optimize-UBSAN-NULL-checks-tp1084891p1084905.html
Sent from the gcc - patches mailing list archive at Nabble.com.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Optimize UBSAN_NULL checks
2014-10-30 19:15 [PATCH] Optimize UBSAN_NULL checks Marek Polacek
2014-10-30 19:46 ` Marek Polacek
@ 2014-10-31 9:35 ` Marek Polacek
2014-10-31 19:54 ` ygribov
2014-10-31 20:14 ` ygribov
1 sibling, 2 replies; 6+ messages in thread
From: Marek Polacek @ 2014-10-31 9:35 UTC (permalink / raw)
To: GCC Patches, Jakub Jelinek
On Thu, Oct 30, 2014 at 07:47:52PM +0100, Marek Polacek wrote:
> This patch tries to optimize away redundant UBSAN_NULL checks.
> It walks the statements, looks for UBSAN_NULL calls and keeps
> track of pointers and statements checking that pointer in a
> hash map. Now, if we can prove that some UBSAN_NULL stmt is
> dominated by other one which requires the same or less strict
> alignment, there's no point in keeping this check around and
> expanding it.
>
> optimize_checks should be enhanced to handle other {ub,a,t}san
> checks as well - which is what I'm going to work on next.
(Strike this version. I'm working on a variant that walks the dominator
tree first to get better optimizations.)
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Optimize UBSAN_NULL checks
2014-10-31 9:35 ` Marek Polacek
@ 2014-10-31 19:54 ` ygribov
2014-10-31 20:14 ` ygribov
1 sibling, 0 replies; 6+ messages in thread
From: ygribov @ 2014-10-31 19:54 UTC (permalink / raw)
To: gcc-patches
On Fri, Oct 31, 2014 at 12:19 PM, Marek Polacek-3 [via gcc]
<ml-node+s1065356n1085049h67@n5.nabble.com> wrote:
> On Thu, Oct 30, 2014 at 07:47:52PM +0100, Marek Polacek wrote:
>
>> This patch tries to optimize away redundant UBSAN_NULL checks.
>> It walks the statements, looks for UBSAN_NULL calls and keeps
>> track of pointers and statements checking that pointer in a
>> hash map. Now, if we can prove that some UBSAN_NULL stmt is
>> dominated by other one which requires the same or less strict
>> alignment, there's no point in keeping this check around and
>> expanding it.
>>
>> optimize_checks should be enhanced to handle other {ub,a,t}san
>> checks as well - which is what I'm going to work on next.
>
> (Strike this version. I'm working on a variant that walks the dominator
> tree first to get better optimizations.)
Just curious how much speedup did you get from this? I've tried
similar optimizations and got pitiful 3% speedup.
-Y
--
View this message in context: http://gcc.1065356.n5.nabble.com/PATCH-Optimize-UBSAN-NULL-checks-tp1084891p1085286.html
Sent from the gcc - patches mailing list archive at Nabble.com.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Optimize UBSAN_NULL checks
2014-10-31 9:35 ` Marek Polacek
2014-10-31 19:54 ` ygribov
@ 2014-10-31 20:14 ` ygribov
1 sibling, 0 replies; 6+ messages in thread
From: ygribov @ 2014-10-31 20:14 UTC (permalink / raw)
To: gcc-patches
On Fri, Oct 31, 2014 at 10:51 PM, Yuri Gribov <tetra2005@gmail.com> wrote:
> I've tried similar optimizations
For Asan that is.
--
View this message in context: http://gcc.1065356.n5.nabble.com/PATCH-Optimize-UBSAN-NULL-checks-tp1084891p1085287.html
Sent from the gcc - patches mailing list archive at Nabble.com.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-31 19:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-30 19:15 [PATCH] Optimize UBSAN_NULL checks Marek Polacek
2014-10-30 19:46 ` Marek Polacek
2014-10-30 21:41 ` ygribov
2014-10-31 9:35 ` Marek Polacek
2014-10-31 19:54 ` ygribov
2014-10-31 20:14 ` ygribov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).