https://github.com/vbvictor created https://github.com/llvm/llvm-project/pull/145229
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. >From d2793c211ce50b24591bd5c233239a139ad941bd Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Sun, 22 Jun 2025 13:54:46 +0300 Subject: [PATCH] [clang][analyzer] fix crash when modelling 'getline' --- clang/docs/ReleaseNotes.rst | 3 + .../StaticAnalyzer/Checkers/MallocChecker.cpp | 56 +++++++- .../Checkers/UnixAPIChecker.cpp | 51 ++++++- .../getline-unixapi-invalid-signatures.c | 130 ++++++++++++++++++ 4 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 clang/test/Analysis/getline-unixapi-invalid-signatures.c 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
