[analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64

[steakhal]

Summary

The fd parameter of

void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset)

should be constrained to the range [0, IntMax] as that is of type int.
Constraining to the range [0, Off_tMax] would result in a crash as that is
of a signed type with the value of 0xff..f (-1).

The crash would happen when we try to apply the arg constraints.
At line 583: assert(Min <= Max), as 0 <= -1 is not satisfied

The mmap64 is fixed for the same reason.

[steakhal]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1738

BTW this should have referred to Off64_tTy instead of Off_tTy.
Not a problem anymore :D

[vsavchenko]

That's a good fix!
How did this happen though that max value of off_t was even used for fd. Seems pretty odd!

[martong]

In D92307#2422468, @vsavchenko wrote:

That's a good fix!
How did this happen though that max value of off_t was even used for fd. Seems pretty odd!

I used a semi-automated approach to create these summaries from cppcheck : I translated the XML into C++ source code, but could not do the translation for ranges. E.g., 0: was just simply copied and I had to manually modify the generated C++ code to have the proper range. And that's where I made the wrong index for the param (cppcheck starts from idx 1, here we start from idx 0). Here the 5th param has the off_t type, so I thought we have to get the max for off_t.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1738

Yep, copy pasta error again, my bad :(

clang/test/Analysis/std-c-library-posix-crash.c

1

I like this new test file. Probably we are going to extend this as we advance with the evaluation of these summaries on open-source projects.

[martong]

Looks good, thanks!

[vsavchenko]

In D92307#2422603, @martong wrote:

In D92307#2422468, @vsavchenko wrote:

That's a good fix!
How did this happen though that max value of off_t was even used for fd. Seems pretty odd!

I used a semi-automated approach to create these summaries from cppcheck : I translated the XML into C++ source code, but could not do the translation for ranges. E.g., 0: was just simply copied and I had to manually modify the generated C++ code to have the proper range. And that's where I made the wrong index for the param (cppcheck starts from idx 1, here we start from idx 0). Here the 5th param has the off_t type, so I thought we have to get the max for off_t.

I see, thanks for clarification!

[joerg]

off_t is s signed type. Please fix the description.

[steakhal]

In D92307#2422624, @joerg wrote:

off_t is s signed type. Please fix the description.

Oh, thanks. Updated.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

addToFunctionSummaryMap(
         Signature(ArgTypes{FilePtrTy, Off_tTy, IntTy}, RetType{IntTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
 
     // off_t ftello(FILE *stream);
     addToFunctionSummaryMap(
         "ftello", Signature(ArgTypes{FilePtrTy}, RetType{Off_tTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
 
-    Optional<RangeInt> Off_tMax = getMaxValue(Off_tTy);
     // void *mmap(void *addr, size_t length, int prot, int flags, int fd,
     // off_t offset);
     addToFunctionSummaryMap(
         "mmap",
         Signature(ArgTypes{VoidPtrTy, SizeTy, IntTy, IntTy, IntTy, Off_tTy},
                   RetType{VoidPtrTy}),
         Summary(NoEvalCall)
             .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, SizeMax)))
             .ArgConstraint(
-                ArgumentCondition(4, WithinRange, Range(0, Off_tMax))));
+                ArgumentCondition(4, WithinRange, Range(0, IntMax))));
 
     Optional<QualType> Off64_tTy = lookupTy("off64_t");
-    Optional<RangeInt> Off64_tMax = getMaxValue(Off_tTy);
     // void *mmap64(void *addr, size_t length, int prot, int flags, int fd,
     // off64_t offset);
     addToFunctionSummaryMap(
         "mmap64",
         Signature(ArgTypes{VoidPtrTy, SizeTy, IntTy, IntTy, IntTy, Off64_tTy},
                   RetType{VoidPtrTy}),
         Summary(NoEvalCall)
             .ArgConstraint(ArgumentCondition(1, WithinRange, Range(1, SizeMax)))
             .ArgConstraint(
-                ArgumentCondition(4, WithinRange, Range(0, Off64_tMax))));
+                ArgumentCondition(4, WithinRange, Range(0, IntMax))));
 
     // int pipe(int fildes[2]);
     addToFunctionSummaryMap(
         "pipe", Signature(ArgTypes{IntPtrTy}, RetType{IntTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
 
     // off_t lseek(int fildes, off_t offset, int whence);
     addToFunctionSummaryMap(

clang/test/Analysis/std-c-library-posix-crash.c

+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -verify %s
+//
+// expected-no-diagnostics
+
+typedef long off_t;
+typedef long long off64_t;
+typedef unsigned long size_t;
+
+void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset);
+void *mmap64(void *addr, size_t length, int prot, int flags, int fd, off64_t offset);
+
+void test(long len) {
+  mmap(0, len, 2, 1, 0, 0);   // no-crash
+  mmap64(0, len, 2, 1, 0, 0); // no-crash
+}