diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2379,6 +2379,13 @@ let Documentation = [Undocumented]; } +def OwnershipParam : InheritableParamAttr { + let Spellings = [Clang<"ownership_takes_param">]; + let Subjects = SubjectList<[ParmVar]>; + let Documentation = [Undocumented]; + let SimpleHandler = 1; +} + def Packed : InheritableAttr { let Spellings = [GCC<"packed">]; // let Subjects = [Tag, Field]; diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -301,11 +301,7 @@ check::NewAllocator, check::PostStmt, check::PostObjCMessage, check::Location, eval::Assume> { public: - /// In pessimistic mode, the checker assumes that it does not know which - /// functions might free the memory. - /// In optimistic mode, the checker assumes that all user-defined functions - /// which might free a pointer are annotated. - DefaultBool ShouldIncludeOwnershipAnnotatedFunctions; + DefaultBool IsOptimisticAnalyzerOptionEnabled; DefaultBool ShouldRegisterNoOwnershipChangeVisitor; @@ -382,6 +378,7 @@ CHECK_FN(checkGMallocN0) CHECK_FN(checkReallocN) CHECK_FN(checkOwnershipAttr) + CHECK_FN(checkOwnershipParamAttr) void checkRealloc(const CallEvent &Call, CheckerContext &C, bool ShouldFreeOnFail) const; @@ -437,6 +434,8 @@ }; bool isMemCall(const CallEvent &Call) const; + + bool isOwnershipAttrSupportEnabled() const; // TODO: Remove mutable by moving the initializtaion to the registry function. mutable Optional KernelZeroFlagVal; @@ -548,6 +547,11 @@ ProgramStateRef FreeMemAttr(CheckerContext &C, const CallEvent &Call, const OwnershipAttr *Att, ProgramStateRef State) const; + + LLVM_NODISCARD + ProgramStateRef FreeMemParamAttr(CheckerContext &C, const CallEvent &Call, + const OwnershipAttr *Att, + ProgramStateRef State) const; /// Models memory deallocation. /// @@ -1080,7 +1084,7 @@ ReallocatingMemFnMap.lookup(Call)) return true; - if (!ShouldIncludeOwnershipAnnotatedFunctions) + if (!IsOptimisticAnalyzerOptionEnabled) return false; const auto *Func = dyn_cast(Call.getDecl()); @@ -1396,17 +1400,35 @@ C.addTransition(State); } -void MallocChecker::checkOwnershipAttr(const CallEvent &Call, - CheckerContext &C) const { - ProgramStateRef State = C.getState(); +static const FunctionDecl *functionDeclForCall(const CallEvent &Call, + CheckerContext &C) { const auto *CE = dyn_cast_or_null(Call.getOriginExpr()); if (!CE) - return; + return nullptr; + const FunctionDecl *FD = C.getCalleeDecl(CE); if (!FD) - return; - if (ShouldIncludeOwnershipAnnotatedFunctions || - ChecksEnabled[CK_MismatchedDeallocatorChecker]) { + return nullptr; + + return FD; +} + +bool MallocChecker::isOwnershipAttrSupportEnabled() const { + bool result = IsOptimisticAnalyzerOptionEnabled || + ChecksEnabled[CK_MismatchedDeallocatorChecker]; + + return result; +} + +void MallocChecker::checkOwnershipAttr(const CallEvent &Call, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + const FunctionDecl *FD = functionDeclForCall(Call, C); + if (!FD) { + return; + } + + if (isOwnershipAttrSupportEnabled()) { // Check all the attributes, if there are any. // There can be multiple of these attributes. if (FD->hasAttrs()) @@ -1425,6 +1447,37 @@ C.addTransition(State); } +void MallocChecker::checkOwnershipParamAttr(const CallEvent &Call, + CheckerContext &C) const { + if (!isOwnershipAttrSupportEnabled()) + return; + + ProgramStateRef State = C.getState(); + const FunctionDecl *FD = functionDeclForCall(Call, C); + if (!FD) + return; + + for (const auto *P : FD->parameters()) { + if (!P->hasAttr()) + continue; + + unsigned int Index = P->getFunctionScopeIndex(); + bool IsParamInBounds = Index >= 0 && Index < Call.getNumArgs(); + if (!IsParamInBounds) + continue; + + bool IsKnownToBeAllocated = false; + bool Holds = false; + + ProgramStateRef StateP = FreeMemAux(C, Call, State, Index, Holds, + IsKnownToBeAllocated, AF_Malloc); + if (StateP != nullptr) + State = StateP; + } + + C.addTransition(State); +} + void MallocChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { if (C.wasInlined) @@ -1455,6 +1508,7 @@ } checkOwnershipAttr(Call, C); + checkOwnershipParamAttr(Call, C); } // Performs a 0-sized allocations check. @@ -3562,7 +3616,7 @@ void ento::registerDynamicMemoryModeling(CheckerManager &mgr) { auto *checker = mgr.registerChecker(); - checker->ShouldIncludeOwnershipAnnotatedFunctions = + checker->IsOptimisticAnalyzerOptionEnabled = mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "Optimistic"); checker->ShouldRegisterNoOwnershipChangeVisitor = mgr.getAnalyzerOptions().getCheckerBooleanOption( diff --git a/clang/test/Analysis/malloc-annotations.c b/clang/test/Analysis/malloc-annotations.c --- a/clang/test/Analysis/malloc-annotations.c +++ b/clang/test/Analysis/malloc-annotations.c @@ -16,6 +16,10 @@ __attribute((ownership_holds(malloc, 1, 2))); void __attribute((ownership_returns(malloc, 1))) *my_malloc2(size_t); void __attribute((ownership_holds(malloc, 1))) my_hold(void *); +void my_free_first_parameter(void * __attribute__((ownership_takes_param))); +void my_free_second_parameter(void *, void * __attribute__((ownership_takes_param))); +void my_free_two_parameters(void * __attribute__((ownership_takes_param)), void * __attribute__((ownership_takes_param))); +void __attribute((ownership_takes(malloc, 1))) my_free_both_params_via_param_and_func(void *, void * __attribute__((ownership_takes_param))); // Duplicate attributes are silly, but not an error. // Duplicate attribute has no extra effect. @@ -273,3 +277,49 @@ my_freeBoth(p, q); } +int *testFreeFirstParameterUseAfterFree() { + int *p = malloc(4); + + my_free_first_parameter(p); + return p; // expected-warning{{Use of memory after it is freed}} +} + +int *testFreeSecondParameterUseAfterFree() { + int *p1 = malloc(4); + int *p2 = malloc(4); + + my_free_second_parameter(p1, p2); + return p2; // expected-warning{{Use of memory after it is freed}} +} + +int *testFreeSecondParameterNoUseAfterFree() { + int *p1 = malloc(4); + int *p2 = malloc(4); + + my_free_second_parameter(p1, p2); + return p1; +} + +int *testFreeFirstOfTwoParametersUseAfterFree() { + int *p1 = malloc(4); + int *p2 = malloc(4); + + my_free_two_parameters(p1, p2); + return p1; // expected-warning{{Use of memory after it is freed}} +} + +int *testFreeSecondOfTwoParametersUseAfterFree() { + int *p1 = malloc(4); + int *p2 = malloc(4); + + my_free_two_parameters(p1, p2); + return p2; // expected-warning{{Use of memory after it is freed}} +} + +int *testAuthorMayUseFuncDeclAndParamOwnershipAttr() { + int *p1 = malloc(4); + int *p2 = malloc(4); + + my_free_both_params_via_param_and_func(p1, p2); + return p2; // expected-warning{{Use of memory after it is freed}} +} \ No newline at end of file