[clang] Improve modeling of 'getcwd' in the StdLibraryFunctionsChecker (PR #77040)
Ben Shi via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 6 03:54:59 PST 2024
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 <bennshi at 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 <bennshi at 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}}
-}
More information about the cfe-commits
mailing list