https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/77040
>From 10a0e9aae5effdd6e26476e78a778b89373358df Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Fri, 5 Jan 2024 10:05:15 +0800 Subject: [PATCH 1/2] Improve modeling of 'getcwd' in the StdLibraryFunctionsChecker 1. Improve the 'errno' modeling. 2. Make the range of the buffer size argument more accurate. --- clang/docs/ReleaseNotes.rst | 5 +++-- .../Checkers/StdLibraryFunctionsChecker.cpp | 7 +++++-- clang/test/Analysis/errno-stdlibraryfunctions.c | 11 +++++++++++ .../Analysis/std-c-library-functions-path-notes.c | 6 ++++++ 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ce7599ad34beaf..f59fe77b447aec 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1138,9 +1138,10 @@ Improvements ^^^^^^^^^^^^ - Improved the ``unix.StdCLibraryFunctions`` checker by modeling more - functions like ``send``, ``recv``, ``readlink``, ``fflush``, ``mkdtemp`` and - ``errno`` behavior. + functions like ``send``, ``recv``, ``readlink``, ``fflush``, ``mkdtemp``, + ``getcwd`` and ``errno`` behavior. (`52ac71f92d38 <https://github.com/llvm/llvm-project/commit/52ac71f92d38f75df5cb88e9c090ac5fd5a71548>`_, + `#77040 <https://github.com/llvm/llvm-project/pull/77040>`_, `#76671 <https://github.com/llvm/llvm-project/pull/76671>`_, `#71373 <https://github.com/llvm/llvm-project/pull/71373>`_, `#76557 <https://github.com/llvm/llvm-project/pull/76557>`_, diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 20068653d530a3..759de10601d08f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -2516,12 +2516,15 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( .ArgConstraint(NotNull(ArgNo(0)))); // char *getcwd(char *buf, size_t size); - // FIXME: Improve for errno modeling. addToFunctionSummaryMap( "getcwd", Signature(ArgTypes{CharPtrTy, SizeTy}, RetType{CharPtrTy}), Summary(NoEvalCall) + .Case({ReturnValueCondition(BO_EQ, ArgNo(0))}, + ErrnoMustNotBeChecked, GenericSuccessMsg) + .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant, GenericFailureMsg) + .ArgConstraint(NotNull(ArgNo(0))) .ArgConstraint( - ArgumentCondition(1, WithinRange, Range(0, SizeMax)))); + ArgumentCondition(1, WithinRange, Range(1, SizeMax)))); // int mkdir(const char *pathname, mode_t mode); addToFunctionSummaryMap( diff --git a/clang/test/Analysis/errno-stdlibraryfunctions.c b/clang/test/Analysis/errno-stdlibraryfunctions.c index 80e14c4e2923ca..b1317a2e2582de 100644 --- a/clang/test/Analysis/errno-stdlibraryfunctions.c +++ b/clang/test/Analysis/errno-stdlibraryfunctions.c @@ -74,3 +74,14 @@ void errno_mkdtemp(char *template) { if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} } } + +void errno_getcwd(char *Buf, size_t sz) { + char *Path = getcwd(Buf, sz); + if (Path == NULL) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} + if (errno) {} // no warning + } else { + clang_analyzer_eval(Path == Buf); // expected-warning{{TRUE}} + if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} + } +} diff --git a/clang/test/Analysis/std-c-library-functions-path-notes.c b/clang/test/Analysis/std-c-library-functions-path-notes.c index 4df00fe1e60646..0f5b9c08e9c0f3 100644 --- a/clang/test/Analysis/std-c-library-functions-path-notes.c +++ b/clang/test/Analysis/std-c-library-functions-path-notes.c @@ -89,3 +89,9 @@ int test_readlink_bufsize_zero(char *Buf, size_t Bufsize) { // expected-warning{{Division by zero}} \ // expected-note{{Division by zero}} } + +char *test_getcwd_bufsize_zero(char *Buf) { + return getcwd(Buf, 0); // \ + // expected-warning {{The 2nd argument to 'getcwd' is 0 but should be > 0}} \ + // expected-note {{The 2nd argument to 'getcwd' is 0 but should be > 0}} +} >From 2e76ecea9d86eb4d759feada271dd2e26b7cac82 Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Sat, 6 Jan 2024 19:35:42 +0800 Subject: [PATCH 2/2] Improve modeling of 'getcwd' in the StdLibraryFunctionsChecker --- .../Checkers/StdLibraryFunctionsChecker.cpp | 9 +++++++-- clang/test/Analysis/errno-stdlibraryfunctions.c | 17 ++++++++++------- .../std-c-library-functions-path-notes.c | 6 ------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 759de10601d08f..9645c99610d3f9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -2519,12 +2519,17 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( addToFunctionSummaryMap( "getcwd", Signature(ArgTypes{CharPtrTy, SizeTy}, RetType{CharPtrTy}), Summary(NoEvalCall) - .Case({ReturnValueCondition(BO_EQ, ArgNo(0))}, + .Case({ArgumentCondition(1, WithinRange, Range(1, SizeMax)), + ReturnValueCondition(BO_EQ, ArgNo(0))}, ErrnoMustNotBeChecked, GenericSuccessMsg) + .Case({ArgumentCondition(1, WithinRange, SingleValue(0))}, + ErrnoNEZeroIrrelevant, GenericFailureMsg) .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant, GenericFailureMsg) .ArgConstraint(NotNull(ArgNo(0))) .ArgConstraint( - ArgumentCondition(1, WithinRange, Range(1, SizeMax)))); + BufferSize(/*Buffer*/ ArgNo(0), /*BufSize*/ ArgNo(1))) + .ArgConstraint( + ArgumentCondition(1, WithinRange, Range(0, SizeMax)))); // int mkdir(const char *pathname, mode_t mode); addToFunctionSummaryMap( diff --git a/clang/test/Analysis/errno-stdlibraryfunctions.c b/clang/test/Analysis/errno-stdlibraryfunctions.c index b1317a2e2582de..2241a8c2b93815 100644 --- a/clang/test/Analysis/errno-stdlibraryfunctions.c +++ b/clang/test/Analysis/errno-stdlibraryfunctions.c @@ -75,13 +75,16 @@ void errno_mkdtemp(char *template) { } } -void errno_getcwd(char *Buf, size_t sz) { - char *Path = getcwd(Buf, sz); - if (Path == NULL) { - clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} - if (errno) {} // no warning +void errno_getcwd_general(char *Buf, size_t Sz) { + char *Path = getcwd(Buf, Sz); + if (Sz == 0) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} + if (errno) {} // no warning + } else if (Path == NULL) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} + if (errno) {} // no warning } else { - clang_analyzer_eval(Path == Buf); // expected-warning{{TRUE}} - if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} + clang_analyzer_eval(Path == Buf); // expected-warning{{TRUE}} + if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} } } diff --git a/clang/test/Analysis/std-c-library-functions-path-notes.c b/clang/test/Analysis/std-c-library-functions-path-notes.c index 0f5b9c08e9c0f3..4df00fe1e60646 100644 --- a/clang/test/Analysis/std-c-library-functions-path-notes.c +++ b/clang/test/Analysis/std-c-library-functions-path-notes.c @@ -89,9 +89,3 @@ int test_readlink_bufsize_zero(char *Buf, size_t Bufsize) { // expected-warning{{Division by zero}} \ // expected-note{{Division by zero}} } - -char *test_getcwd_bufsize_zero(char *Buf) { - return getcwd(Buf, 0); // \ - // expected-warning {{The 2nd argument to 'getcwd' is 0 but should be > 0}} \ - // expected-note {{The 2nd argument to 'getcwd' is 0 but should be > 0}} -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits