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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
