[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

[Ding Fei via Phabricator via cfe-commits]

danix800 added a comment.

In D158707#4613743 <https://reviews.llvm.org/D158707#4613743>, @donat.nagy wrote:

> Thanks for creating this commit, it's a useful improvement!
>
> I added some inline comments on minor implementation details; moreover, note that "Releted checkers:" (instead of "related") is a typo in the commit message.
>
> I also have a less concrete question about the root cause of these bugs. Does this commit fix the "root" of the issue by eliminating some misuse of correctly implemented (but perhaps surprising) `SVal` / `APSInt` arithmetic; or is there an underlying bug in the `SVal` / `APSInt` arithmetic which is just avoided (and not eliminated) by this commit? In the latter case, what obstacles prevent us from fixing the root cause?

For the observed signed compared to unsigned unexpectation from ArrayBoundV2,
the constraint manager does give the CORRECT result.

It's a misuse of the constraint API, mainly caused by unexpected unsigned extent
set by memory modeling. Actually `ArrayBoundV2::getSimplifiedOffsets()` has clear
comment that this `signed <-> unsigned` comparison is problematic.

  // TODO: once the constraint manager is smart enough to handle non simplified
  // symbolic expressions remove this function. Note that this can not be used in
  // the constraint manager as is, since this does not handle overflows. It is
  // safe to assume, however, that memory offsets will not overflow.
  // NOTE: callers of this function need to be aware of the effects of overflows
  // and signed<->unsigned conversions!
  static std::pair<NonLoc, nonloc::ConcreteInt>
  getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt extent,
                       SValBuilder &svalBuilder) {


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158707/new/

https://reviews.llvm.org/D158707