https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/205371
From eb451e1439f5e0fe2b86ea01bf8e55859b00875e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <[email protected]> Date: Tue, 23 Jun 2026 17:08:07 +0200 Subject: [PATCH 1/2] [clang][analyzer] Add allocation failure modeling to DynamicMemoryModeling New option is added to the checker to create branches with null return value from memory allocations (off by default). --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 9 ++- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 75 ++++++++++++++++-- clang/test/Analysis/analyzer-config.c | 1 + clang/test/Analysis/malloc-failure.c | 76 +++++++++++++++++++ 4 files changed, 155 insertions(+), 6 deletions(-) create mode 100644 clang/test/Analysis/malloc-failure.c diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index eca2afbe340a9..0249d292e51cd 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -512,7 +512,14 @@ def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">, "NoStoreFuncVisitor.", "true", Released, - Hide> + Hide>, + CmdLineOption<Boolean, + "ModelAllocationFailure", + "At allocation functions, add extra execution branches for " + "failed memory allocation (function returns null value when " + "size is non-null) if applicable.", + "false", + Released> ]>, Dependencies<[CStringModeling]>, Documentation<NotDocumented>, diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 25ad8959cac5f..8c096a5097cbf 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -401,6 +401,14 @@ class MallocChecker bool ShouldRegisterNoOwnershipChangeVisitor = false; + /// Add extra branches for allocation failure. Generally a return value of an + /// allocation function is not constrained to be null or non-null and + /// information about a later null pointer access can be lost. When failure + /// branches are added, they contain the constrained null pointer return value + /// and allow detection of a null pointer access if the result of an + /// allocation is not checked for null. + bool ModelAllocationFailure = false; + // This checker family implements many bug types and frontends, and several // bug types are shared between multiple frontends, so most of the frontends // are declared with the helper class DynMemFrontend. @@ -466,6 +474,7 @@ class MallocChecker CHECK_FN(checkFree) CHECK_FN(checkIfNameIndex) CHECK_FN(checkBasicAlloc) + CHECK_FN(checkBasicAllocMayFail) CHECK_FN(checkKernelMalloc) CHECK_FN(checkCalloc) CHECK_FN(checkAlloca) @@ -530,7 +539,7 @@ class MallocChecker }; CallDescriptionMap<CheckFn> AllocatingMemFnMap{ - {{CDM::CLibrary, {"malloc"}, 1}, &MallocChecker::checkBasicAlloc}, + {{CDM::CLibrary, {"malloc"}, 1}, &MallocChecker::checkBasicAllocMayFail}, {{CDM::CLibrary, {"malloc"}, 3}, &MallocChecker::checkKernelMalloc}, {{CDM::CLibrary, {"calloc"}, 2}, &MallocChecker::checkCalloc}, {{CDM::CLibrary, {"valloc"}, 1}, &MallocChecker::checkBasicAlloc}, @@ -538,7 +547,7 @@ class MallocChecker {{CDM::CLibrary, {"strdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"_strdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"kmalloc"}, 2}, &MallocChecker::checkKernelMalloc}, - {{CDM::CLibrary, {"if_nameindex"}, 1}, &MallocChecker::checkIfNameIndex}, + {{CDM::CLibrary, {"if_nameindex"}, 0}, &MallocChecker::checkIfNameIndex}, {{CDM::CLibrary, {"wcsdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"_wcsdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"g_malloc"}, 1}, &MallocChecker::checkBasicAlloc}, @@ -663,6 +672,11 @@ class MallocChecker SVal Init, ProgramStateRef State, AllocationFamily Family) const; + [[nodiscard]] ProgramStateRef FailedAlloc(CheckerContext &C, + const CallEvent &Call, + int SizeArgI1, int SizeArgI2, + ProgramStateRef State) const; + // Check if this malloc() for special flags. At present that means M_ZERO or // __GFP_ZERO (in which case, treat it like calloc). [[nodiscard]] std::optional<ProgramStateRef> @@ -1354,6 +1368,17 @@ void MallocChecker::checkBasicAlloc(ProgramStateRef State, C.addTransition(State); } +void MallocChecker::checkBasicAllocMayFail(ProgramStateRef State, + const CallEvent &Call, + CheckerContext &C) const { + C.addTransition(FailedAlloc(C, Call, 0, -1, State)); + + State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State, + AllocationFamily(AF_Malloc)); + State = ProcessZeroAllocCheck(C, Call, 0, State); + C.addTransition(State); +} + void MallocChecker::checkKernelMalloc(ProgramStateRef State, const CallEvent &Call, CheckerContext &C) const { @@ -1389,14 +1414,18 @@ static bool isGRealloc(const CallEvent &Call) { void MallocChecker::checkRealloc(ProgramStateRef State, const CallEvent &Call, CheckerContext &C, bool ShouldFreeOnFail) const { + bool StandardRealloc = isStandardRealloc(Call); // Ignore calls to functions whose type does not match the expected type of // either the standard realloc or g_realloc from GLib. // FIXME: Should we perform this kind of checking consistently for each // function? If yes, then perhaps extend the `CallDescription` interface to // handle this. - if (!isStandardRealloc(Call) && !isGRealloc(Call)) + if (!StandardRealloc && !isGRealloc(Call)) return; + if (StandardRealloc) + C.addTransition(FailedAlloc(C, Call, 1, -1, State)); + State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(C, Call, 1, State); @@ -1405,6 +1434,8 @@ void MallocChecker::checkRealloc(ProgramStateRef State, const CallEvent &Call, void MallocChecker::checkCalloc(ProgramStateRef State, const CallEvent &Call, CheckerContext &C) const { + C.addTransition(FailedAlloc(C, Call, 0, 1, State)); + State = CallocMem(C, Call, State); State = ProcessZeroAllocCheck(C, Call, 0, State); State = ProcessZeroAllocCheck(C, Call, 1, State); @@ -1434,20 +1465,23 @@ void MallocChecker::checkStrdup(ProgramStateRef State, const CallEvent &Call, const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); if (!CE) return; + + C.addTransition(FailedAlloc(C, Call, -1, -1, State)); + State = MallocMemAux(C, Call, UnknownVal(), UnknownVal(), State, AllocationFamily(AF_Malloc)); - C.addTransition(State); } void MallocChecker::checkIfNameIndex(ProgramStateRef State, const CallEvent &Call, CheckerContext &C) const { + C.addTransition(FailedAlloc(C, Call, -1, -1, State)); + // Should we model this differently? We can allocate a fixed number of // elements with zeros in the last one. State = MallocMemAux(C, Call, UnknownVal(), UnknownVal(), State, AllocationFamily(AF_IfNameIndex)); - C.addTransition(State); } @@ -2033,6 +2067,7 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, SVal RetVal = State->getSVal(CE, C.getStackFrame()); // Fill the region with the initialization value. + // FIXME: Why use stack frame of the predecessor? State = State->bindDefaultInitial(RetVal, Init, SF); // If Size is somehow undefined at this point, this line prevents a crash. @@ -2048,6 +2083,33 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, return MallocUpdateRefState(C, CE, State, Family); } +ProgramStateRef MallocChecker::FailedAlloc(CheckerContext &C, + const CallEvent &Call, int SizeArgI1, + int SizeArgI2, + ProgramStateRef State) const { + if (!State || !ModelAllocationFailure) + return nullptr; + + auto AssumeArgNonZero = [&Call](ProgramStateRef State, + int ArgI) -> ProgramStateRef { + if (!State || ArgI < 0) + return State; + auto DefArgVal = Call.getArgSVal(ArgI).getAs<DefinedOrUnknownSVal>(); + if (!DefArgVal) + return nullptr; + return State->assume(*DefArgVal, true); + }; + + State = AssumeArgNonZero(State, SizeArgI1); + State = AssumeArgNonZero(State, SizeArgI2); + if (!State) + return nullptr; + + auto RetVal = State->getSVal(Call.getOriginExpr(), C.getStackFrame()) + .castAs<DefinedOrUnknownSVal>(); + return State->assume(RetVal, false); +} + static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State, AllocationFamily Family, @@ -4201,6 +4263,9 @@ void ento::registerDynamicMemoryModeling(CheckerManager &Mgr) { Chk->ShouldRegisterNoOwnershipChangeVisitor = Mgr.getAnalyzerOptions().getCheckerBooleanOption( DMMName, "AddNoOwnershipChangeNotes"); + Chk->ModelAllocationFailure = + Mgr.getAnalyzerOptions().getCheckerBooleanOption( + DMMName, "ModelAllocationFailure"); } bool ento::shouldRegisterDynamicMemoryModeling(const CheckerManager &mgr) { diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 04dc8c24421bc..a952c64aab0f4 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -135,6 +135,7 @@ // CHECK-NEXT: track-conditions = true // CHECK-NEXT: track-conditions-debug = false // CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = true +// CHECK-NEXT: unix.DynamicMemoryModeling:ModelAllocationFailure = false // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false // CHECK-NEXT: unix.Errno:AllowErrnoReadOutsideConditionExpressions = true // CHECK-NEXT: unix.StdCLibraryFunctions:DisplayLoadedSummaries = false diff --git a/clang/test/Analysis/malloc-failure.c b/clang/test/Analysis/malloc-failure.c new file mode 100644 index 0000000000000..aea58dad470fa --- /dev/null +++ b/clang/test/Analysis/malloc-failure.c @@ -0,0 +1,76 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core,unix.DynamicMemoryModeling \ +// RUN: -analyzer-config unix.DynamicMemoryModeling:ModelAllocationFailure=true +// RUN: %clang_analyze_cc1 -verify=nofailurebranch %s \ +// RUN: -analyzer-checker=core,unix.DynamicMemoryModeling \ +// RUN: -analyzer-config unix.DynamicMemoryModeling:ModelAllocationFailure=false + +// nofailurebranch-no-diagnostics + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); +void *alloca(size_t); +void *valloc(size_t); +void free(void *); +void *realloc(void *ptr, size_t size); +void *calloc(size_t nmemb, size_t size); +char *strdup(const char *s); +struct if_nameindex { char x; }; +struct if_nameindex *if_nameindex(void); +void if_freenameindex(struct if_nameindex *ptr); + +void test_malloc(size_t s) { + char *p = malloc(s); + if (s > 0) { + *p = 1; //expected-warning{{Dereference of null pointer}} + } else { + *p = 1; + } + free(p); +} + +void test_calloc(size_t n, size_t s) { + char *p = calloc(n, s); + if (n > 0 && s > 0) { + *p = 1; //expected-warning{{Dereference of null pointer}} + } else { + *p = 1; + } + free(p); +} + +void test_valloc(size_t s) { + int *p = valloc(s); + *p = 1; //no-warning + free(p); +} + +void test_alloca(size_t s) { + int *p = alloca(s); + *p = 1; //no-warning +} + +void test_realloc(size_t s) { + char *p = malloc(10); + if (!p) + return; + p = realloc(p, s); + if (s > 0) { + *p = 1; //expected-warning{{Dereference of null pointer}} + } else { + *p = 1; + } + free(p); +} + +void test_strdup() { + char *p = strdup("abcd"); + *p = 1; //expected-warning{{Dereference of null pointer}} + free(p); +} + +void test_ifnameindex() { + struct if_nameindex *p = if_nameindex(); + p->x = 1; //expected-warning{{dereference of a null pointer}} + if_freenameindex(p); +} From 722eadaf1d661956ffcc69f8006091ecb9b53d2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <[email protected]> Date: Thu, 25 Jun 2026 16:04:14 +0200 Subject: [PATCH 2/2] added release notes, restored if_nameindex --- clang/docs/ReleaseNotes.rst | 7 +++++++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 2 +- clang/test/Analysis/malloc-failure.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8a00b235860e4..6299a45ce5345 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -951,6 +951,13 @@ Crash and bug fixes - Fixed ``security.VAList`` checker producing false positives when analyzing C23 code where ``va_start`` expands to ``__builtin_c23_va_start``. +New checkers and features +^^^^^^^^^^^^^^^^^^^^^^^^^ +- Added a new configuration option + ``unix.DynamicMemoryModeling:ModelAllocationFailure`` that is used to add + failure-only branches to calls of ``malloc``-like functions. The new + functionality is off by default. + .. comment: This is for the Static Analyzer. Using the caret `^^^` underlining for subsections: diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 8c096a5097cbf..4b0efdec520af 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -547,7 +547,7 @@ class MallocChecker {{CDM::CLibrary, {"strdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"_strdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"kmalloc"}, 2}, &MallocChecker::checkKernelMalloc}, - {{CDM::CLibrary, {"if_nameindex"}, 0}, &MallocChecker::checkIfNameIndex}, + {{CDM::CLibrary, {"if_nameindex"}, 1}, &MallocChecker::checkIfNameIndex}, {{CDM::CLibrary, {"wcsdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"_wcsdup"}, 1}, &MallocChecker::checkStrdup}, {{CDM::CLibrary, {"g_malloc"}, 1}, &MallocChecker::checkBasicAlloc}, diff --git a/clang/test/Analysis/malloc-failure.c b/clang/test/Analysis/malloc-failure.c index aea58dad470fa..bf421fe7672c8 100644 --- a/clang/test/Analysis/malloc-failure.c +++ b/clang/test/Analysis/malloc-failure.c @@ -71,6 +71,6 @@ void test_strdup() { void test_ifnameindex() { struct if_nameindex *p = if_nameindex(); - p->x = 1; //expected-warning{{dereference of a null pointer}} + p->x = 1; //FIXME: if_nameindex is not recognized by the checker if_freenameindex(p); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
