manas added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1395
+
+ if (Min > Max) {
+ // This implies that an overflow has occured as either boundary would have
----------------
vsavchenko wrote:
> I commented on this part previously, you shouldn't get fixated on `Min > Max`
> because `Max >= Min` doesn't imply that overflow didn't happen (less
> important) and that the range `[Min, Max]` is correct (more important).
> Recall one of the examples that I had in that email.
>
> There is an interesting pattern about results that we can use:
> * no overflows happened -> `[Min, Max]` (should be always true)
> * 1 overflow happened -> we need to invert the range, but there are two
> different cases:
> * `Min > Max`, perfect, that's what we expected -> `[Tmin, Max] ∪ [Min,
> Tmax]`
> * `Max >= Min`, we overflowed and wrapped the whole range -> `[Tmin,
> Tmax]`
> * 2 overflows happened on one side -> `[Min, Max]`
> * 2 overflows happened on both sides -> `[Tmin, Tmax]`
>
> You need to think of this problem in terms of what really happens and not in
> terms of combating with assertions.
I think I agree with you. This pattern (and the code following this pattern)
makes much more sense, and easier to follow. I think I will refactor the patch
accordingly.
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
