manas added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1409-1415
+ if ((LHS.From() > 0 && RHS.From() > 0 && Min < 0) ||
+ (LHS.From() < 0 && RHS.From() < 0 && Min > 0) ||
+ (LHS.To() > 0 && RHS.To() > 0 && Max < 0) ||
+ (LHS.To() < 0 && RHS.To() < 0 && Max > 0)) {
+ // return [Tmin, Tmax]
+ return {RangeFactory, Tmin, Tmax};
+ }
----------------
vsavchenko wrote:
> manas wrote:
> > vsavchenko wrote:
> > > manas wrote:
> > > > vsavchenko wrote:
> > > > > I thought we talked quite a lot that there is nothing bad with
> > > > > overflows and here we have that if ANY overflow happened, we bail out
> > > > > and don't give any result.
> > > > Understood! Should I replace it with code returning EmptySet()?
> > > Why `EmptySet()`? `EmptySet()` means that this can never happen, this
> > > path is infeasible. Is that the case?
> > > Let's say we have: `[INT_MAX - 20, INT_MAX - 10] + [30, 40]` what happens
> > > in this case?
> > Right! That will not be the case.
> >
> > In this particular case, the range will be `[INT_MIN + 9, INT_MIN + 29]`
> > which is far smaller than `[Tmin, Tmax]`.
> >
> > Also, I think I misunderstood the part of //bailing out and not giving any
> > result// as returning empty.
> Gotcha!
>
> Because of cases like this, you need to re-think that part about `Min > Max`
> and maybe count the number of overflows on each side?
That's right. If it is able to deduce how many times overflows are occurring
then it can reason about whether it will be MaxRangeSet or a subset of it.
Fixing it!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103440/new/
https://reviews.llvm.org/D103440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
