[clang] [clang][analyzer] fix crash when modelling 'getline' function in checkers (PR #145229)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 22 04:30:25 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Baranov Victor (vbvictor)
<details>
<summary>Changes</summary>
Fixed crush caused by types mismatch of expected `getline` and `getdelim` signatures and actual user-defined signatures.
There are 2 ways to fix the issue:
1. Make sure that all parameters of `CallEvent` underlying `FunctionDecl` has the exact types of unix `getline` function. This way bloats the code by rigorous checking but may eliminate unnecessary transitions. I didn't find a way to eliminate code duplication, is there a `Utils` directory for CSA to place such utility functions?
2. Add more error-handling to `CheckGetDelim` methods to avoid crushes (as for now, it crushed trying to dereference an invalid `optional`). However, we will still try to model "incorrect" `getline` methods, wasting cpu.
I currently implemented 1st solution, but I'm happy to implement 2nd if it's more appropriate for CSA, WDYT?
I will create more PRs for other potential crushes of `unix`-function if we settle on the way of fixing it.
Addresses https://github.com/llvm/llvm-project/issues/144884.
---
Full diff: https://github.com/llvm/llvm-project/pull/145229.diff
4 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+3)
- (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+54-2)
- (modified) clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp (+50-1)
- (added) clang/test/Analysis/getline-unixapi-invalid-signatures.c (+130)
``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 12816eed2e8b5..b6f6dce29a87b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1060,6 +1060,9 @@ impact the linker behaviour like the other `-static-*` flags.
Crash and bug fixes
^^^^^^^^^^^^^^^^^^^
+- Fixed a crash in ``UnixAPIMisuseChecker`` and ``MallocChecker`` when analyzing
+ code with non-standard ``getline`` or ``getdelim`` function signatures.
+
Improvements
^^^^^^^^^^^^
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 35e98a5e2719a..b3f0110539ffa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1482,11 +1482,63 @@ static bool isFromStdNamespace(const CallEvent &Call) {
return FD->isInStdNamespace();
}
+static bool IsGetDelim(const CallEvent &Call, CheckerContext &C) {
+ const FunctionDecl *FD = dyn_cast_if_present<FunctionDecl>(Call.getDecl());
+ if (!FD || FD->getKind() != Decl::Function)
+ return false;
+
+ const StringRef FName = C.getCalleeName(FD);
+ const bool IsDelim = FName == "getdelim";
+ const bool IsLine = FName == "getline";
+
+ unsigned ExpectedNumArgs = 0;
+ if (IsDelim)
+ ExpectedNumArgs = 4;
+ else if (IsLine)
+ ExpectedNumArgs = 3;
+ else
+ return false;
+
+ if (FD->getNumParams() != ExpectedNumArgs)
+ return false;
+
+ // Should be char**
+ const QualType CharPtrType = FD->getParamDecl(0)->getType()->getPointeeType();
+ if (CharPtrType.isNull())
+ return false;
+ const QualType CharType = CharPtrType->getPointeeType();
+ if (CharType.isNull() || !CharType->isCharType())
+ return false;
+
+ // Should be size_t*
+ const QualType SizePtrType = FD->getParamDecl(1)->getType();
+ if (!SizePtrType->isPointerType())
+ return false;
+ const QualType SizeArgType = SizePtrType->getPointeeType();
+ const ASTContext &Ctx = C.getASTContext();
+ if (SizeArgType.isNull() ||
+ !Ctx.hasSameUnqualifiedType(SizeArgType, Ctx.getSizeType()))
+ return false;
+
+ // For getdelim, should be int
+ if (IsDelim)
+ if (!FD->getParamDecl(2)->getType()->isIntegerType())
+ return false;
+
+ // Last parameter should be FILE*
+ const QualType Pointee =
+ FD->getParamDecl(IsDelim ? 3 : 2)->getType()->getPointeeType();
+ if (Pointee.isNull() || Pointee.getAsString() != "FILE")
+ return false;
+
+ return true;
+}
+
void MallocChecker::preGetdelim(ProgramStateRef State, const CallEvent &Call,
CheckerContext &C) const {
// Discard calls to the C++ standard library function std::getline(), which
// is completely unrelated to the POSIX getline() that we're checking.
- if (isFromStdNamespace(Call))
+ if (isFromStdNamespace(Call) || !IsGetDelim(Call, C))
return;
const auto LinePtr = getPointeeVal(Call.getArgSVal(0), State);
@@ -1509,7 +1561,7 @@ void MallocChecker::checkGetdelim(ProgramStateRef State, const CallEvent &Call,
CheckerContext &C) const {
// Discard calls to the C++ standard library function std::getline(), which
// is completely unrelated to the POSIX getline() that we're checking.
- if (isFromStdNamespace(Call))
+ if (isFromStdNamespace(Call) || !IsGetDelim(Call, C))
return;
// Handle the post-conditions of getline and getdelim:
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 79d10d99e11d0..11d82f6de5e62 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -147,6 +147,55 @@ ProgramStateRef UnixAPIMisuseChecker::EnsurePtrNotNull(
return PtrNotNull;
}
+static bool IsGetDelim(const CallEvent &Call, const FunctionDecl *FD,
+ CheckerContext &C) {
+ const StringRef FName = C.getCalleeName(FD);
+ const bool IsDelim = FName == "getdelim";
+ const bool IsLine = FName == "getline";
+
+ unsigned ExpectedNumArgs = 0;
+ if (IsDelim)
+ ExpectedNumArgs = 4;
+ else if (IsLine)
+ ExpectedNumArgs = 3;
+ else
+ return false;
+
+ if (FD->getNumParams() != ExpectedNumArgs)
+ return false;
+
+ // Should be char**
+ const QualType CharPtrType = FD->getParamDecl(0)->getType()->getPointeeType();
+ if (CharPtrType.isNull())
+ return false;
+ const QualType CharType = CharPtrType->getPointeeType();
+ if (CharType.isNull() || !CharType->isCharType())
+ return false;
+
+ // Should be size_t*
+ const QualType SizePtrType = FD->getParamDecl(1)->getType();
+ if (!SizePtrType->isPointerType())
+ return false;
+ const QualType SizeArgType = SizePtrType->getPointeeType();
+ const ASTContext &Ctx = C.getASTContext();
+ if (SizeArgType.isNull() ||
+ !Ctx.hasSameUnqualifiedType(SizeArgType, Ctx.getSizeType()))
+ return false;
+
+ // For getdelim, should be int
+ if (IsDelim)
+ if (!FD->getParamDecl(2)->getType()->isIntegerType())
+ return false;
+
+ // Last parameter should be FILE*
+ const QualType Pointee =
+ FD->getParamDecl(IsDelim ? 3 : 2)->getType()->getPointeeType();
+ if (Pointee.isNull() || Pointee.getAsString() != "FILE")
+ return false;
+
+ return true;
+}
+
//===----------------------------------------------------------------------===//
// "open" (man 2 open)
//===----------------------------------------------------------------------===/
@@ -176,7 +225,7 @@ void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
else if (FName == "pthread_once")
CheckPthreadOnce(C, Call);
- else if (is_contained({"getdelim", "getline"}, FName))
+ else if (IsGetDelim(Call, FD, C))
CheckGetDelim(C, Call);
}
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
diff --git a/clang/test/Analysis/getline-unixapi-invalid-signatures.c b/clang/test/Analysis/getline-unixapi-invalid-signatures.c
new file mode 100644
index 0000000000000..ad218937f1184
--- /dev/null
+++ b/clang/test/Analysis/getline-unixapi-invalid-signatures.c
@@ -0,0 +1,130 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_CORRECT
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_2
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_3
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_4
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_5
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_GH144884
+
+// emulator of "system-header-simulator.h" because of redefinition of 'getline' function
+typedef struct _FILE FILE;
+typedef __typeof(sizeof(int)) size_t;
+typedef long ssize_t;
+#define NULL 0
+
+int fclose(FILE *fp);
+FILE *tmpfile(void);
+
+#ifdef TEST_CORRECT
+ssize_t getline(char **lineptr, size_t *n, FILE *stream);
+ssize_t getdelim(char **lineptr, size_t *n, int delimiter, FILE *stream);
+
+void test_correct() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ char *buffer = NULL;
+ getline(&buffer, NULL, F1); // expected-warning {{Size pointer might be NULL}}
+ fclose(F1);
+}
+
+void test_delim_correct() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ char *buffer = NULL;
+ getdelim(&buffer, NULL, ',', F1); // expected-warning {{Size pointer might be NULL}}
+ fclose(F1);
+}
+#endif
+
+#ifdef TEST_GETLINE_1
+// expected-no-diagnostics
+ssize_t getline(int lineptr);
+
+void test() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ int buffer = 0;
+ getline(buffer);
+ fclose(F1);
+}
+#endif
+
+#ifdef TEST_GETLINE_2
+// expected-no-diagnostics
+ssize_t getline(char **lineptr, size_t *n);
+
+void test() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ char *buffer = NULL;
+ getline(&buffer, NULL);
+ fclose(F1);
+}
+#endif
+
+#ifdef TEST_GETLINE_3
+// expected-no-diagnostics
+ssize_t getline(char **lineptr, size_t n, FILE *stream);
+
+void test() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ char *buffer = NULL;
+ getline(&buffer, 0, F1);
+ fclose(F1);
+}
+#endif
+
+#ifdef TEST_GETLINE_4
+// expected-no-diagnostics
+ssize_t getline(char **lineptr, size_t *n, int stream);
+ssize_t getdelim(char **lineptr, size_t *n, int delimiter, int stream);
+
+void test() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ char *buffer = NULL;
+ getline(&buffer, NULL, 1);
+ fclose(F1);
+}
+
+void test_delim() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ char *buffer = NULL;
+ getdelim(&buffer, NULL, ',', 1);
+ fclose(F1);
+}
+#endif
+
+#ifdef TEST_GETLINE_5
+// expected-no-diagnostics
+ssize_t getdelim(char **lineptr, size_t *n, const char* delimiter, FILE *stream);
+
+void test_delim() {
+ FILE *F1 = tmpfile();
+ if (!F1)
+ return;
+ char *buffer = NULL;
+ getdelim(&buffer, NULL, ",", F1);
+ fclose(F1);
+}
+#endif
+
+#ifdef TEST_GETLINE_GH144884
+// expected-no-diagnostics
+struct AW_string {};
+void getline(int *, struct AW_string);
+void top() {
+ struct AW_string line;
+ int getline_file_info;
+ getline(&getline_file_info, line);
+}
+#endif
``````````
</details>
https://github.com/llvm/llvm-project/pull/145229
More information about the cfe-commits
mailing list