* [PATCH] Fix ubsan shift instrumentation
@ 2014-10-23 12:34 Marek Polacek
2014-10-23 13:06 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Marek Polacek @ 2014-10-23 12:34 UTC (permalink / raw)
To: Jakub Jelinek, GCC Patches
The issue here was that we were diagnosing an artificial check
that we created within the scope of shift instrumentation. In
other words, for shifts we create something like
(unsigned) A >> (B - C) and signed-integer-overflow triggered
on that subtraction. Fixed by making the subtraction work on
unsigned types. This only happened in C99/C++11 mode.
Middle end seems to cope well with RSHIFT_EXPR whose second op
has an unsigned type.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2014-10-23 Marek Polacek <polacek@redhat.com>
* c-ubsan.c (ubsan_instrument_shift): Perform the MINUS_EXPR
in unsigned type.
* c-c++-common/ubsan/undefined-2.c: New test.
diff --git gcc/c-family/c-ubsan.c gcc/c-family/c-ubsan.c
index 5a42303..7f4dc25 100644
--- gcc/c-family/c-ubsan.c
+++ gcc/c-family/c-ubsan.c
@@ -128,19 +128,19 @@ ubsan_instrument_shift (location_t loc, enum tree_code code,
tree op1_utype = unsigned_type_for (type1);
HOST_WIDE_INT op0_prec = TYPE_PRECISION (type0);
tree uprecm1 = build_int_cst (op1_utype, op0_prec - 1);
- tree precm1 = build_int_cst (type1, op0_prec - 1);
t = fold_convert_loc (loc, op1_utype, op1);
t = fold_build2 (GT_EXPR, boolean_type_node, t, uprecm1);
/* For signed x << y, in C99/C11, the following:
- (unsigned) x >> (precm1 - y)
+ (unsigned) x >> (uprecm1 - y)
if non-zero, is undefined. */
if (code == LSHIFT_EXPR
&& !TYPE_UNSIGNED (type0)
&& flag_isoc99)
{
- tree x = fold_build2 (MINUS_EXPR, integer_type_node, precm1, op1);
+ tree x = fold_build2 (MINUS_EXPR, unsigned_type_node, uprecm1,
+ fold_convert (op1_utype, op1));
tt = fold_convert_loc (loc, unsigned_type_for (type0), op0);
tt = fold_build2 (RSHIFT_EXPR, TREE_TYPE (tt), tt, x);
tt = fold_build2 (NE_EXPR, boolean_type_node, tt,
@@ -148,13 +148,14 @@ ubsan_instrument_shift (location_t loc, enum tree_code code,
}
/* For signed x << y, in C++11 and later, the following:
- x < 0 || ((unsigned) x >> (precm1 - y))
+ x < 0 || ((unsigned) x >> (uprecm1 - y))
if > 1, is undefined. */
if (code == LSHIFT_EXPR
&& !TYPE_UNSIGNED (TREE_TYPE (op0))
&& (cxx_dialect >= cxx11))
{
- tree x = fold_build2 (MINUS_EXPR, integer_type_node, precm1, op1);
+ tree x = fold_build2 (MINUS_EXPR, unsigned_type_node, uprecm1,
+ fold_convert (op1_utype, op1));
tt = fold_convert_loc (loc, unsigned_type_for (type0), op0);
tt = fold_build2 (RSHIFT_EXPR, TREE_TYPE (tt), tt, x);
tt = fold_build2 (GT_EXPR, boolean_type_node, tt,
diff --git gcc/testsuite/c-c++-common/ubsan/undefined-2.c gcc/testsuite/c-c++-common/ubsan/undefined-2.c
index e69de29..7b06709 100644
--- gcc/testsuite/c-c++-common/ubsan/undefined-2.c
+++ gcc/testsuite/c-c++-common/ubsan/undefined-2.c
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=signed-integer-overflow" } */
+/* { dg-additional-options "-std=gnu11" { target c } } */
+/* { dg-additional-options "-std=c++11" { target c++ } } */
+
+volatile int w, z;
+
+__attribute__ ((noinline, noclone)) int
+foo (int x, int y)
+{
+ z++;
+ return x << y;
+}
+
+int
+main ()
+{
+ w = foo (0, -__INT_MAX__);
+ return 0;
+}
+
+/* { dg-output "shift exponent -\[^\n\r]* is negative\[^\n\r]*(\n|\r\n|\r)" } */
Marek
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix ubsan shift instrumentation
2014-10-23 13:06 ` Jakub Jelinek
@ 2014-10-23 12:51 ` Marek Polacek
0 siblings, 0 replies; 3+ messages in thread
From: Marek Polacek @ 2014-10-23 12:51 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: GCC Patches
On Thu, Oct 23, 2014 at 02:46:52PM +0200, Jakub Jelinek wrote:
> Ok. Can you please queue it for 4.9 branch too, after 4.9.2 is released?
> There is no -f*sanitize-recover* support, but it can be supposedly left out
> from dg-options for the branch.
Sure.
Marek
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix ubsan shift instrumentation
2014-10-23 12:34 [PATCH] Fix ubsan shift instrumentation Marek Polacek
@ 2014-10-23 13:06 ` Jakub Jelinek
2014-10-23 12:51 ` Marek Polacek
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2014-10-23 13:06 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Thu, Oct 23, 2014 at 02:34:04PM +0200, Marek Polacek wrote:
> The issue here was that we were diagnosing an artificial check
> that we created within the scope of shift instrumentation. In
> other words, for shifts we create something like
> (unsigned) A >> (B - C) and signed-integer-overflow triggered
> on that subtraction. Fixed by making the subtraction work on
> unsigned types. This only happened in C99/C++11 mode.
> Middle end seems to cope well with RSHIFT_EXPR whose second op
> has an unsigned type.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2014-10-23 Marek Polacek <polacek@redhat.com>
>
> * c-ubsan.c (ubsan_instrument_shift): Perform the MINUS_EXPR
> in unsigned type.
>
> * c-c++-common/ubsan/undefined-2.c: New test.
Ok. Can you please queue it for 4.9 branch too, after 4.9.2 is released?
There is no -f*sanitize-recover* support, but it can be supposedly left out
from dg-options for the branch.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-10-23 13:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-23 12:34 [PATCH] Fix ubsan shift instrumentation Marek Polacek
2014-10-23 13:06 ` Jakub Jelinek
2014-10-23 12:51 ` Marek Polacek
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).