[PATCH] D31982: [analyzer] Improve suppression for inlined defensive checks when operator& is involved.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

In code

  void foo(int *p) {
    if (p) {}
  }
  void bar(int *p) {
    foo(p);
    *p = 5;
  }

we suppress the null dereference warning in `*p = 5` because the state split within the inlined function is essentially irrelevant, as this is merely a defensive check that does not necessarily trigger in this caller context.

The suppression works by tracking the null pointer symbol in a bug report visitor and seeing if it was constrained to 0 inside a callee context. The fact that we have to rely on such manual suppressions is a downside of the inlining-based approach to interprocedural analysis.

The suppression gets confused when the null dereference becomes more complex, eg:

  struct S {
    int x;
  }
  void foo(struct S *s) {
    if (s) {}
  }
  void bar(struct S *s) {
    foo(s);
    *(&(s->x)) = 5;
  }

In this case `s` is `&SymRegion{reg_$0<s>}`, and `reg_$0<s>` is constrained to `[0, 0]`, similarly to the above. However, while computing `s->x` and then `&(s->x)`, the symbol is collapsed to a constant `0 (Loc)`, making it harder to track the symbol. The visitor is improved to handle this case.

While the fact that //offset of field `x` in struct `S` in our example is equal to 0// is purely accidental, with any other offset it'd still be a null dereference, worth suppressing. How does the analyzer handle the non-zero offset case and still see a null dereference? Simply and powerfully: it mis-computes the offset, incorrectly believing that all offsets are equal to 0 (woohoo!). I add a test case to document this behavior; even though it's trivial to fix, the positive side effects of this bug are for now too useful to discard.


https://reviews.llvm.org/D31982

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/explain-svals.cpp
  test/Analysis/inlining/inline-defensive-checks.c
  test/Analysis/inlining/inline-defensive-checks.cpp
  test/Analysis/uninit-const.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31982.94986.patch
Type: text/x-patch
Size: 5803 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170412/4ffe84aa/attachment.bin>