* [PATCH] Optimize BIT_AND_EXPRs for UBSAN_OBJECT_SIZE
@ 2014-11-03 20:47 Marek Polacek
2014-11-03 22:07 ` Jeff Law
2014-11-03 22:29 ` Jakub Jelinek
0 siblings, 2 replies; 3+ messages in thread
From: Marek Polacek @ 2014-11-03 20:47 UTC (permalink / raw)
To: GCC Patches, Jakub Jelinek
We don't emit UBSAN_BOUNDS checks in case we can at compile-time
prove that the array access is fine. Also if we have [i & CST],
where CST is <= bound_of_the_array, we know we're fine as well.
But we don't have similar BIT_AND_EXPR check for UBSAN_OBJECT_SIZE
which is what this patch attempts to add.
(This is unrelated to the UBSAN_NULL optimization I posted earlier
today.)
Bootstrap-ubsan/regtest passed on x86_64-linux, ok for trunk?
2014-11-03 Marek Polacek <polacek@redhat.com>
* ubsan.c (instrument_object_size): Optimize [x & CST] array accesses.
testsuite/
* c-c++-common/ubsan/object-size-10.c: New test.
diff --git gcc/testsuite/c-c++-common/ubsan/object-size-10.c gcc/testsuite/c-c++-common/ubsan/object-size-10.c
index e69de29..ebc8582 100644
--- gcc/testsuite/c-c++-common/ubsan/object-size-10.c
+++ gcc/testsuite/c-c++-common/ubsan/object-size-10.c
@@ -0,0 +1,79 @@
+/* { dg-do run } */
+/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */
+/* { dg-options "-fsanitize=undefined" } */
+
+static char a[128];
+static int b[128];
+
+__attribute__ ((noinline, noclone)) int
+fn1 (int i)
+{
+ asm ("");
+ return a[i & 127];
+}
+
+__attribute__ ((noinline, noclone)) int
+fn2 (int i)
+{
+ asm ("");
+ return a[i & 128];
+}
+
+/* { dg-output "index 128 out of bounds for type 'char \\\[128\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'char'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */
+
+__attribute__ ((noinline, noclone)) int
+fn3 (int i)
+{
+ asm ("");
+ return b[i & 127];
+}
+
+__attribute__ ((noinline, noclone)) int
+fn4 (int i)
+{
+ asm ("");
+ return b[i & 128];
+}
+
+/* { dg-output "\[^\n\r]*index 128 out of bounds for type 'int \\\[128\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */
+
+__attribute__ ((noinline, noclone)) int
+fn5 (int i, int j)
+{
+ asm ("");
+ return b[i & j];
+}
+
+/* { dg-output "\[^\n\r]*index 128 out of bounds for type 'int \\\[128\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */
+
+__attribute__ ((noinline, noclone)) int
+fn6 (int i)
+{
+ asm ("");
+ return b[i & 0];
+}
+
+int
+main (void)
+{
+ fn1 (128);
+ fn2 (128);
+ fn3 (128);
+ fn4 (128);
+ fn5 (128, 127);
+ fn5 (128, 128);
+ fn6 (128);
+ return 0;
+}
diff --git gcc/ubsan.c gcc/ubsan.c
index ed2fc54..41cf546 100644
--- gcc/ubsan.c
+++ gcc/ubsan.c
@@ -1438,6 +1438,7 @@ instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs)
location_t loc = gimple_location (stmt);
tree t = is_lhs ? gimple_get_lhs (stmt) : gimple_assign_rhs1 (stmt);
tree type;
+ tree index = NULL_TREE;
HOST_WIDE_INT size_in_bytes;
type = TREE_TYPE (t);
@@ -1456,6 +1457,8 @@ instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs)
}
break;
case ARRAY_REF:
+ index = TREE_OPERAND (t, 1);
+ break;
case INDIRECT_REF:
case MEM_REF:
case VAR_DECL:
@@ -1537,6 +1540,24 @@ instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs)
&& tree_int_cst_le (t, sizet))
return;
+ if (index != NULL_TREE
+ && TREE_CODE (index) == SSA_NAME
+ && TREE_CODE (sizet) == INTEGER_CST)
+ {
+ gimple def = SSA_NAME_DEF_STMT (index);
+ if (is_gimple_assign (def)
+ && gimple_assign_rhs_code (def) == BIT_AND_EXPR
+ && TREE_CODE (gimple_assign_rhs2 (def)) == INTEGER_CST)
+ {
+ tree cst = gimple_assign_rhs2 (def);
+ tree sz = fold_build2 (EXACT_DIV_EXPR, sizetype, sizet,
+ TYPE_SIZE_UNIT (type));
+ if (tree_int_cst_sgn (cst) >= 0
+ && tree_int_cst_lt (cst, sz))
+ return;
+ }
+ }
+
/* Nope. Emit the check. */
t = force_gimple_operand_gsi (gsi, t, true, NULL_TREE, true,
GSI_SAME_STMT);
Marek
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Optimize BIT_AND_EXPRs for UBSAN_OBJECT_SIZE
2014-11-03 20:47 [PATCH] Optimize BIT_AND_EXPRs for UBSAN_OBJECT_SIZE Marek Polacek
@ 2014-11-03 22:07 ` Jeff Law
2014-11-03 22:29 ` Jakub Jelinek
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Law @ 2014-11-03 22:07 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Jakub Jelinek
On 11/03/14 13:47, Marek Polacek wrote:
> We don't emit UBSAN_BOUNDS checks in case we can at compile-time
> prove that the array access is fine. Also if we have [i & CST],
> where CST is <= bound_of_the_array, we know we're fine as well.
>
> But we don't have similar BIT_AND_EXPR check for UBSAN_OBJECT_SIZE
> which is what this patch attempts to add.
> (This is unrelated to the UBSAN_NULL optimization I posted earlier
> today.)
>
> Bootstrap-ubsan/regtest passed on x86_64-linux, ok for trunk?
>
> 2014-11-03 Marek Polacek <polacek@redhat.com>
>
> * ubsan.c (instrument_object_size): Optimize [x & CST] array accesses.
> testsuite/
> * c-c++-common/ubsan/object-size-10.c: New test.
OK.
jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Optimize BIT_AND_EXPRs for UBSAN_OBJECT_SIZE
2014-11-03 20:47 [PATCH] Optimize BIT_AND_EXPRs for UBSAN_OBJECT_SIZE Marek Polacek
2014-11-03 22:07 ` Jeff Law
@ 2014-11-03 22:29 ` Jakub Jelinek
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2014-11-03 22:29 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Mon, Nov 03, 2014 at 09:47:05PM +0100, Marek Polacek wrote:
> @@ -1456,6 +1457,8 @@ instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs)
> }
> break;
> case ARRAY_REF:
> + index = TREE_OPERAND (t, 1);
> + break;
> case INDIRECT_REF:
> case MEM_REF:
> case VAR_DECL:
In theory, perhaps you could check the offset from get_inner_reference
instead, and look for SSA_NAME set to BIT_AND_EXPR there instead, to handle
cases like
struct S { int a, b; } a[1024];
...
int j = a[i & 1023].b;
etc. - you have size of the base already computed, if the constant part
of offset and bitoffset together is less than that size and the variable
part of it is SSA_NAME set to BIT_AND_EXPR with non-negative const smaller
than size - (constant part of offset + (bitoffset + BITS_PER_UNIT - 1) /
BITS_PER_UNIT + access size), optimize the check away. But perhaps it can
wait for later.
> @@ -1537,6 +1540,24 @@ instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs)
> && tree_int_cst_le (t, sizet))
> return;
>
> + if (index != NULL_TREE
> + && TREE_CODE (index) == SSA_NAME
> + && TREE_CODE (sizet) == INTEGER_CST)
> + {
> + gimple def = SSA_NAME_DEF_STMT (index);
> + if (is_gimple_assign (def)
> + && gimple_assign_rhs_code (def) == BIT_AND_EXPR
> + && TREE_CODE (gimple_assign_rhs2 (def)) == INTEGER_CST)
> + {
> + tree cst = gimple_assign_rhs2 (def);
> + tree sz = fold_build2 (EXACT_DIV_EXPR, sizetype, sizet,
> + TYPE_SIZE_UNIT (type));
> + if (tree_int_cst_sgn (cst) >= 0
> + && tree_int_cst_lt (cst, sz))
> + return;
> + }
> + }
> +
> /* Nope. Emit the check. */
> t = force_gimple_operand_gsi (gsi, t, true, NULL_TREE, true,
> GSI_SAME_STMT);
>
Ok, thanks.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-03 22:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-03 20:47 [PATCH] Optimize BIT_AND_EXPRs for UBSAN_OBJECT_SIZE Marek Polacek
2014-11-03 22:07 ` Jeff Law
2014-11-03 22:29 ` Jakub Jelinek
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).