Author: Balázs Kéri Date: 2026-07-02T09:44:04+02:00 New Revision: b25e5be70aa7d335f07d95b4484e5638544f0f98
URL: https://github.com/llvm/llvm-project/commit/b25e5be70aa7d335f07d95b4484e5638544f0f98 DIFF: https://github.com/llvm/llvm-project/commit/b25e5be70aa7d335f07d95b4484e5638544f0f98.diff LOG: [clang][analyzer] Add allocation failure modeling to DynamicMemoryModeling (#205371) New option is added to the checker to create branches with null return value from memory allocations (off by default). Added: clang/test/Analysis/malloc-failure.c Modified: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/analyzer-config.c Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index b565481d28fdb..d33b21245bd7f 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..7ea028246a2ee 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}, @@ -663,6 +672,19 @@ class MallocChecker SVal Init, ProgramStateRef State, AllocationFamily Family) const; + /// Models a non-successful memory allocation. + /// Can be used if the allocation function may return null on failure when the + /// size to be allocated is non-zero. + /// + /// \param [in] Call The expression that allocates memory. + /// \param [in] State The \c ProgramState right before allocation. + /// \param [in] SizeArgIndexes Indexes of arguments that specify the + /// allocation size. + /// \returns The ProgramState right after an unsuccessful allocation. + [[nodiscard]] ProgramStateRef + FailedAlloc(CheckerContext &C, const CallEvent &Call, ProgramStateRef State, + llvm::ArrayRef<unsigned> SizeArgIndexes = {}) 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 +1376,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, State, {0})); + + 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 +1422,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, State, {1})); + State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(C, Call, 1, State); @@ -1405,6 +1442,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, State, {0, 1})); + State = CallocMem(C, Call, State); State = ProcessZeroAllocCheck(C, Call, 0, State); State = ProcessZeroAllocCheck(C, Call, 1, State); @@ -1434,20 +1473,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, 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, State)); + // Should we model this diff erently? 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 +2075,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 +2091,27 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, return MallocUpdateRefState(C, CE, State, Family); } +ProgramStateRef +MallocChecker::FailedAlloc(CheckerContext &C, const CallEvent &Call, + ProgramStateRef State, + llvm::ArrayRef<unsigned> SizeArgIndexes) const { + if (!State || !ModelAllocationFailure) + return nullptr; + + for (unsigned SizeArgI : SizeArgIndexes) { + auto DefArgVal = Call.getArgSVal(SizeArgI).getAs<DefinedOrUnknownSVal>(); + if (!DefArgVal) + return nullptr; + State = State->assume(*DefArgVal, true); + 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 +4265,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..bf421fe7672c8 --- /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; //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
