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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
