https://github.com/benedekaibas updated https://github.com/llvm/llvm-project/pull/225489
>From f2d72cf338ff9c26dbffe6a34fc741d7aa05a80f Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Tue, 22 Sep 2026 19:57:05 +0200 Subject: [PATCH 1/6] [analyzer] Don't invalidate pointee of pointer-to-const on callback arguments --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 2 +- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 124 ++++++++++-------- clang/test/Analysis/call-invalidation.cpp | 25 ++++ 3 files changed, 92 insertions(+), 59 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 140ea90eafc4bd..7db15838bffa96 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3940,7 +3940,7 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( if (const RefState *RS = State->get<RegionState>(sym)) if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) - if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS)) + if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS) || (Call && Call->argumentsMayEscape())) State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } return State; diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 2338c06d5f9925..0eea0d686be2ea 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -236,6 +236,69 @@ static const MemRegion *getThisRegionBaseOrNull(const CallEvent &Call) { return nullptr; } +// Returns true if the Callee is one of the APIs that can let an +// argument escape even through a pointer to const paramter. +static bool escapingAPIs(const CallEvent &Call) { + const Decl *D = Call.getDecl(); + if (!D) + return false; + + const auto *FD = dyn_cast<FunctionDecl>(D); + if (!FD) + return false; + + const IdentifierInfo *II = FD->getIdentifier(); + if (!II) + return false; + + // This set of "escaping" APIs is + + // - 'int pthread_setspecific(ptheread_key k, const void *)' stores a + // value into thread local storage. The value can later be retrieved with + // 'void *ptheread_getspecific(pthread_key)'. So even thought the + // parameter is 'const void *', the region escapes through the call. + if (II->isStr("pthread_setspecific")) + return true; + + // - xpc_connection_set_context stores a value which can be retrieved later + // with xpc_connection_get_context. + if (II->isStr("xpc_connection_set_context")) + return true; + + // - funopen - sets a buffer for future IO calls. + if (II->isStr("funopen")) + return true; + + // - __cxa_demangle - can reallocate memory and can return the pointer to + // the input buffer. + if (II->isStr("__cxa_demangle")) + return true; + + StringRef FName = II->getName(); + + // - CoreFoundation functions that end with "NoCopy" can free a passed-in + // buffer even if it is const. + if (FName.ends_with("NoCopy")) + return true; + + // - NSXXInsertXX, for example NSMapInsertIfAbsent, since they can + // be deallocated by NSMapRemove. + if (FName.starts_with("NS") && FName.contains("Insert")) + return true; + + // - Many CF containers allow objects to escape through custom + // allocators/deallocators upon container construction. (PR12101) + if (FName.starts_with("CF") || FName.starts_with("CG")) { + return FName.contains_insensitive("InsertValue") || + FName.contains_insensitive("AddValue") || + FName.contains_insensitive("SetValue") || + FName.contains_insensitive("WithData") || + FName.contains_insensitive("AppendValue") || + FName.contains_insensitive("SetAttribute"); + } + return false; +} + ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, ProgramStateRef State) const { // Don't invalidate anything if the callee is marked pure/const. @@ -250,7 +313,7 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, // Indexes of arguments whose values will be preserved by the call. llvm::SmallSet<unsigned, 4> PreserveArgs; - if (!argumentsMayEscape()) + if (!escapingAPIs(*this) && !hasVoidPointerToNonConstArg()) findPtrToConstParams(PreserveArgs, *this); // We should not preserve the contents of the region pointed by "this" when @@ -635,64 +698,9 @@ void AnyFunctionCall::getInitialStackFrameContents(const StackFrame *CalleeSF, } bool AnyFunctionCall::argumentsMayEscape() const { - if (CallEvent::argumentsMayEscape() || hasVoidPointerToNonConstArg()) - return true; - - const FunctionDecl *D = getDecl(); - if (!D) - return true; - - const IdentifierInfo *II = D->getIdentifier(); - if (!II) - return false; - - // This set of "escaping" APIs is - - // - 'int pthread_setspecific(ptheread_key k, const void *)' stores a - // value into thread local storage. The value can later be retrieved with - // 'void *ptheread_getspecific(pthread_key)'. So even thought the - // parameter is 'const void *', the region escapes through the call. - if (II->isStr("pthread_setspecific")) - return true; - - // - xpc_connection_set_context stores a value which can be retrieved later - // with xpc_connection_get_context. - if (II->isStr("xpc_connection_set_context")) - return true; - - // - funopen - sets a buffer for future IO calls. - if (II->isStr("funopen")) - return true; - - // - __cxa_demangle - can reallocate memory and can return the pointer to - // the input buffer. - if (II->isStr("__cxa_demangle")) - return true; - - StringRef FName = II->getName(); - - // - CoreFoundation functions that end with "NoCopy" can free a passed-in - // buffer even if it is const. - if (FName.ends_with("NoCopy")) - return true; - - // - NSXXInsertXX, for example NSMapInsertIfAbsent, since they can - // be deallocated by NSMapRemove. - if (FName.starts_with("NS") && FName.contains("Insert")) - return true; - - // - Many CF containers allow objects to escape through custom - // allocators/deallocators upon container construction. (PR12101) - if (FName.starts_with("CF") || FName.starts_with("CG")) { - return FName.contains_insensitive("InsertValue") || - FName.contains_insensitive("AddValue") || - FName.contains_insensitive("SetValue") || - FName.contains_insensitive("WithData") || - FName.contains_insensitive("AppendValue") || - FName.contains_insensitive("SetAttribute"); - } - return false; + return CallEvent::argumentsMayEscape() || hasVoidPointerToNonConstArg() || + escapingAPIs(*this); } const FunctionDecl *SimpleFunctionCall::getDecl() const { diff --git a/clang/test/Analysis/call-invalidation.cpp b/clang/test/Analysis/call-invalidation.cpp index 2dad9a5dc64d31..67f362bafe2241 100644 --- a/clang/test/Analysis/call-invalidation.cpp +++ b/clang/test/Analysis/call-invalidation.cpp @@ -309,3 +309,28 @@ int aliasing_ptrs_via_mutable_and_const_ptrs() { return 100 / (x - 1); // expected-warning@-1 {{Division by zero}} FIXME: We shouldn't report this. } + +void gh222438_opaque(int *p, const int *q); +void gh222438_opaque_cb(int *p, const int *q, void (*cb)()); +void cb(); + +void testNoCallbackWithConstArg() { + int x = 0, y = 0; + gh222438_opaque(&x, &y); + clang_analyzer_eval(x == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} +} + +void testCallbackWithConstArg() { + int x = 1, y = 1; + gh222438_opaque_cb(&x, &y, cb); + clang_analyzer_eval(x == 1); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(y == 1); // expected-warning{{TRUE}} +} + +void testNullCallbackWithConstArg() { + int x = 1, y = 1; + gh222438_opaque_cb(&x, &y, 0); + clang_analyzer_eval(x == 1); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(y == 1); // expected-warning{{TRUE}} +} >From bceecc3fb072cdd5e3850393581228bb4813b20d Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Tue, 22 Sep 2026 21:12:42 +0200 Subject: [PATCH 2/6] Commit the formatted changes in MallocChecker. --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 7db15838bffa96..cfa868e4a2c11d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3940,7 +3940,7 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( if (const RefState *RS = State->get<RegionState>(sym)) if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) - if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS) || (Call && Call->argumentsMayEscape())) + if (IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS)) State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } return State; >From 0dca779d15debe431ec0a66b4a53179b02ef4914 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Tue, 22 Sep 2026 22:27:59 +0200 Subject: [PATCH 3/6] Re-run clang-format again. --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index cfa868e4a2c11d..3e8298703b407a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3940,7 +3940,8 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( if (const RefState *RS = State->get<RegionState>(sym)) if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) - if (IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS)) + if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS) || + (Call && Call->argumentsMayEscape())) State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } return State; >From ba73186b9c234bd110e062a6a8b8ad03a2fd61c3 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Fri, 9 Oct 2026 22:49:01 +0200 Subject: [PATCH 4/6] Inline argumentsMayEscape to MallocChecker. --- .../Core/PathSensitive/CallEvent.h | 2 - .../StaticAnalyzer/Checkers/MallocChecker.cpp | 219 +++++++++++------- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 71 +----- 3 files changed, 136 insertions(+), 156 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h index 2010e4b0da84b0..e8e9ef153953c7 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -525,8 +525,6 @@ class AnyFunctionCall : public CallEvent { RuntimeDefinition getRuntimeDefinition() const override; - bool argumentsMayEscape() const override; - void getInitialStackFrameContents(const StackFrame *CalleeSF, BindingsTy &Bindings) const override; diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 3e8298703b407a..74c2304e0769a6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3725,86 +3725,20 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, return state; } -bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( - const CallEvent *Call, - ProgramStateRef State, - SymbolRef &EscapingSymbol) const { - assert(Call); - EscapingSymbol = nullptr; - - // For now, assume that any C++ or block call can free memory. - // TODO: If we want to be more optimistic here, we'll need to make sure that - // regions escape to C++ containers. They seem to do that even now, but for - // mysterious reasons. - if (!isa<SimpleFunctionCall, ObjCMethodCall>(Call)) - return true; - - // Check Objective-C messages by selector name. - if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) { - // If it's not a framework call, or if it takes a callback, assume it - // can free memory. - if (!Call->isInSystemHeader() || Call->argumentsMayEscape()) - return true; - - // If it's a method we know about, handle it explicitly post-call. - // This should happen before the "freeWhenDone" check below. - if (isKnownDeallocObjCMethodName(*Msg)) - return false; - - // If there's a "freeWhenDone" parameter, but the method isn't one we know - // about, we can't be sure that the object will use free() to deallocate the - // memory, so we can't model it explicitly. The best we can do is use it to - // decide whether the pointer escapes. - if (std::optional<bool> FreeWhenDone = getFreeWhenDoneArg(*Msg)) - return *FreeWhenDone; - - // If the first selector piece ends with "NoCopy", and there is no - // "freeWhenDone" parameter set to zero, we know ownership is being - // transferred. Again, though, we can't be sure that the object will use - // free() to deallocate the memory, so we can't model it explicitly. - StringRef FirstSlot = Msg->getSelector().getNameForSlot(0); - if (FirstSlot.ends_with("NoCopy")) - return true; - - // If the first selector starts with addPointer, insertPointer, - // or replacePointer, assume we are dealing with NSPointerArray or similar. - // This is similar to C++ containers (vector); we still might want to check - // that the pointers get freed by following the container itself. - if (FirstSlot.starts_with("addPointer") || - FirstSlot.starts_with("insertPointer") || - FirstSlot.starts_with("replacePointer") || - FirstSlot == "valueWithPointer") { - return true; - } - - // We should escape receiver on call to 'init'. This is especially relevant - // to the receiver, as the corresponding symbol is usually not referenced - // after the call. - if (Msg->getMethodFamily() == OMF_init) { - EscapingSymbol = Msg->getReceiverSVal().getAsSymbol(); - return true; - } - - // Otherwise, assume that the method does not free memory. - // Most framework methods do not free memory. +// Returns true if an argument may escape through this call: this +// covers callees that take ownership of a buffer and may free it +// later and callees that let an argument escape even through a +// pointer to a const parameter. Also returns true for a non-null +// callback argument or a void pointer to non-const argument. +static bool mayFreeOrLetArgumentsEscape(const CallEvent *Call) { + const Decl *D = Call->getDecl(); + if (!D) return false; - } - // At this point the only thing left to handle is straight function calls. - const FunctionDecl *FD = cast<SimpleFunctionCall>(Call)->getDecl(); + const FunctionDecl *FD = dyn_cast<FunctionDecl>(D); if (!FD) - return true; - - // If it's one of the allocation functions we can reason about, we model - // its behavior explicitly. - if (isMemCall(*Call)) return false; - // If it's not a system call, assume it frees memory. - if (!Call->isInSystemHeader()) - return true; - - // White list the system functions whose arguments escape. const IdentifierInfo *II = FD->getIdentifier(); if (!II) return true; @@ -3831,15 +3765,17 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( // 'closefn' is specified (and if that function does free memory), // but it will not if closefn is not specified. // Currently, we do not inspect the 'closefn' function (PR12101). - if (FName == "funopen") + if (FName == "funopen") { if (Call->getNumArgs() >= 4 && Call->getArgSVal(4).isConstant(0)) return false; + return true; + } // Do not warn on pointers passed to 'setbuf' when used with std streams, // these leaks might be intentional when setting the buffer for stdio. // http://stackoverflow.com/questions/2671151/who-frees-setvbuf-buffer - if (FName == "setbuf" || FName =="setbuffer" || - FName == "setlinebuf" || FName == "setvbuf") { + if (FName == "setbuf" || FName == "setbuffer" || FName == "setlinebuf" || + FName == "setvbuf") { if (Call->getNumArgs() >= 1) { const Expr *ArgE = Call->getArgExpr(0)->IgnoreParenCasts(); if (const DeclRefExpr *ArgDRE = dyn_cast<DeclRefExpr>(ArgE)) @@ -3886,11 +3822,126 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( return true; } + // 'int pthread_setspecific(ptheread_key k, const void *)' stores a + // value into thread local storage. The value can later be retrieved with + // 'void *ptheread_getspecific(pthread_key)'. So even thought the + // parameter is 'const void *', the region escapes through the call. + if (II->isStr("pthread_setspecific")) + return true; + + // xpc_connection_set_context stores a value which can be retrieved later + // with xpc_connection_get_context. + if (II->isStr("xpc_connection_set_context")) + return true; + + // __cxa_demangle can reallocate memory and can return the pointer to + // the input buffer. + if (II->isStr("__cxa_demangle")) + return true; + + // NSXXInsertXX, for example NSMapInsertIfAbsent, since they can + // be deallocated by NSMapRemove. + if (FName.starts_with("NS") && FName.contains("Insert")) + return true; + + // Many CF containers allow objects to escape through custom + // allocators/deallocators upon container construction. (PR12101) + if (FName.starts_with("CF") || FName.starts_with("CG")) { + return FName.contains_insensitive("InsertValue") || + FName.contains_insensitive("AddValue") || + FName.contains_insensitive("SetValue") || + FName.contains_insensitive("WithData") || + FName.contains_insensitive("AppendValue") || + FName.contains_insensitive("SetAttribute"); + } + + if (Call->argumentsMayEscape() || Call->hasVoidPointerToNonConstArg()) + return true; + return false; +} + +static bool mayEscapeThroughKnownObjCAPI(const CallEvent *Call, + SymbolRef &EscapingSymbol) { + // Check Objective-C messages by selector name. + if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) { + EscapingSymbol = nullptr; + // If it takes a callback, assume it can free memory. + if (Call->argumentsMayEscape()) + return true; + + // If it's a method we know about, handle it explicitly post-call. + // This should happen before the "freeWhenDone" check below. + if (isKnownDeallocObjCMethodName(*Msg)) + return false; + + // If there's a "freeWhenDone" parameter, but the method isn't one we know + // about, we can't be sure that the object will use free() to deallocate the + // memory, so we can't model it explicitly. The best we can do is use it to + // decide whether the pointer escapes. + if (std::optional<bool> FreeWhenDone = getFreeWhenDoneArg(*Msg)) + return *FreeWhenDone; + + // If the first selector piece ends with "NoCopy", and there is no + // "freeWhenDone" parameter set to zero, we know ownership is being + // transferred. Again, though, we can't be sure that the object will use + // free() to deallocate the memory, so we can't model it explicitly. + StringRef FirstSlot = Msg->getSelector().getNameForSlot(0); + if (FirstSlot.ends_with("NoCopy")) + return true; + + // If the first selector starts with addPointer, insertPointer, + // or replacePointer, assume we are dealing with NSPointerArray or similar. + // This is similar to C++ containers (vector); we still might want to check + // that the pointers get freed by following the container itself. + if (FirstSlot.starts_with("addPointer") || + FirstSlot.starts_with("insertPointer") || + FirstSlot.starts_with("replacePointer") || + FirstSlot == "valueWithPointer") { + return true; + } + + // We should escape receiver on call to 'init'. This is especially relevant + // to the receiver, as the corresponding symbol is usually not referenced + // after the call. + if (Msg->getMethodFamily() == OMF_init) { + EscapingSymbol = Msg->getReceiverSVal().getAsSymbol(); + return true; + } + // Otherwise, assume that the method does not free memory. + // Most framework methods do not free memory. + return false; + } + return false; +} + +bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( + const CallEvent *Call, ProgramStateRef State, + SymbolRef &EscapingSymbol) const { + assert(Call); + EscapingSymbol = nullptr; + + // For now, assume that any C++ or block call can free memory. + // TODO: If we want to be more optimistic here, we'll need to make sure that + // regions escape to C++ containers. They seem to do that even now, but for + // mysterious reasons. + if (!isa<SimpleFunctionCall, ObjCMethodCall>(Call)) + return true; + + // If it's one of the allocation functions we can reason about, we model + // its behavior explicitly. + if (isMemCall(*Call)) + return false; + + // If it's not a system call, assume it frees memory. + if (!Call->isInSystemHeader()) + return true; + // Handle cases where we know a buffer's /address/ can escape. // Note that the above checks handle some special cases where we know that // even though the address escapes, it's still our responsibility to free the // buffer. - if (Call->argumentsMayEscape()) + if (mayFreeOrLetArgumentsEscape(Call) || + mayEscapeThroughKnownObjCAPI(Call, EscapingSymbol)) return true; // Otherwise, assume that the function does not free memory. @@ -3898,10 +3949,9 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( return false; } -ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State, - const InvalidatedSymbols &Escaped, - const CallEvent *Call, - PointerEscapeKind Kind) const { +ProgramStateRef MallocChecker::checkPointerEscape( + ProgramStateRef State, const InvalidatedSymbols &Escaped, + const CallEvent *Call, PointerEscapeKind Kind) const { return checkPointerEscapeAux(State, Escaped, Call, Kind, /*IsConstPointerEscape*/ false); } @@ -3941,7 +3991,8 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( if (const RefState *RS = State->get<RegionState>(sym)) if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS) || - (Call && Call->argumentsMayEscape())) + (Call && mayFreeOrLetArgumentsEscape(Call)) || + (Call && mayEscapeThroughKnownObjCAPI(Call, EscapingSymbol))) State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } return State; diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 0eea0d686be2ea..5ac77b070b508f 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -236,69 +236,6 @@ static const MemRegion *getThisRegionBaseOrNull(const CallEvent &Call) { return nullptr; } -// Returns true if the Callee is one of the APIs that can let an -// argument escape even through a pointer to const paramter. -static bool escapingAPIs(const CallEvent &Call) { - const Decl *D = Call.getDecl(); - if (!D) - return false; - - const auto *FD = dyn_cast<FunctionDecl>(D); - if (!FD) - return false; - - const IdentifierInfo *II = FD->getIdentifier(); - if (!II) - return false; - - // This set of "escaping" APIs is - - // - 'int pthread_setspecific(ptheread_key k, const void *)' stores a - // value into thread local storage. The value can later be retrieved with - // 'void *ptheread_getspecific(pthread_key)'. So even thought the - // parameter is 'const void *', the region escapes through the call. - if (II->isStr("pthread_setspecific")) - return true; - - // - xpc_connection_set_context stores a value which can be retrieved later - // with xpc_connection_get_context. - if (II->isStr("xpc_connection_set_context")) - return true; - - // - funopen - sets a buffer for future IO calls. - if (II->isStr("funopen")) - return true; - - // - __cxa_demangle - can reallocate memory and can return the pointer to - // the input buffer. - if (II->isStr("__cxa_demangle")) - return true; - - StringRef FName = II->getName(); - - // - CoreFoundation functions that end with "NoCopy" can free a passed-in - // buffer even if it is const. - if (FName.ends_with("NoCopy")) - return true; - - // - NSXXInsertXX, for example NSMapInsertIfAbsent, since they can - // be deallocated by NSMapRemove. - if (FName.starts_with("NS") && FName.contains("Insert")) - return true; - - // - Many CF containers allow objects to escape through custom - // allocators/deallocators upon container construction. (PR12101) - if (FName.starts_with("CF") || FName.starts_with("CG")) { - return FName.contains_insensitive("InsertValue") || - FName.contains_insensitive("AddValue") || - FName.contains_insensitive("SetValue") || - FName.contains_insensitive("WithData") || - FName.contains_insensitive("AppendValue") || - FName.contains_insensitive("SetAttribute"); - } - return false; -} - ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, ProgramStateRef State) const { // Don't invalidate anything if the callee is marked pure/const. @@ -313,7 +250,7 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, // Indexes of arguments whose values will be preserved by the call. llvm::SmallSet<unsigned, 4> PreserveArgs; - if (!escapingAPIs(*this) && !hasVoidPointerToNonConstArg()) + if (!hasVoidPointerToNonConstArg()) findPtrToConstParams(PreserveArgs, *this); // We should not preserve the contents of the region pointed by "this" when @@ -697,12 +634,6 @@ void AnyFunctionCall::getInitialStackFrameContents(const StackFrame *CalleeSF, addParameterValuesToBindings(CalleeSF, Bindings, SVB, *this, D->parameters()); } -bool AnyFunctionCall::argumentsMayEscape() const { - - return CallEvent::argumentsMayEscape() || hasVoidPointerToNonConstArg() || - escapingAPIs(*this); -} - const FunctionDecl *SimpleFunctionCall::getDecl() const { const FunctionDecl *D = getOriginExpr()->getDirectCallee(); if (D) >From 3f80b3593b3f6424aeee1d655b17bd344213d956 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Wed, 14 Oct 2026 23:12:09 +0200 Subject: [PATCH 5/6] Rewrite helper functions. --- .../Core/PathSensitive/CallEvent.h | 11 --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 83 +++++++++---------- .../Checkers/SimpleStreamChecker.cpp | 2 +- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 11 --- 4 files changed, 38 insertions(+), 69 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h index e8e9ef153953c7..6164b32e98385d 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -328,13 +328,6 @@ class CallEvent { /// Returns true if any of the arguments is void*. bool hasVoidPointerToNonConstArg() const; - /// Returns true if any of the arguments are known to escape to long- - /// term storage, even if this method will not modify them. - // NOTE: The exact semantics of this are still being defined! - // We don't really want a list of hardcoded exceptions in the long run, - // but we don't want duplicated lists of known APIs in the short term either. - virtual bool argumentsMayEscape() const { return hasNonZeroCallbackArg(); } - /// Returns true if the callee is an externally-visible function in the /// top-level namespace, such as \c malloc. /// @@ -666,8 +659,6 @@ class BlockCall : public CallEvent { return RuntimeDefinition(LambdaCallOperator); } - bool argumentsMayEscape() const override { return true; } - void getInitialStackFrameContents(const StackFrame *CalleeSF, BindingsTy &Bindings) const override; @@ -1336,8 +1327,6 @@ class ObjCMethodCall : public CallEvent { RuntimeDefinition getRuntimeDefinition() const override; - bool argumentsMayEscape() const override; - void getInitialStackFrameContents(const StackFrame *CalleeSF, BindingsTy &Bindings) const override; diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 74c2304e0769a6..7b13a697c130de 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -813,19 +813,11 @@ class MallocChecker void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, const Stmt *S) const; - /// Check if the function is known to free memory, or if it is - /// "interesting" and should be modeled explicitly. - /// - /// \param [out] EscapingSymbol A function might not free memory in general, - /// but could be known to free a particular symbol. In this case, false is - /// returned and the single escaping symbol is returned through the out - /// parameter. - /// - /// We assume that pointers do not escape through calls to system functions - /// not handled by this checker. - bool mayFreeAnyEscapedMemoryOrIsModeledExplicitly(const CallEvent *Call, - ProgramStateRef State, - SymbolRef &EscapingSymbol) const; + /// Returns true if the call may release memory that escapes to it. + /// Returns false for allocation functions which are modelled + /// explicitly and for system calls that this checker does not + /// handle. + bool mayReleaseEscapedRegions(const CallEvent &Call) const; /// Implementation of the checkPointerEscape callbacks. [[nodiscard]] ProgramStateRef @@ -3730,8 +3722,8 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, // later and callees that let an argument escape even through a // pointer to a const parameter. Also returns true for a non-null // callback argument or a void pointer to non-const argument. -static bool mayFreeOrLetArgumentsEscape(const CallEvent *Call) { - const Decl *D = Call->getDecl(); +static bool mayFreeOrLetArgumentsEscape(const CallEvent &Call) { + const Decl *D = Call.getDecl(); if (!D) return false; @@ -3750,8 +3742,8 @@ static bool mayFreeOrLetArgumentsEscape(const CallEvent *Call) { // Look for the deallocator argument. We know that the memory ownership // is not transferred only if the deallocator argument is // 'kCFAllocatorNull'. - for (unsigned i = 1; i < Call->getNumArgs(); ++i) { - const Expr *ArgE = Call->getArgExpr(i)->IgnoreParenCasts(); + for (unsigned i = 1; i < Call.getNumArgs(); ++i) { + const Expr *ArgE = Call.getArgExpr(i)->IgnoreParenCasts(); if (const DeclRefExpr *DE = dyn_cast<DeclRefExpr>(ArgE)) { StringRef DeallocatorName = DE->getFoundDecl()->getName(); if (DeallocatorName == "kCFAllocatorNull") @@ -3766,7 +3758,7 @@ static bool mayFreeOrLetArgumentsEscape(const CallEvent *Call) { // but it will not if closefn is not specified. // Currently, we do not inspect the 'closefn' function (PR12101). if (FName == "funopen") { - if (Call->getNumArgs() >= 4 && Call->getArgSVal(4).isConstant(0)) + if (Call.getNumArgs() >= 4 && Call.getArgSVal(4).isConstant(0)) return false; return true; } @@ -3776,8 +3768,8 @@ static bool mayFreeOrLetArgumentsEscape(const CallEvent *Call) { // http://stackoverflow.com/questions/2671151/who-frees-setvbuf-buffer if (FName == "setbuf" || FName == "setbuffer" || FName == "setlinebuf" || FName == "setvbuf") { - if (Call->getNumArgs() >= 1) { - const Expr *ArgE = Call->getArgExpr(0)->IgnoreParenCasts(); + if (Call.getNumArgs() >= 1) { + const Expr *ArgE = Call.getArgExpr(0)->IgnoreParenCasts(); if (const DeclRefExpr *ArgDRE = dyn_cast<DeclRefExpr>(ArgE)) if (const VarDecl *D = dyn_cast<VarDecl>(ArgDRE->getDecl())) if (D->getCanonicalDecl()->getName().contains("std")) @@ -3854,19 +3846,18 @@ static bool mayFreeOrLetArgumentsEscape(const CallEvent *Call) { FName.contains_insensitive("AppendValue") || FName.contains_insensitive("SetAttribute"); } - - if (Call->argumentsMayEscape() || Call->hasVoidPointerToNonConstArg()) + if (Call.hasNonZeroCallbackArg() || Call.hasVoidPointerToNonConstArg()) return true; return false; } -static bool mayEscapeThroughKnownObjCAPI(const CallEvent *Call, +static bool mayEscapeThroughKnownObjCAPI(const CallEvent &Call, SymbolRef &EscapingSymbol) { // Check Objective-C messages by selector name. - if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) { + if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call)) { EscapingSymbol = nullptr; // If it takes a callback, assume it can free memory. - if (Call->argumentsMayEscape()) + if (Call.hasNonZeroCallbackArg()) return true; // If it's a method we know about, handle it explicitly post-call. @@ -3914,12 +3905,15 @@ static bool mayEscapeThroughKnownObjCAPI(const CallEvent *Call, return false; } -bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( - const CallEvent *Call, ProgramStateRef State, - SymbolRef &EscapingSymbol) const { - assert(Call); - EscapingSymbol = nullptr; +// Return true for calls that may release memory escaping to them even +// when it is escaped through a pointer-to-const parameter. +static bool mayReleaseThroughPtrToConstParam(const CallEvent &Call, + SymbolRef &EscapingSymbol) { + return mayFreeOrLetArgumentsEscape(Call) || + mayEscapeThroughKnownObjCAPI(Call, EscapingSymbol); +} +bool MallocChecker::mayReleaseEscapedRegions(const CallEvent &Call) const { // For now, assume that any C++ or block call can free memory. // TODO: If we want to be more optimistic here, we'll need to make sure that // regions escape to C++ containers. They seem to do that even now, but for @@ -3929,19 +3923,11 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( // If it's one of the allocation functions we can reason about, we model // its behavior explicitly. - if (isMemCall(*Call)) + if (isMemCall(Call)) return false; // If it's not a system call, assume it frees memory. - if (!Call->isInSystemHeader()) - return true; - - // Handle cases where we know a buffer's /address/ can escape. - // Note that the above checks handle some special cases where we know that - // even though the address escapes, it's still our responsibility to free the - // buffer. - if (mayFreeOrLetArgumentsEscape(Call) || - mayEscapeThroughKnownObjCAPI(Call, EscapingSymbol)) + if (!Call.isInSystemHeader()) return true; // Otherwise, assume that the function does not free memory. @@ -3977,10 +3963,16 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( // If we know that the call does not free memory, or we want to process the // call later, keep tracking the top level arguments. SymbolRef EscapingSymbol = nullptr; - if (Kind == PSK_DirectEscapeOnCall && - !mayFreeAnyEscapedMemoryOrIsModeledExplicitly(Call, State, - EscapingSymbol) && - !EscapingSymbol) { + + bool MayReleaseEscaped = false; + bool MayReleaseThroughPTConst = false; + if (Call) { + MayReleaseEscaped = mayReleaseEscapedRegions(*Call); + MayReleaseThroughPTConst = + mayReleaseThroughPtrToConstParam(*Call, EscapingSymbol); + } + if (Kind == PSK_DirectEscapeOnCall && !MayReleaseThroughPTConst && + !MayReleaseEscaped) { return State; } @@ -3991,8 +3983,7 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( if (const RefState *RS = State->get<RegionState>(sym)) if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS) || - (Call && mayFreeOrLetArgumentsEscape(Call)) || - (Call && mayEscapeThroughKnownObjCAPI(Call, EscapingSymbol))) + MayReleaseThroughPTConst) State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } return State; diff --git a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp index 5152624d00f466..e1a41f6c6c99b7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -201,7 +201,7 @@ bool SimpleStreamChecker::guaranteedNotToCloseFile(const CallEvent &Call) const{ return false; // Handle cases where we know a buffer's /address/ can escape. - if (Call.argumentsMayEscape()) + if (Call.hasNonZeroCallbackArg()) return false; // Note, even though fclose closes the file, we do not list it here diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 5ac77b070b508f..23805b678ad206 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -1329,17 +1329,6 @@ RuntimeDefinition ObjCMethodCall::getRuntimeDefinition() const { return {}; } -bool ObjCMethodCall::argumentsMayEscape() const { - if (isInSystemHeader() && !isInstanceMessage()) { - Selector Sel = getSelector(); - if (Sel.getNumArgs() == 1 && - Sel.getIdentifierInfoForSlot(0)->isStr("valueWithPointer")) - return true; - } - - return CallEvent::argumentsMayEscape(); -} - void ObjCMethodCall::getInitialStackFrameContents(const StackFrame *CalleeSF, BindingsTy &Bindings) const { const auto *D = cast<ObjCMethodDecl>(CalleeSF->getDecl()); >From 0379a588101730f0c5d3a83fd8652d17661a60c8 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Thu, 15 Oct 2026 17:41:05 +0200 Subject: [PATCH 6/6] Rewrite the if guard and capitalize index variable. --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 91 ++++++++++--------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 7b13a697c130de..1f9540c7686b76 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3742,8 +3742,8 @@ static bool mayFreeOrLetArgumentsEscape(const CallEvent &Call) { // Look for the deallocator argument. We know that the memory ownership // is not transferred only if the deallocator argument is // 'kCFAllocatorNull'. - for (unsigned i = 1; i < Call.getNumArgs(); ++i) { - const Expr *ArgE = Call.getArgExpr(i)->IgnoreParenCasts(); + for (unsigned I = 1; I < Call.getNumArgs(); ++I) { + const Expr *ArgE = Call.getArgExpr(I)->IgnoreParenCasts(); if (const DeclRefExpr *DE = dyn_cast<DeclRefExpr>(ArgE)) { StringRef DeallocatorName = DE->getFoundDecl()->getName(); if (DeallocatorName == "kCFAllocatorNull") @@ -3854,54 +3854,55 @@ static bool mayFreeOrLetArgumentsEscape(const CallEvent &Call) { static bool mayEscapeThroughKnownObjCAPI(const CallEvent &Call, SymbolRef &EscapingSymbol) { // Check Objective-C messages by selector name. - if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call)) { - EscapingSymbol = nullptr; - // If it takes a callback, assume it can free memory. - if (Call.hasNonZeroCallbackArg()) - return true; + const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call); + if (!Msg) + return false; - // If it's a method we know about, handle it explicitly post-call. - // This should happen before the "freeWhenDone" check below. - if (isKnownDeallocObjCMethodName(*Msg)) - return false; + EscapingSymbol = nullptr; + // If it takes a callback, assume it can free memory. + if (Call.hasNonZeroCallbackArg()) + return true; - // If there's a "freeWhenDone" parameter, but the method isn't one we know - // about, we can't be sure that the object will use free() to deallocate the - // memory, so we can't model it explicitly. The best we can do is use it to - // decide whether the pointer escapes. - if (std::optional<bool> FreeWhenDone = getFreeWhenDoneArg(*Msg)) - return *FreeWhenDone; - - // If the first selector piece ends with "NoCopy", and there is no - // "freeWhenDone" parameter set to zero, we know ownership is being - // transferred. Again, though, we can't be sure that the object will use - // free() to deallocate the memory, so we can't model it explicitly. - StringRef FirstSlot = Msg->getSelector().getNameForSlot(0); - if (FirstSlot.ends_with("NoCopy")) - return true; + // If it's a method we know about, handle it explicitly post-call. + // This should happen before the "freeWhenDone" check below. + if (isKnownDeallocObjCMethodName(*Msg)) + return false; - // If the first selector starts with addPointer, insertPointer, - // or replacePointer, assume we are dealing with NSPointerArray or similar. - // This is similar to C++ containers (vector); we still might want to check - // that the pointers get freed by following the container itself. - if (FirstSlot.starts_with("addPointer") || - FirstSlot.starts_with("insertPointer") || - FirstSlot.starts_with("replacePointer") || - FirstSlot == "valueWithPointer") { - return true; - } + // If there's a "freeWhenDone" parameter, but the method isn't one we know + // about, we can't be sure that the object will use free() to deallocate the + // memory, so we can't model it explicitly. The best we can do is use it to + // decide whether the pointer escapes. + if (std::optional<bool> FreeWhenDone = getFreeWhenDoneArg(*Msg)) + return *FreeWhenDone; + + // If the first selector piece ends with "NoCopy", and there is no + // "freeWhenDone" parameter set to zero, we know ownership is being + // transferred. Again, though, we can't be sure that the object will use + // free() to deallocate the memory, so we can't model it explicitly. + StringRef FirstSlot = Msg->getSelector().getNameForSlot(0); + if (FirstSlot.ends_with("NoCopy")) + return true; - // We should escape receiver on call to 'init'. This is especially relevant - // to the receiver, as the corresponding symbol is usually not referenced - // after the call. - if (Msg->getMethodFamily() == OMF_init) { - EscapingSymbol = Msg->getReceiverSVal().getAsSymbol(); - return true; - } - // Otherwise, assume that the method does not free memory. - // Most framework methods do not free memory. - return false; + // If the first selector starts with addPointer, insertPointer, + // or replacePointer, assume we are dealing with NSPointerArray or similar. + // This is similar to C++ containers (vector); we still might want to check + // that the pointers get freed by following the container itself. + if (FirstSlot.starts_with("addPointer") || + FirstSlot.starts_with("insertPointer") || + FirstSlot.starts_with("replacePointer") || + FirstSlot == "valueWithPointer") { + return true; + } + + // We should escape receiver on call to 'init'. This is especially relevant + // to the receiver, as the corresponding symbol is usually not referenced + // after the call. + if (Msg->getMethodFamily() == OMF_init) { + EscapingSymbol = Msg->getReceiverSVal().getAsSymbol(); + return true; } + // Otherwise, assume that the method does not free memory. + // Most framework methods do not free memory. return false; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
