Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Scope context issues with variables. #31136

Open Quuxplusone opened 7 years ago

Quuxplusone commented 7 years ago
Bugzilla Link PR32163
Status NEW
Importance P enhancement
Reported by Artem Dergachev (noqnoqneo@gmail.com)
Reported on 2017-03-07 01:56:46 -0800
Last modified on 2017-03-07 01:59:22 -0800
Version unspecified
Hardware PC All
CC llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
This report is made in order to document a couple of issues which should
ideally be fixed by https://reviews.llvm.org/D19979 or a similar approach.

Test case 1:

  int *arr[2];

  void foo() {
    for (int i = 0; i < 2; ++i) {
      int x;
      arr[i] = &x;
    }
    clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{UNKNOWN}}
  }

Test case 2:

  int *arr[2];

  void bar(int i) {
    int x;
    arr[i] = &x;
  }

  void foo() {
    for (int i = 0; i < 2; ++i)
      bar(i);
    clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{UNKNOWN}}
  }

We currently yield TRUE in both cases, which is wrong because different
instances of local variable `x' may have different addresses on the stack.

The analyzer discriminates between different instances of the same AST variable
by assigning them to different memory space superregions (or block data
regions, which are not technically memory spaces) - in this case, different
instances of StackLocalsSpaceRegion.

In test 2, these instances should be different because they are constructed
with different StackFrameContext objects. However, because StackFrameContext
doesn't include a block count on the call site, but only the call expression
itself, they accidentally coincide. We could fix this by adding block count to
StackFrameContext.

In test 1, it's harder because the stack frame is the same. We'd need a full-
featured ScopeContext to fix this properly, so that the analyzer realized that
we're in different scopes on every loop iteration; i don't see an easier fix.
That'd fix test 2 automagically - the StackFrameContext instances would be
different because their parent ScopeContext instances would be different.
Quuxplusone commented 7 years ago

Even though test cases are artificial, this bug does manifest quite noticeably on some checkers. See, for instance, https://reviews.llvm.org/D24246#inline-257995