public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR80806
@ 2017-05-18 18:56 Prathamesh Kulkarni
  2017-05-22  4:50 ` PR80806 Jeff Law
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Prathamesh Kulkarni @ 2017-05-18 18:56 UTC (permalink / raw)
  To: gcc Patches, Richard Biener

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

Hi,
The attached patch tries to fix PR80806 by warning when a variable is
set using memset (and friends) but not used. I chose to warn in dse
pass since dse would detect if the variable passed as 1st argument is
a dead store. Does this approach look OK ?

There were following fallouts observed during bootstrap build:

* double-int.c (div_and_round_double):
Warning emitted 'den' set but not used for following call to memset:
memset (den, 0, sizeof den);

I assume the warning is correct since there's immediately call to:
encode (den, lden, hden);

and encode overwrites all the contents of den.
Should the above call to memset be removed from the source ?

* tree-streamer.c (streamer_check_handled_ts_structures)
The function defines a local array bool handled_p[LAST_TS_ENUM];
and the warning is emitted for:
memset (&handled_p, 0, sizeof (handled_p));

That's because the function then initializes each element of the array
handled_p to true
making the memset call redundant.
I am not sure if warning for the above case is a good idea ? The call
to memset() seems deliberate, to initialize all elements to 0, and
later assert checks if all the elements were explicitly set to true.

* value-prof.c (free_hist):
Warns for the call to memset:

static int
free_hist (void **slot, void *data ATTRIBUTE_UNUSED)
{
  histogram_value hist = *(histogram_value *) slot;
  free (hist->hvalue.counters);
  if (flag_checking)
    memset (hist, 0xab, sizeof (*hist));
  free (hist);
  return 1;
}

Assuming flag_checking was true, the call to memset would be dead
anyway since it would be immediately freed ? Um, I don't understand
the purpose of memset in the above function.

* testsuite fallout
I verified regressing test-cases were not false positives and added
-Wno-unused-but-set-variable.

Thanks,
Prathamesh

[-- Attachment #2: pr80806-5.diff --]
[-- Type: text/plain, Size: 4727 bytes --]

diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c
index 895a50e2677..6cbcc419976 100644
--- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c
+++ b/gcc/testsuite/c-c++-common/Wsizeof-pointer-memaccess2.c
@@ -1,7 +1,7 @@
 /* Test -Wsizeof-pointer-memaccess warnings.  */
 /* { dg-do compile } */
-/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -ftrack-macro-expansion=0" } */
-/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -Wno-c++-compat -ftrack-macro-expansion=0" {target c} } */
+/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -Wno-unused-but-set-variable -ftrack-macro-expansion=0" } */
+/* { dg-options "-Wall -O2 -Wno-sizeof-array-argument -Wno-c++-compat -Wno-unused-but-set-variable -ftrack-macro-expansion=0" {target c} } */
 /* { dg-require-effective-target alloca } */
 
 #define bos(ptr) __builtin_object_size (ptr, 1)
diff --git a/gcc/testsuite/gcc.dg/Wstrict-aliasing-bogus-vla-1.c b/gcc/testsuite/gcc.dg/Wstrict-aliasing-bogus-vla-1.c
index 87f5ef9d171..c42e3270db9 100644
--- a/gcc/testsuite/gcc.dg/Wstrict-aliasing-bogus-vla-1.c
+++ b/gcc/testsuite/gcc.dg/Wstrict-aliasing-bogus-vla-1.c
@@ -1,6 +1,6 @@
 /* PR 41673: bogus -Wstrict-aliasing warning from VLA dereference.  */
 /* { dg-do compile } */
-/* { dg-options "-std=gnu99 -O2 -Wall" } */
+/* { dg-options "-std=gnu99 -O2 -Wall -Wno-unused-but-set-variable" } */
 /* { dg-require-effective-target alloca } */
 
 int main(int argc, char *argv[])
diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size.c b/gcc/testsuite/gcc.dg/attr-alloc_size.c
index f50ba7c53db..8d71b3a4e6d 100644
--- a/gcc/testsuite/gcc.dg/attr-alloc_size.c
+++ b/gcc/testsuite/gcc.dg/attr-alloc_size.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+/* { dg-options "-O2 -Wall -Wno-unused-but-set-variable -ftrack-macro-expansion=0" } */
 
 extern void abort (void);
 
diff --git a/gcc/testsuite/gcc.dg/pr79715.c b/gcc/testsuite/gcc.dg/pr79715.c
index 0f0f90f7122..cd59f6dbf14 100644
--- a/gcc/testsuite/gcc.dg/pr79715.c
+++ b/gcc/testsuite/gcc.dg/pr79715.c
@@ -1,7 +1,7 @@
 /* PR tree-optimization/79715 - hand-rolled strdup with unused result
    not eliminated
    { dg-do compile }
-   { dg-options "-O2 -Wall -fdump-tree-optimized" } */
+   { dg-options "-O2 -Wall -Wno-unused-but-set-variable -fdump-tree-optimized" } */
 
 void f (const char *s)
 {
diff --git a/gcc/testsuite/gcc.dg/pr80806.c b/gcc/testsuite/gcc.dg/pr80806.c
new file mode 100644
index 00000000000..551555c8fd1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr80806.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+void f1(void)
+{
+  char *p = __builtin_malloc (10);
+  __builtin_memset (p, 0, 10); /* { dg-warning "'p' set but not used" } */
+}
+
+void f2(void)
+{
+  char buf[10];
+  __builtin_memset (buf, 0, 10); /* { dg-warning "'buf' set but not used" } */
+}
diff --git a/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c b/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c
index a73e45fb809..9ca9b287d25 100644
--- a/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c
+++ b/gcc/testsuite/gcc.dg/torture/Wsizeof-pointer-memaccess1.c
@@ -1,6 +1,6 @@
 /* Test -Wsizeof-pointer-memaccess warnings.  */
 /* { dg-do compile } */
-/* { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow" } */
+/* { dg-options "-Wall -Wno-sizeof-array-argument -Wno-stringop-overflow -Wno-unused-but-set-variable" } */
 /* Test just twice, once with -O0 non-fortified, once with -O2 fortified.  */
 /* { dg-skip-if "" { *-*-* }  { "*" } { "-O0" "-O2" } } */
 /* { dg-skip-if "" { *-*-* }  { "-flto" } { "" } } */
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 90230abe822..f6d583f8034 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-cfgcleanup.h"
 #include "params.h"
 #include "alias.h"
+#include "diagnostic.h"
 
 /* This file implements dead store elimination.
 
@@ -742,7 +743,22 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi)
 		}
 
 	      if (store_status == DSE_STORE_DEAD)
-		delete_dead_call (gsi);
+		{
+		  tree ptr = gimple_call_arg (stmt, 0);
+		  if (TREE_CODE (ptr) == SSA_NAME
+		      || TREE_CODE (ptr) == ADDR_EXPR)
+		    {
+		      tree base = (TREE_CODE (ptr) == SSA_NAME)
+				  ? SSA_NAME_VAR (ptr)
+				  : TREE_OPERAND (ptr, 0);
+
+		      if (base && VAR_P (base))
+			warning_at (gimple_location (stmt),
+				    OPT_Wunused_but_set_variable,
+				    "%qD set but not used", base);
+		    }
+		  delete_dead_call (gsi);
+		}
 	      return;
 	    }
 

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-06-30  8:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-18 18:56 PR80806 Prathamesh Kulkarni
2017-05-22  4:50 ` PR80806 Jeff Law
2017-05-23 13:50   ` PR80806 Prathamesh Kulkarni
2017-05-23 15:59 ` PR80806 Martin Sebor
2017-05-24  5:42   ` PR80806 Martin Sebor
2017-06-29 17:57   ` PR80806 Jeff Law
2017-06-29 18:05     ` PR80806 Jeff Law
2017-06-29 21:45       ` PR80806 Martin Sebor
2017-06-29 18:20 ` PR80806 Jeff Law
2017-06-30  8:25   ` PR80806 Richard Biener
2017-06-30  8:33     ` PR80806 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).