[PATCH] D128535: [analyzer] Improve loads from reinterpret-cast fields

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, martong, ASDenysPetrov.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Consider this example:

  struct header {
    unsigned a : 1;
    unsigned b : 1;
  };
  struct parse_t {
    unsigned bits0 : 1;
    unsigned bits2 : 2; // <-- header
    unsigned bits4 : 4;
  };
  int parse(parse_t *p) {
    unsigned copy = p->bits2;
    clang_analyzer_dump(copy);
    // expected-warning at -1 {{reg_$1<unsigned int SymRegion{reg_$0<struct Bug_55934::parse_t * p>}.bits2>}}
  
    header *bits = (header *)©
    clang_analyzer_dump(bits->b); // <--- Was UndefinedVal previously.
    // expected-warning at -1 {{derived_$2{reg_$1<unsigned int SymRegion{reg_$0<struct Bug_55934::parse_t * p>}.bits2>,Element{copy,0 S64b,struct Bug_55934::header}.b}}}
    return bits->b; // no-warning: it's not UndefinedVal
  }

`bits->b` should have the same content as the second bit of `p->bits2`
(assuming that the bitfields are in spelling order).

---

The `Store` has the correct bindings. The problem is with the load of `bits->b`.
It will eventually reach `RegionStoreManager::getBindingForField()` with
`Element{copy,0 S64b,struct header}.b`, which is a `FieldRegion`.
It did not find any direct bindings, so the `getBindingForFieldOrElementCommon()`
gets called. That won't find any bindings, but it sees that the variable
is on the //stack//, thus it must be an uninitialized local variable;
thus it returns `UndefinedVal`.

Instead of doing this, it should have created a //derived symbol//
representing the slice of the region corresponding to the member.
So, if the value of `copy` is `reg1`, then the value of `bits->b` should
be `derived{reg1, elem{copy,0, header}.b}`.

Actually, the `getBindingForElement()` already does exactly this for reinterpret-casts, so I decided to hoist that and reuse the logic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128535

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/ptr-arith.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128535.439758.patch
Type: text/x-patch
Size: 4839 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220624/549c83a9/attachment.bin>