[PATCH] D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions

[Gabor Marton via Phabricator via cfe-commits]

martong marked 2 inline comments as done.
martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1735-1747
+      if (!addToFunctionSummaryMap(
+              "accept", Summary(ArgTypes{IntTy, *StructSockaddrPtrRestrictTy,
+                                         *Socklen_tPtrRestrictTy},
+                                RetType{IntTy}, NoEvalCall)
+                            .ArgConstraint(ArgumentCondition(
+                                0, WithinRange, Range(0, IntMax)))))
+        // Do not add constraints on sockaddr.
----------------
Szelethus wrote:
> martong wrote:
> > Szelethus wrote:
> > > This is quite a bit of code duplication -- if we failed to match the precise `accept`, we just try again with the middle argument type being unspecified? Whats the difference between this and trying to match with `Irrelevant` straight away?
> > >  if we failed to match the precise accept, we just try again with the middle argument type being unspecified?
> > Yes. And that will be a successful match in the case where `sockaddr` is a transparent union.
> > 
> > > Whats the difference between this and trying to match with Irrelevant straight away?
> > Yeah, it's a good catch. There is no difference with `accept` because we don't have any constraints on `sockaddr` anyway. But, the other functions have fewer constraints with the transparent union.
> > Perhaps we should separate the body of the `Summary` from the `Signature` and in this case we could reuse that for both signatures. Or as you suggested we could just match by default with the `Irrelevant`. What do you think?
> > Perhaps we should separate the body of the Summary from the Signature and in this case we could reuse that for both signatures. Or as you suggested we could just match by default with the Irrelevant. What do you think?
> 
> Separating the summary and the signature sounds great! But, that also adds sufficiently complex code that would definitely warrant individual tests, so we should add that as well.
Ok, so I added a new overload to `addToFunctionSummaryMap` that can take a `Signature`. This signature is set to the given Summary, this way we can avoid the code duplication. Note, this change involved that the Signature is no longer a const member of the Summary, plus the Args and Ret cannot be a const member anymore of the Signature (we have to overwrite the signature of the given summary and that involves the copy assignment op).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83407