[PATCH] D92771: [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

[Balázs Benics via Phabricator via cfe-commits]

steakhal accepted this revision.
steakhal added a comment.
This revision is now accepted and ready to land.

It must have been a tedious task to collect all these - without any copy-paste errors, really impressive!

It's good to go, however, if you don't mind there would be some readability issues yet to solve in a later path in the inline comments.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1367-1368
+                                .Case(ReturnsZeroOrMinusOne)
                                 .ArgConstraint(ArgumentCondition(
                                     0, WithinRange, Range(0, IntMax))));
 
----------------
I think you should hoist this ArgumentCondition construction with a lambda call. It would lead to more readable summaries.

```lang=c++
const auto ValidFileDescriptorArgAt = [](unsigned ArgIdx) {
  return ArgumentCondition(ArgIdx, WithinRange, Range(0, IntMax))));
};
```
Probably the very same principle would apply for handling `off_t` arguments.

You can probably find a better name, but you get the idea.
If you agree on this, you could create a follow-up patch.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1888
+        Summary(NoEvalCall)
+            .Case({ReturnValueCondition(WithinRange, SingleValue(-1))})
+            .ArgConstraint(NotNull(ArgNo(0))));
----------------
The same principle applies here too.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1905
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(WithinRange, Range(-1, UCharRangeMax))})
             .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
----------------
It's a sane overapproximation. Perfectly fine for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92771