[analyzer][StdLibraryFunctionsChecker] Add return value constraint to functions with BufferSize

[steakhal]

I've double-checked the return values of each touched summary.
Everything seems fine to me, besides the two I've highlighted.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1775

1788

Same here.

1863

Aaa, I get it. We deal with this in a single transition. Fine.

2068

BTW, this is quite frequently repeated.
Do you think worth hoisting such a return value constraint?

I'm thinking of something like ReturnsZeroOrMinusOne.

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1775

The two constraints are applied in the same case (branch), the second is more generic. The first constraint may not have any effect if Arg2 is Unknown. Consequently, [-1, -1] would be too strict in such cases.

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

2068

Good idea!

[martong]

• Remove comments
• Hoist Range(-1,0) to ReturnsZeroOrMinusOne

[steakhal]

Awesome, thank you.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1775

I overlooked that we have a single Case, my bad.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

class StdLibraryFunctionsChecker
 
   public:
     Summary(InvalidationKind InvalidationKd) : InvalidationKd(InvalidationKd) {}
 
     Summary &Case(ConstraintSet &&CS) {
       CaseConstraints.push_back(std::move(CS));
       return *this;
     }
+    Summary &Case(const ConstraintSet &CS) {
+      CaseConstraints.push_back(CS);
+      return *this;
+    }
     Summary &ArgConstraint(ValueConstraintPtr VC) {
       assert(VC->getArgNo() != Ret &&
              "Arg constraint should not refer to the return value");
       ArgConstraints.push_back(VC);
       return *this;
     }
 
     InvalidationKind getInvalidationKd() const { return InvalidationKd; }
addToFunctionSummaryMap(
       "getchar", Signature(ArgTypes{}, RetType{IntTy}),
       Summary(NoEvalCall)
           .Case({ReturnValueCondition(WithinRange,
                                       {{EOFv, EOFv}, {0, UCharRangeMax}})}));
 
   // read()-like functions that never return more than buffer size.
   auto FreadSummary =
       Summary(NoEvalCall)
-          .Case({
-              ReturnValueCondition(LessThanOrEq, ArgNo(2)),
-          })
+          .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+                 ReturnValueCondition(WithinRange, Range(0, SizeMax))})
           .ArgConstraint(NotNull(ArgNo(0)))
           .ArgConstraint(NotNull(ArgNo(3)))
           .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1),
                                     /*BufSizeMultiplier=*/ArgNo(2)));
 
   // size_t fread(void *restrict ptr, size_t size, size_t nitems,
   //              FILE *restrict stream);
   addToFunctionSummaryMap(
if (ModelPOSIX) {
 
     // ssize_t readlink(const char *restrict path, char *restrict buf,
     //                  size_t bufsize);
     addToFunctionSummaryMap(
         "readlink",
         Signature(ArgTypes{ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTy},
                   RetType{Ssize_tTy}),
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+                   ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
             .ArgConstraint(NotNull(ArgNo(0)))
             .ArgConstraint(NotNull(ArgNo(1)))
             .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(1),
                                       /*BufSize=*/ArgNo(2)))
             .ArgConstraint(
                 ArgumentCondition(2, WithinRange, Range(0, SizeMax))));
 
     // ssize_t readlinkat(int fd, const char *restrict path,
     //                    char *restrict buf, size_t bufsize);
     addToFunctionSummaryMap(
         "readlinkat",
         Signature(
             ArgTypes{IntTy, ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTy},
             RetType{Ssize_tTy}),
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(LessThanOrEq, ArgNo(3)),
+                   ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
             .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
             .ArgConstraint(NotNull(ArgNo(1)))
             .ArgConstraint(NotNull(ArgNo(2)))
             .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(2),
                                       /*BufSize=*/ArgNo(3)))
             .ArgConstraint(
                 ArgumentCondition(3, WithinRange, Range(0, SizeMax))));
 
