This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by steakhal on Nov 30 2020, 3:39 AM.

Details

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.

Diff Detail

Event Timeline

steakhal created this revision.Nov 30 2020, 3:39 AM
steakhal requested review of this revision.Nov 30 2020, 3:39 AM
steakhal added inline comments.
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
1738

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

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

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
2

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 accepted this revision.Nov 30 2020, 5:05 AM

Looks good, thanks!

This revision is now accepted and ready to land.Nov 30 2020, 5:05 AM
vsavchenko accepted this revision.Nov 30 2020, 5:11 AM

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 added a subscriber: joerg.Nov 30 2020, 5:17 AM

off_t is s signed type. Please fix the description.

steakhal edited the summary of this revision. (Show Details)Nov 30 2020, 8:18 AM

off_t is s signed type. Please fix the description.

Oh, thanks. Updated.