vsavchenko added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1397
+ (LHS.To() < 0 && RHS.To() < 0 && Max > 0) ||
+ (LHS.To() < 0 && RHS.To() < 0 && Max > 0)) {
+ return {RangeFactory, Tmin, Tmax};
----------------
This clause is exactly the same as the previous one, it is a mistake.
And I think we should have a test that could've shown that.
Also, since you are checking for overflows for both the beginning and the end,
we should have tests where both overflow.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1401-1402
+
+ // FIXME: This case in particular is resulting in failed assertion.
+ Range First = Range(Tmin, Max);
+ Range Second = Range(Min, Tmax);
----------------
manas wrote:
> I changed the logic from using getCrucialPoints (which I mentioned in the
> e-mail thread) to simple checks to determine overflows using signedness. But
> the failed assertions again popped up. And I was unable to pin-point the
> issue here. Can someone help me?
>
> Although, I am thinking of revising the above logic by using bitwise methods
> to detect overflows.
This is actually one place that I won't expect an assertion failure.
Can we get a bit more detail on it? Is it again `From > To` (which will indeed
be weird) or something different?
NOTE: Asking for help (either here or on StackOverflow) is a lot like filing a
bug report, try to put as much information as possible, try to structure this
information so it is easy to follow. It's also good to tell people what you
tried, instead of saying that you tried something and it didn't work.
================
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};
+ }
----------------
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.
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
