vsavchenko added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1400
+  if (ResultType.isUnsigned()) {
+    LHS.From().uadd_ov(RHS.From(), HasMinOverflowed);
+    LHS.To().uadd_ov(RHS.To(), HasMaxOverflowed);
----------------
manas wrote:
> vsavchenko wrote:
> > manas wrote:
> > > vsavchenko wrote:
> > > > manas wrote:
> > > > > Using `uadd_ov` (and `sadd_ov`), we can get the added value as well 
> > > > > as whether overflow occurred or not. A point is that these functions 
> > > > > return `APInt` instead of `APSInt`.
> > > > > 
> > > > > But when I tried just using:
> > > > >   Min = LHS.From().uadd_ov(RHS.From(), HasMinOverflowed);
> > > > >   Max = LHS.To().uadd_ov(RHS.From(), HasMaxOverflowed);
> > > > > 
> > > > > instead of
> > > > >   Min = LHS.From() + RHS.From();
> > > > >   Max = LHS.To() + RHS.To();
> > > > > 
> > > > > just for the added value, then the following tests failed (//these 
> > > > > tests and all other tests pass when I use the latter method to get 
> > > > > Min/Max//):                                                           
> > > > >  
> > > > >   Clang :: Analysis/PR3991.m
> > > > >   Clang :: Analysis/global-region-invalidation.c
> > > > >   Clang :: Analysis/malloc-overflow2.c
> > > > >   Clang :: Analysis/out-of-bounds-new.cpp
> > > > >   Clang :: Analysis/taint-generic.c
> > > > > 
> > > > > I am working on fixing this part.
> > > > You can easily construct `APSInt` from `APInt` using `APSInt 
> > > > ::APSInt(APInt I, bool isUnsigned)` constructor.
> > > Okay. I will try with using `uadd_ov` only then. And check whether those 
> > > tests pass or not.
> > Hmm, why only `uadd_ov`?  What about those tests?  How do they fail?  Try 
> > to look at the reasons and not brute-force by trying different solutions 
> > blindly.
> > Those tests are your friends, it's much better to get failures right now 
> > then getting them later when you land the patch.
> I meant related functions as well.
> 
> Most of them were failing due to `Error evaluating statements`.  Although, I 
> have rebased my tree so I will re-check again.
`Error evaluating statements` just tells you when we crashed (for the context 
and fast insight).
Below this message is usually the real reason (and the last time I checked, it 
was assertion `From > To` failure).


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

Reply via email to