diff mbox

Fix up bogus warning (PR sanitizer/59331)

Message ID 20131128171400.GI31608@redhat.com
State New
Headers show

Commit Message

Marek Polacek Nov. 28, 2013, 5:14 p.m. UTC 
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.


	Marek

Comments

Jason Merrill Nov. 29, 2013, 12:04 a.m. UTC | #1 
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
diff mbox

Patch

--- 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)