balazske added inline comments.

================
Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1123-1124
+      "abs", Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
+                 .Case({ArgumentCondition(0, WithinRange, SingleValue(0)),
+                        ReturnValueCondition(WithinRange, SingleValue(0))})
+                 .Case({ArgumentCondition(0, WithinRange, Range(1, IntMax)),
----------------
martong wrote:
> NoQ wrote:
> > NoQ wrote:
> > > The three-way state split is unjustified here. Usage of `abs` is not a 
> > > sufficient indication that the value may be 0, otherwise:
> > > ```lang=c++
> > > int foo(int x, int y) {
> > >   int z = abs(y);   // Assuming 'abs' has taken branch on which y == 0...
> > >   return x / z;     // ... we'll be forced to emit a division by zero 
> > > warning here.
> > > }
> > > ```
> > > 
> > > Generally, there are very few cases when state splits upon function calls 
> > > are justified. The common cases are:
> > > - The function returns bool and finding that bool is the only reason to 
> > > ever call this function. Eg., `isalpha()` and such.
> > > - The function can at any time completely unpredictably take any of the 
> > > branches, in other words, taint is involved. Eg., `scanf()` can always 
> > > fail simply because the user of the program wrote something special into 
> > > stdin.
> > > returns bool
> > 
> > Or something that kinda resembles bool (eg., `isalpha()` returns a variety 
> > of different ints in practice due to its static lookup table implementation 
> > strategy but the user only cares about whether it's zero or non-zero).
> Alright, I agree, I'll remove these cases. 
> 
> Generally speaking I realized that it is hard to create these cases, and it 
> is very rare when we can come up with a meaningful case set for any function. 
> So while we are at it, could you please take a look at another patch, where I 
> add no cases just argument constraints: D82288 ?
But it could make sense that if the input of the function is known before the 
call, we can compute the output? For `abs(x)` if we know that `x == 0` is true 
the checker could assume `abs(x) == 0` too. These "cases" provide data for this 
functionality.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79432/new/

https://reviews.llvm.org/D79432

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D79432: [analyzer] S... Balázs Kéri via Phabricator via cfe-commits

Reply via email to