Szelethus 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. ---------------- 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. 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