* [PATCH] Fix up bogus warning (PR sanitizer/59331)
@ 2013-11-28 23:06 Marek Polacek
2013-11-29 10:32 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Marek Polacek @ 2013-11-28 23:06 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
We wrongly warned on instrumented VLAs that the size expression's
value is not used (with cc1plus only). Unfortunately, this hasn't been
detected before due to disabled warnings in the VLA tests. This patch
adds a (void) cast to suppress the warning as well as enables the
warnings in the VLA tests to detect unwanted warnings next time.
Tested x86_64-linux, ok for trunk?
2013-11-28 Marek Polacek <polacek@redhat.com>
PR sanitizer/59331
cp/
* decl.c (compute_array_index_type): Cast the expression to void.
testsuite/
* g++.dg/ubsan/pr59331.C: New test.
* g++.dg/ubsan/cxx1y-vla.C: Enable -Wall -Wno-unused-variable.
Disable the -w option.
* c-c++-common/ubsan/vla-1.c: Likewise.
* c-c++-common/ubsan/vla-2.c: Likewise.
* c-c++-common/ubsan/vla-3.c: Don't use the -w option.
--- gcc/cp/decl.c.mp5 2013-11-28 16:15:42.606690956 +0100
+++ gcc/cp/decl.c 2013-11-28 17:49:44.120202587 +0100
@@ -8435,7 +8435,9 @@ compute_array_index_type (tree name, tre
tree t = fold_build2 (PLUS_EXPR, TREE_TYPE (itype), itype,
build_one_cst (TREE_TYPE (itype)));
t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t),
- ubsan_instrument_vla (input_location, t), t);
+ ubsan_instrument_vla (input_location, t),
+ /* Cast to void to prevent bogus warning. */
+ build1 (CONVERT_EXPR, void_type_node, t));
finish_expr_stmt (t);
}
}
--- gcc/testsuite/g++.dg/ubsan/pr59331.C.mp5 2013-11-28 16:29:13.967882392 +0100
+++ gcc/testsuite/g++.dg/ubsan/pr59331.C 2013-11-28 17:54:24.125451857 +0100
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
+
+void foo(int i)
+{
+ /* Don't warn here with "value computed is not used". */
+ char a[i];
+}
--- gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C.mp5 2013-11-28 17:51:51.066755487 +0100
+++ gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C 2013-11-28 17:59:49.578744756 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w -std=c++1y" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable -std=c++1y" } */
/* { dg-shouldfail "ubsan" } */
int
--- gcc/testsuite/c-c++-common/ubsan/vla-1.c.mp5 2013-11-28 18:03:32.318664603 +0100
+++ gcc/testsuite/c-c++-common/ubsan/vla-1.c 2013-11-28 18:03:45.627715609 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
static int
bar (void)
--- gcc/testsuite/c-c++-common/ubsan/vla-3.c.mp5 2013-11-28 18:04:25.737865780 +0100
+++ gcc/testsuite/c-c++-common/ubsan/vla-3.c 2013-11-28 18:04:34.796900021 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w" } */
+/* { dg-options "-fsanitize=vla-bound" } */
/* Don't instrument the arrays here. */
int
--- gcc/testsuite/c-c++-common/ubsan/vla-2.c.mp5 2013-11-28 18:03:54.249748290 +0100
+++ gcc/testsuite/c-c++-common/ubsan/vla-2.c 2013-11-28 18:04:07.666798731 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
int
main (void)
Marek
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Fix up bogus warning (PR sanitizer/59331)
2013-11-28 23:06 [PATCH] Fix up bogus warning (PR sanitizer/59331) Marek Polacek
@ 2013-11-29 10:32 ` Jason Merrill
2013-11-29 13:16 ` Marek Polacek
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2013-11-29 10:32 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 11/28/2013 12:14 PM, Marek Polacek wrote:
> t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t),
> - ubsan_instrument_vla (input_location, t), t);
> + ubsan_instrument_vla (input_location, t),
> + /* Cast to void to prevent bogus warning. */
> + build1 (CONVERT_EXPR, void_type_node, t));
> finish_expr_stmt (t);
Why do you need the COMPOUND_EXPR at all? Why can't you just do
t = ubsan_instrument_vla (input_location, t);
?
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix up bogus warning (PR sanitizer/59331)
2013-11-29 10:32 ` Jason Merrill
@ 2013-11-29 13:16 ` Marek Polacek
2013-11-30 16:56 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Marek Polacek @ 2013-11-29 13:16 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Thu, Nov 28, 2013 at 07:04:43PM -0500, Jason Merrill wrote:
> On 11/28/2013 12:14 PM, Marek Polacek wrote:
> > t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t),
> >- ubsan_instrument_vla (input_location, t), t);
> >+ ubsan_instrument_vla (input_location, t),
> >+ /* Cast to void to prevent bogus warning. */
> >+ build1 (CONVERT_EXPR, void_type_node, t));
> > finish_expr_stmt (t);
>
> Why do you need the COMPOUND_EXPR at all? Why can't you just do
>
> t = ubsan_instrument_vla (input_location, t);
>
> ?
You're right, I don't need it at all here. It was needed in the C FE
and I lived under wrong impression that I'll need it in the C++ as
well. And of course the warning goes away... Thanks a lot.
Tested x86_64-unknown-linux-gnu. Ok now?
2013-11-29 Marek Polacek <polacek@redhat.com>
PR sanitizer/59331
cp/
* decl.c (compute_array_index_type): Don't build COMPOUND_EXPR for
instrumentation.
testsuite/
* g++.dg/ubsan/pr59331.C: New test.
* g++.dg/ubsan/cxx1y-vla.C: Enable -Wall -Wno-unused-variable.
Disable the -w option.
* c-c++-common/ubsan/vla-1.c: Likewise.
* c-c++-common/ubsan/vla-2.c: Likewise.
* c-c++-common/ubsan/vla-3.c: Don't use the -w option.
--- gcc/cp/decl.c.mp5 2013-11-28 16:15:42.606690956 +0100
+++ gcc/cp/decl.c 2013-11-29 12:31:17.032996706 +0100
@@ -8434,8 +8434,7 @@ compute_array_index_type (tree name, tre
LE_EXPR rather than LT_EXPR. */
tree t = fold_build2 (PLUS_EXPR, TREE_TYPE (itype), itype,
build_one_cst (TREE_TYPE (itype)));
- t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t),
- ubsan_instrument_vla (input_location, t), t);
+ t = ubsan_instrument_vla (input_location, t);
finish_expr_stmt (t);
}
}
--- gcc/testsuite/g++.dg/ubsan/pr59331.C.mp5 2013-11-28 16:29:13.967882392 +0100
+++ gcc/testsuite/g++.dg/ubsan/pr59331.C 2013-11-28 17:54:24.125451857 +0100
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
+
+void foo(int i)
+{
+ /* Don't warn here with "value computed is not used". */
+ char a[i];
+}
--- gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C.mp5 2013-11-28 17:51:51.066755487 +0100
+++ gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C 2013-11-28 18:28:33.411162834 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w -std=c++1y" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable -std=c++1y" } */
/* { dg-shouldfail "ubsan" } */
int
--- gcc/testsuite/c-c++-common/ubsan/vla-1.c.mp5 2013-11-28 18:03:32.318664603 +0100
+++ gcc/testsuite/c-c++-common/ubsan/vla-1.c 2013-11-28 18:28:33.410162830 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
static int
bar (void)
--- gcc/testsuite/c-c++-common/ubsan/vla-3.c.mp5 2013-11-28 18:04:25.737865780 +0100
+++ gcc/testsuite/c-c++-common/ubsan/vla-3.c 2013-11-28 18:28:33.411162834 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w" } */
+/* { dg-options "-fsanitize=vla-bound" } */
/* Don't instrument the arrays here. */
int
--- gcc/testsuite/c-c++-common/ubsan/vla-2.c.mp5 2013-11-28 18:03:54.249748290 +0100
+++ gcc/testsuite/c-c++-common/ubsan/vla-2.c 2013-11-28 18:28:33.410162830 +0100
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fsanitize=vla-bound -w" } */
+/* { dg-options "-fsanitize=vla-bound -Wall -Wno-unused-variable" } */
int
main (void)
Marek
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-29 20:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-28 23:06 [PATCH] Fix up bogus warning (PR sanitizer/59331) Marek Polacek
2013-11-29 10:32 ` Jason Merrill
2013-11-29 13:16 ` Marek Polacek
2013-11-30 16:56 ` Jason Merrill
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).