Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve modeling of 'getcwd' in the StdLibraryFunctionsChecker #77040

Merged
merged 3 commits into from
Jan 9, 2024
Merged

Improve modeling of 'getcwd' in the StdLibraryFunctionsChecker #77040

merged 3 commits into from
Jan 9, 2024

Conversation

benshi001
Copy link
Member

  1. Improve the 'errno' modeling.
  2. Improve the buffer size argument's constraint.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Jan 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ben Shi (benshi001)

Changes
  1. Improve the 'errno' modeling.
  2. Improve the buffer size argument's constraint.

Full diff: https://github.com/llvm/llvm-project/pull/77040.diff

4 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp (+5-2)
  • (modified) clang/test/Analysis/errno-stdlibraryfunctions.c (+11)
  • (modified) clang/test/Analysis/std-c-library-functions-arg-constraints.c (+2)
  • (modified) clang/test/Analysis/std-c-library-functions-path-notes.c (+6)
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-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
index 0b817dda98c727..9011aee6b3f714 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -168,6 +168,7 @@ void test_notnull_concrete(FILE *fp) {
   // bugpath-warning{{The 1st argument to 'fread' is NULL but should not be NULL}} \
   // bugpath-note{{The 1st argument to 'fread' is NULL but should not be NULL}}
 }
+
 void test_notnull_symbolic(FILE *fp, int *buf) {
   fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(buf != 0); // \
@@ -176,6 +177,7 @@ void test_notnull_symbolic(FILE *fp, int *buf) {
   // bugpath-note{{TRUE}} \
   // bugpath-note{{'buf' is not equal to null}}
 }
+
 void test_notnull_symbolic2(FILE *fp, int *buf) {
   if (!buf)                          // bugpath-note{{Assuming 'buf' is null}} \
             // bugpath-note{{Taking true branch}}
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}}
+}

1. Improve the 'errno' modeling.
2. Make the range of the buffer size argument more accurate.
@benshi001 benshi001 requested a review from balazske January 6, 2024 11:55
@benshi001 benshi001 requested a review from balazske January 8, 2024 10:35
@benshi001 benshi001 merged commit 7dd2063 into llvm:main Jan 9, 2024
5 checks passed
@benshi001 benshi001 deleted the csa-getcwd branch January 9, 2024 01:28
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…77040)

1. Improve the 'errno' modeling.
2. Improve constraints of the arguments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants