NoQ 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)),
----------------
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.


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

Reply via email to