llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Denys Fedoryshchenko (nuclearcat) <details> <summary>Changes</summary> Adds `SSIZE_MAX` size-argument constraints to the `unix.StdCLibraryFunctions` summaries for `read`, `write`, `readlink`, and `readlinkat`. Also adds summaries for `pread`, `pread64`, `pwrite`, and `pwrite64`, using the same size constraint and return-value bounds as `read` and `write`. The `readlink` and `readlinkat` success cases use `SSIZE_MAX` instead of `INT_MAX` for the buffer-size bound. Split out of #<!-- -->196499 so the Static Analyzer changes can be reviewed independently of the Sema diagnostics. Part of #<!-- -->142230. Tested with an assertions-enabled Clang build: all 24 `std-c-library-functions` analyzer tests passed. --- Full diff: https://github.com/llvm/llvm-project/pull/224980.diff 5 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+7) - (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp (+39-7) - (modified) clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h (+4) - (modified) clang/test/Analysis/std-c-library-functions-POSIX.c (+4) - (modified) clang/test/Analysis/std-c-library-functions-arg-constraints.c (+58) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 7e3e8468914c7..4d86b646dc165 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -929,6 +929,13 @@ features cannot lower the translation-unit ABI level; #### Improvements +- `unix.StdCLibraryFunctions` now diagnoses size arguments greater than + `SSIZE_MAX` passed to `read`, `write`, `readlink`, and `readlinkat`. + +- `unix.StdCLibraryFunctions` now models `pread`, `pread64`, `pwrite`, and + `pwrite64` with the same size constraint and return-value bounds as `read` + and `write`. + - The lock-order-reversal check in ``alpha.unix.PthreadLock`` is now disabled by default. It can be re-enabled with the ``WarnOnLockOrderReversal`` option. diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index f6a6e535f7638..1b4735d819e41 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -2069,12 +2069,17 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( std::optional<QualType> Ssize_tTy = lookupTy("ssize_t"); std::optional<RangeInt> Ssize_tMax = getMaxValue(Ssize_tTy); + auto ValidSsize_tSize = [&](ArgNo ArgN) { + return ArgumentCondition(ArgN, WithinRange, Range(0, Ssize_tMax), + "a value not greater than SSIZE_MAX"); + }; auto ReadSummary = Summary(NoEvalCall) .Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)), ReturnValueCondition(WithinRange, Range(-1, Ssize_tMax))}, - ErrnoIrrelevant); + ErrnoIrrelevant) + .ArgConstraint(ValidSsize_tSize(ArgNo(2))); // FIXME these are actually defined by POSIX and not by the C standard, we // should handle them together with the rest of the POSIX functions. @@ -3009,6 +3014,35 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( .ArgConstraint( ArgumentCondition(0, WithinRange, Range(0, IntMax)))); + // ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset); + addToFunctionSummaryMap( + "pread", + Signature(ArgTypes{IntTy, VoidPtrTy, SizeTyCanonTy, Off_tTy}, + RetType{Ssize_tTy}), + ReadSummary); + + // ssize_t pread64(int fildes, void *buf, size_t nbyte, off64_t offset); + addToFunctionSummaryMap( + "pread64", + Signature(ArgTypes{IntTy, VoidPtrTy, SizeTyCanonTy, Off64_tTy}, + RetType{Ssize_tTy}), + ReadSummary); + + // ssize_t pwrite(int fildes, const void *buf, size_t nbyte, off_t offset); + addToFunctionSummaryMap( + "pwrite", + Signature(ArgTypes{IntTy, ConstVoidPtrTy, SizeTyCanonTy, Off_tTy}, + RetType{Ssize_tTy}), + ReadSummary); + + // ssize_t pwrite64(int fildes, const void *buf, size_t nbyte, + // off64_t offset); + addToFunctionSummaryMap( + "pwrite64", + Signature(ArgTypes{IntTy, ConstVoidPtrTy, SizeTyCanonTy, Off64_tTy}, + RetType{Ssize_tTy}), + ReadSummary); + // ssize_t readlink(const char *restrict path, char *restrict buf, // size_t bufsize); addToFunctionSummaryMap( @@ -3017,7 +3051,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( ArgTypes{ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTyCanonTy}, RetType{Ssize_tTy}), Summary(NoEvalCall) - .Case({ArgumentCondition(2, WithinRange, Range(1, IntMax)), + .Case({ArgumentCondition(2, WithinRange, Range(1, Ssize_tMax)), ReturnValueCondition(LessThanOrEq, ArgNo(2)), ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))}, ErrnoMustNotBeChecked, GenericSuccessMsg) @@ -3030,8 +3064,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( .ArgConstraint(NotNull(ArgNo(1))) .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(1), /*BufSize=*/ArgNo(2))) - .ArgConstraint( - ArgumentCondition(2, WithinRange, Range(0, SizeMax)))); + .ArgConstraint(ValidSsize_tSize(ArgNo(2)))); // ssize_t readlinkat(int fd, const char *restrict path, // char *restrict buf, size_t bufsize); @@ -3041,7 +3074,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( SizeTyCanonTy}, RetType{Ssize_tTy}), Summary(NoEvalCall) - .Case({ArgumentCondition(3, WithinRange, Range(1, IntMax)), + .Case({ArgumentCondition(3, WithinRange, Range(1, Ssize_tMax)), ReturnValueCondition(LessThanOrEq, ArgNo(3)), ReturnValueCondition(WithinRange, Range(1, Ssize_tMax))}, ErrnoMustNotBeChecked, GenericSuccessMsg) @@ -3055,8 +3088,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( .ArgConstraint(NotNull(ArgNo(2))) .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(2), /*BufSize=*/ArgNo(3))) - .ArgConstraint( - ArgumentCondition(3, WithinRange, Range(0, SizeMax)))); + .ArgConstraint(ValidSsize_tSize(ArgNo(3)))); // int renameat(int olddirfd, const char *oldpath, int newdirfd, const char // *newpath); diff --git a/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h b/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h index b146068eedb08..83d753a22b347 100644 --- a/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h +++ b/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h @@ -124,6 +124,10 @@ 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); int pipe(int fildes[2]); off_t lseek(int fildes, off_t offset, int whence); +ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset); +ssize_t pread64(int fildes, void *buf, size_t nbyte, off64_t offset); +ssize_t pwrite(int fildes, const void *buf, size_t nbyte, off_t offset); +ssize_t pwrite64(int fildes, const void *buf, size_t nbyte, off64_t offset); ssize_t readlink(const char *restrict path, char *restrict buf, size_t bufsize); ssize_t readlinkat(int fd, const char *restrict path, char *restrict buf, size_t bufsize); int renameat(int olddirfd, const char *oldpath, int newdirfd, const char *newpath); diff --git a/clang/test/Analysis/std-c-library-functions-POSIX.c b/clang/test/Analysis/std-c-library-functions-POSIX.c index f6d88e6c1502d..619a049d79055 100644 --- a/clang/test/Analysis/std-c-library-functions-POSIX.c +++ b/clang/test/Analysis/std-c-library-functions-POSIX.c @@ -98,6 +98,10 @@ // CHECK: Loaded summary for: void *mmap64(void *addr, size_t length, int prot, int flags, int fd, off64_t offset) // CHECK: Loaded summary for: int pipe(int fildes[2]) // CHECK: Loaded summary for: off_t lseek(int fildes, off_t offset, int whence) +// CHECK: Loaded summary for: ssize_t pread(int fildes, void *buf, size_t nbyte, off_t offset) +// CHECK: Loaded summary for: ssize_t pread64(int fildes, void *buf, size_t nbyte, off64_t offset) +// CHECK: Loaded summary for: ssize_t pwrite(int fildes, const void *buf, size_t nbyte, off_t offset) +// CHECK: Loaded summary for: ssize_t pwrite64(int fildes, const void *buf, size_t nbyte, off64_t offset) // CHECK: Loaded summary for: ssize_t readlink(const char *restrict path, char *restrict buf, size_t bufsize) // CHECK: Loaded summary for: ssize_t readlinkat(int fd, const char *restrict path, char *restrict buf, size_t bufsize) // CHECK: Loaded summary for: int renameat(int olddirfd, const char *oldpath, int newdirfd, const char *newpath) diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c index 2cefa80341fc4..cd2d12d05ec83 100644 --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c @@ -370,3 +370,61 @@ void test_file_fd_at_functions() { (void)readlinkat(AT_FDCWD, "newpath", Buf, 10); (void)renameat(AT_FDCWD, "oldpath", AT_FDCWD, "newpath"); } + +#define SSIZE_MAX_PLUS_ONE ((size_t)1 << (sizeof(size_t) * __CHAR_BIT__ - 1)) + +void test_read_ssize_max_io_size(int fd, char *Buf) { + read(fd, Buf, SSIZE_MAX_PLUS_ONE); + // report-warning@-1{{The 3rd argument to 'read' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 3rd argument to 'read' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 3rd argument to 'read' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} + +void test_write_ssize_max_io_size(int fd, char *Buf) { + write(fd, Buf, SSIZE_MAX_PLUS_ONE); + // report-warning@-1{{The 3rd argument to 'write' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 3rd argument to 'write' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 3rd argument to 'write' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} + +void test_pread_ssize_max_io_size(int fd, char *Buf) { + pread(fd, Buf, SSIZE_MAX_PLUS_ONE, 0); + // report-warning@-1{{The 3rd argument to 'pread' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 3rd argument to 'pread' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 3rd argument to 'pread' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} + +void test_pread64_ssize_max_io_size(int fd, char *Buf) { + pread64(fd, Buf, SSIZE_MAX_PLUS_ONE, 0); + // report-warning@-1{{The 3rd argument to 'pread64' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 3rd argument to 'pread64' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 3rd argument to 'pread64' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} + +void test_pwrite_ssize_max_io_size(int fd, char *Buf) { + pwrite(fd, Buf, SSIZE_MAX_PLUS_ONE, 0); + // report-warning@-1{{The 3rd argument to 'pwrite' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 3rd argument to 'pwrite' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 3rd argument to 'pwrite' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} + +void test_pwrite64_ssize_max_io_size(int fd, char *Buf) { + pwrite64(fd, Buf, SSIZE_MAX_PLUS_ONE, 0); + // report-warning@-1{{The 3rd argument to 'pwrite64' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 3rd argument to 'pwrite64' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 3rd argument to 'pwrite64' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} + +void test_readlink_ssize_max_io_size(char *Buf) { + readlink("path", Buf, SSIZE_MAX_PLUS_ONE); + // report-warning@-1{{The 3rd argument to 'readlink' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 3rd argument to 'readlink' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 3rd argument to 'readlink' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} + +void test_readlinkat_ssize_max_io_size(char *Buf) { + readlinkat(AT_FDCWD, "path", Buf, SSIZE_MAX_PLUS_ONE); + // report-warning@-1{{The 4th argument to 'readlinkat' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-warning@-2{{The 4th argument to 'readlinkat' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} + // bugpath-note@-3{{The 4th argument to 'readlinkat' is 9223372036854775808 but should be a value not greater than SSIZE_MAX}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/224980 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
