[PATCH] D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc
Gabor Marton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 23 07:58:36 PDT 2020
martong marked 2 inline comments as done.
martong 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)),
----------------
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 ?
================
Comment at: clang/test/Analysis/std-c-library-functions.c:231
+ if (p > 0)
+ clang_analyzer_eval(abs(p) < 0); // expected-warning{{TRUE}}
+ if (p < 0)
----------------
NoQ wrote:
> Emm :)
Ups, ouch :D
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79432/new/
https://reviews.llvm.org/D79432
More information about the cfe-commits
mailing list