[LLVMbugs] [Bug 8464] New: False positives due to a coding style with invariants

[bugzilla-daemon at llvm.org]

http://llvm.org/bugs/show_bug.cgi?id=8464

           Summary: False positives due to a coding style with invariants
           Product: clang
           Version: trunk
          Platform: PC
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: Static Analyzer
        AssignedTo: kremenek at apple.com
        ReportedBy: alexei.svitkine at gmail.com
                CC: llvmbugs at cs.uiuc.edu


Consider a function that writes to a buffer:

void foo() {
  int buf[32];
  int i = 0;

  if (a) { buf[i] = 1; i++; }
  if (b) { buf[i] = 42; i++; }
  if (c) { buf[i] = 42; i++; buf[i] = 100; i++; }
  if (d) { buf[i] = 5; i++; }
}

If this is given to the static analyzer presently, it will warn of a dead store
for the last i++ increment.

However, in the code above the warning is noise as far as the user is
concerned.

Yes, it is true that the last increment of i does not change the program
behaviour, and thus can be removed. But removing this statement would break the
invariant the author of the code has been following.

The invariant, in this case is 'i is the index to the next element to write to
in the buffer'.

Such an invariant is useful to have - for example if you add a more code to the
function, you can do so under the assumption that the invariant is true,
without hunting down whoever was previously the last person to write to the
buffer (especially if the function has complicated other logic all around the
buffer writing - unlike this trivial example).

There's other variants on this pattern - for example copying strings into a
char buffer and incrementing by the length of the strings, or reading input. In
all cases, warning on dead increments / stores in such cases is simply not
helpful and just creates extra noise (essentially, false positives as far as
the author of the code is concerned). In some cases, this may even be getting
done by a macro, so there's no way to remove the i++ increment if you're using
the macro.

It should be possible for clang to detect such invariants automatically, since
they are localized to a function, and avoid issuing analysis warnings in such a
case.

Detection can be pretty simple - before issuing the warning, do another pass
over the code and try to detect if the same pattern occurs previously - to
figure out if the variable in question is being modified to enforce some
invariant in the code.

-- 
Configure bugmail: http://llvm.org/bugs/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.