Optional<QualType> StructSockaddrPtrRestrictTy =
         getRestrictTy(StructSockaddrPtrTy);
     Optional<QualType> ConstStructSockaddrPtrRestrictTy =
         getRestrictTy(ConstStructSockaddrPtrTy);
     Optional<QualType> Socklen_tTy = lookupTy("socklen_t");
     Optional<QualType> Socklen_tPtrTy = getPointerTy(Socklen_tTy);
     Optional<QualType> Socklen_tPtrRestrictTy = getRestrictTy(Socklen_tPtrTy);
     Optional<RangeInt> Socklen_tMax = getMaxValue(Socklen_tTy);
 
+    const auto ReturnsZeroOrMinusOne =
+        ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, 0))};
+
     // In 'socket.h' of some libc implementations with C99, sockaddr parameter
     // is a transparent union of the underlying sockaddr_ family of pointers
     // instead of being a pointer to struct sockaddr. In these cases, the
     // standardized signature will not match, thus we try to match with another
     // signature that has the joker Irrelevant type. We also remove those
     // constraints which require pointer types for the sockaddr param.
     auto Accept =
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(WithinRange, Range(-1, IntMax))})
             .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)));
     if (!addToFunctionSummaryMap(
             "accept",
             // int accept(int socket, struct sockaddr *restrict address,
             //            socklen_t *restrict address_len);
             Signature(ArgTypes{IntTy, StructSockaddrPtrRestrictTy,
                                Socklen_tPtrRestrictTy},
                       RetType{IntTy}),
             Accept))
       addToFunctionSummaryMap(
           "accept",
           Signature(ArgTypes{IntTy, Irrelevant, Socklen_tPtrRestrictTy},
                     RetType{IntTy}),
           Accept);
 
     // int bind(int socket, const struct sockaddr *address, socklen_t
     //          address_len);
     if (!addToFunctionSummaryMap(
             "bind",
             Signature(ArgTypes{IntTy, ConstStructSockaddrPtrTy, Socklen_tTy},
                       RetType{IntTy}),
             Summary(NoEvalCall)
+                .Case(ReturnsZeroOrMinusOne)
                 .ArgConstraint(
                     ArgumentCondition(0, WithinRange, Range(0, IntMax)))
                 .ArgConstraint(NotNull(ArgNo(1)))
                 .ArgConstraint(
                     BufferSize(/*Buffer=*/ArgNo(1), /*BufSize=*/ArgNo(2)))
                 .ArgConstraint(
                     ArgumentCondition(2, WithinRange, Range(0, Socklen_tMax)))))
       // Do not add constraints on sockaddr.
       addToFunctionSummaryMap(
           "bind",
           Signature(ArgTypes{IntTy, Irrelevant, Socklen_tTy}, RetType{IntTy}),
           Summary(NoEvalCall)
+              .Case(ReturnsZeroOrMinusOne)
               .ArgConstraint(
                   ArgumentCondition(0, WithinRange, Range(0, IntMax)))
               .ArgConstraint(
                   ArgumentCondition(2, WithinRange, Range(0, Socklen_tMax))));
 
     // int getpeername(int socket, struct sockaddr *restrict address,
     //                 socklen_t *restrict address_len);
     if (!addToFunctionSummaryMap(
             "getpeername",
             Signature(ArgTypes{IntTy, StructSockaddrPtrRestrictTy,
                                Socklen_tPtrRestrictTy},
                       RetType{IntTy}),
             Summary(NoEvalCall)
+                .Case(ReturnsZeroOrMinusOne)
                 .ArgConstraint(
                     ArgumentCondition(0, WithinRange, Range(0, IntMax)))
                 .ArgConstraint(NotNull(ArgNo(1)))
                 .ArgConstraint(NotNull(ArgNo(2)))))
       addToFunctionSummaryMap(
           "getpeername",
           Signature(ArgTypes{IntTy, Irrelevant, Socklen_tPtrRestrictTy},
                     RetType{IntTy}),
           Summary(NoEvalCall)
+              .Case(ReturnsZeroOrMinusOne)
               .ArgConstraint(
                   ArgumentCondition(0, WithinRange, Range(0, IntMax))));
 
     // int getsockname(int socket, struct sockaddr *restrict address,
     //                 socklen_t *restrict address_len);
     if (!addToFunctionSummaryMap(
             "getsockname",
             Signature(ArgTypes{IntTy, StructSockaddrPtrRestrictTy,
                                Socklen_tPtrRestrictTy},
                       RetType{IntTy}),
             Summary(NoEvalCall)
+                .Case(ReturnsZeroOrMinusOne)
                 .ArgConstraint(
                     ArgumentCondition(0, WithinRange, Range(0, IntMax)))
                 .ArgConstraint(NotNull(ArgNo(1)))
                 .ArgConstraint(NotNull(ArgNo(2)))))
       addToFunctionSummaryMap(
           "getsockname",
           Signature(ArgTypes{IntTy, Irrelevant, Socklen_tPtrRestrictTy},
                     RetType{IntTy}),
           Summary(NoEvalCall)
+              .Case(ReturnsZeroOrMinusOne)
               .ArgConstraint(
                   ArgumentCondition(0, WithinRange, Range(0, IntMax))));
 
     // int connect(int socket, const struct sockaddr *address, socklen_t
     //             address_len);
     if (!addToFunctionSummaryMap(
             "connect",
             Signature(ArgTypes{IntTy, ConstStructSockaddrPtrTy, Socklen_tTy},
                       RetType{IntTy}),
             Summary(NoEvalCall)
+                .Case(ReturnsZeroOrMinusOne)
                 .ArgConstraint(
                     ArgumentCondition(0, WithinRange, Range(0, IntMax)))
                 .ArgConstraint(NotNull(ArgNo(1)))))
       addToFunctionSummaryMap(
           "connect",
           Signature(ArgTypes{IntTy, Irrelevant, Socklen_tTy}, RetType{IntTy}),
           Summary(NoEvalCall)
+              .Case(ReturnsZeroOrMinusOne)
               .ArgConstraint(
                   ArgumentCondition(0, WithinRange, Range(0, IntMax))));
 
     auto Recvfrom =
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+                   ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
             .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
             .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(1),
                                       /*BufSize=*/ArgNo(2)));
     if (!addToFunctionSummaryMap(
             "recvfrom",
             // ssize_t recvfrom(int socket, void *restrict buffer,
             //                  size_t length,
             //                  int flags, struct sockaddr *restrict address,
             //                  socklen_t *restrict address_len);
             Signature(ArgTypes{IntTy, VoidPtrRestrictTy, SizeTy, IntTy,
                                StructSockaddrPtrRestrictTy,
                                Socklen_tPtrRestrictTy},
                       RetType{Ssize_tTy}),
             Recvfrom))
       addToFunctionSummaryMap(
           "recvfrom",
           Signature(ArgTypes{IntTy, VoidPtrRestrictTy, SizeTy, IntTy,
                              Irrelevant, Socklen_tPtrRestrictTy},
                     RetType{Ssize_tTy}),
           Recvfrom);
 
     auto Sendto =
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+                   ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
             .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
             .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(1),
                                       /*BufSize=*/ArgNo(2)));
     if (!addToFunctionSummaryMap(
             "sendto",
             // ssize_t sendto(int socket, const void *message, size_t length,
             //                int flags, const struct sockaddr *dest_addr,
             //                socklen_t dest_len);
             Signature(ArgTypes{IntTy, ConstVoidPtrTy, SizeTy, IntTy,
                                ConstStructSockaddrPtrTy, Socklen_tTy},
                       RetType{Ssize_tTy}),
             Sendto))
       addToFunctionSummaryMap(
           "sendto",
           Signature(ArgTypes{IntTy, ConstVoidPtrTy, SizeTy, IntTy, Irrelevant,
                              Socklen_tTy},
                     RetType{Ssize_tTy}),
           Sendto);
 
     // int listen(int sockfd, int backlog);
     addToFunctionSummaryMap("listen",
                             Signature(ArgTypes{IntTy, IntTy}, RetType{IntTy}),
                             Summary(NoEvalCall)
+                                .Case(ReturnsZeroOrMinusOne)
                                 .ArgConstraint(ArgumentCondition(
                                     0, WithinRange, Range(0, IntMax))));
 
     // ssize_t recv(int sockfd, void *buf, size_t len, int flags);
     addToFunctionSummaryMap(
         "recv",
         Signature(ArgTypes{IntTy, VoidPtrTy, SizeTy, IntTy},
                   RetType{Ssize_tTy}),
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+                   ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
             .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
             .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(1),
                                       /*BufSize=*/ArgNo(2))));
 
     Optional<QualType> StructMsghdrTy = lookupTy("msghdr");
     Optional<QualType> StructMsghdrPtrTy = getPointerTy(StructMsghdrTy);
     Optional<QualType> ConstStructMsghdrPtrTy =
         getPointerTy(getConstTy(StructMsghdrTy));
 
     // ssize_t recvmsg(int sockfd, struct msghdr *msg, int flags);
