[llvm-bugs] [Bug 24506] New: Add dead store related warnings

[via llvm-bugs]

https://llvm.org/bugs/show_bug.cgi?id=24506

            Bug ID: 24506
           Summary: Add dead store related warnings
           Product: clang
           Version: unspecified
          Hardware: PC
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: C++
          Assignee: unassignedclangbugs at nondot.org
          Reporter: dblaikie at gmail.com
                CC: chandlerc at gmail.com, dgregor at apple.com,
                    llvm-bugs at lists.llvm.org, nlewycky at google.com,
                    rtrieu at google.com
    Classification: Unclassified

A while ago this bug came up:

void func(bool *b) {
  b = false;
}

Where the user forgot to dereference 'b' before assigning. This is now warned
about (-Wbool-conversion) in < C++11, and a hard error in >= C++11 (C++11 and
up, only the literal 0, not all integer zero literals, is a valid null pointer
- so '\0' is also caught)

But this bug could still be written with integers:

void func1(int *i) {
  i = 0;
}

One way to catch this would be to implement a warning for dead stores. Any
situation in which a variable is written to, but never read. (the zero was
written, and then 'i' went out of scope - thus never read)

This wouldn't catch another case that came up, though:

void func2(bool b) {
  b = false;
  f1(b);
  f2(b);
  ...
}

The author intended the boolean to be a reference and to communicate back to
the caller some state about the execution of f1, f2, etc...

A dead store warning, extended somewhat, could catch this - if we treat the
input parameter 'b' as a store itself, then it is a dead store if we
unconditionally write to 'b' before we ever read from it. (as is the case
func2)

It may be the case that a general dead store warning (even with the further
generalization for parameter 'stores') would be too noisy for existing
codebases, consider common idioms like:

f(x); ++x
f(x); ++x

etc. People copy/paste these, and they may intentionally leave the ++x on the
last line for consistency.

So to get a good warning we might need to blacklist certain store operations to
be acceptably dead - or whitelist those (like parameter stores) that would
never be acceptably dead.

GCC has a simpler form of such a warning which wouldn't require control flow
analysis, -Wunused-but-set-parameter. Just looks for the parameter to be
assigned to in the function, but never to be read from at all. This would catch
the basic cases shown at the start, but not func2 or anything more involved.
May be worth implementing if someone wants some coverage but doesn't want to
pay the compile time penalty for CFG-based warnings.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20150819/efb5ef48/attachment.html>