This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR80806


On 06/29/2017 12:05 PM, Jeff Law wrote:
On 06/29/2017 11:57 AM, Jeff Law wrote:
On 05/23/2017 09:58 AM, Martin Sebor wrote:
On 05/18/2017 12:55 PM, Prathamesh Kulkarni wrote:
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 ?

Detecting -Wunused-but-set-variable in the optimizer means that
the warning will not be issued without optimization.  It also
means that the warning will trigger in cases where the variable
is used conditionally and the condition is subject to constant
propagation.  For instance:
Yea.  There's definitely tradeoffs for implementing warnings early vs
late.  There's little doubt we could construct testcases where an early
warning would miss cases that could be caught by a late warning.



  void sink (void*);

  void test (int i)
  {
      char buf[10];   // -Wunused-but-set-variable
      memset (buf, 0, sizeof(buf));

      if (i)
        sink (buf);
  }

  void f (void)
  {
      test (0);
  }

I suspect this would be considered a false positive by most users.
In my view, it would be more in line with the design of the warning
to enhance the front end to detect this case, and it would avoid
these issues.
Given no knowledge of sink() here, don't we have to assume that buf is
used?  So, yea, I'd probably consider that a false positive.
Oh, wait, I missed the constant propagation.  That makes this one less
clear cut in my mind -- it means its context sensitive.  I could easily
argue either way on this one.

Suppose buf were the small buffer in std::string and sink() some
conditional use of the class, like in this more complicated code:

  void foo (std::string&);
  void bar (char*);

  void test (int i)
  {
    std::string s;   // calls memset (s.buf, 0, sizeof s.buf)

    char array[100] = "";

    if (i > 100)
      foo (s);       // needs a large buffer
    else
      bar (array);   // works with a small buffer
  }

Variations on this idiom aren't uncommon.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]