-    addToFunctionSummaryMap("recvmsg",
-                            Signature(ArgTypes{IntTy, StructMsghdrPtrTy, IntTy},
-                                      RetType{Ssize_tTy}),
-                            Summary(NoEvalCall)
-                                .ArgConstraint(ArgumentCondition(
-                                    0, WithinRange, Range(0, IntMax))));
+    addToFunctionSummaryMap(
+        "recvmsg",
+            .Case({ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
+            .ArgConstraint(
+                ArgumentCondition(0, WithinRange, Range(0, IntMax))));
 
     // ssize_t sendmsg(int sockfd, const struct msghdr *msg, int flags);
     addToFunctionSummaryMap(
         "sendmsg",
         Signature(ArgTypes{IntTy, ConstStructMsghdrPtrTy, IntTy},
                   RetType{Ssize_tTy}),
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
             .ArgConstraint(
                 ArgumentCondition(0, WithinRange, Range(0, IntMax))));
 
     // int setsockopt(int socket, int level, int option_name,
     //                const void *option_value, socklen_t option_len);
     addToFunctionSummaryMap(
         "setsockopt",
         Signature(ArgTypes{IntTy, IntTy, IntTy, ConstVoidPtrTy, Socklen_tTy},
                   RetType{IntTy}),
         Summary(NoEvalCall)
+            .Case(ReturnsZeroOrMinusOne)
             .ArgConstraint(NotNull(ArgNo(3)))
             .ArgConstraint(
                 BufferSize(/*Buffer=*/ArgNo(3), /*BufSize=*/ArgNo(4)))
             .ArgConstraint(
                 ArgumentCondition(4, WithinRange, Range(0, Socklen_tMax))));
 
     // int getsockopt(int socket, int level, int option_name,
     //                void *restrict option_value,
     //                socklen_t *restrict option_len);
     addToFunctionSummaryMap(
         "getsockopt",
         Signature(ArgTypes{IntTy, IntTy, IntTy, VoidPtrRestrictTy,
                            Socklen_tPtrRestrictTy},
                   RetType{IntTy}),
         Summary(NoEvalCall)
+            .Case(ReturnsZeroOrMinusOne)
             .ArgConstraint(NotNull(ArgNo(3)))
             .ArgConstraint(NotNull(ArgNo(4))));
 
     // ssize_t send(int sockfd, const void *buf, size_t len, int flags);
     addToFunctionSummaryMap(
         "send",
         Signature(ArgTypes{IntTy, ConstVoidPtrTy, SizeTy, IntTy},
                   RetType{Ssize_tTy}),
         Summary(NoEvalCall)
+            .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)),
+                   ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))})
             .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
             .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(1),
                                       /*BufSize=*/ArgNo(2))));
 
     // int socketpair(int domain, int type, int protocol, int sv[2]);
     addToFunctionSummaryMap(
         "socketpair",
         Signature(ArgTypes{IntTy, IntTy, IntTy, IntPtrTy}, RetType{IntTy}),
-        Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(3))));
+        Summary(NoEvalCall)
+            .Case(ReturnsZeroOrMinusOne)
+            .ArgConstraint(NotNull(ArgNo(3))));
 
     // int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen,
     //                 char *restrict node, socklen_t nodelen,
     //                 char *restrict service,
     //                 socklen_t servicelen, int flags);
     //
     // This is defined in netdb.h. And contrary to 'socket.h', the sockaddr
     // parameter is never handled as a transparent union in netdb.h