From c1746eec0e985bb394ecd604129cd0c30d5c66ca Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Wed, 17 Jul 2024 16:41:20 +0300 Subject: [PATCH 1/9] clang/sema: disallow more than one 'onweship_takes' with different classes --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++++ clang/lib/Sema/SemaDeclAttr.cpp | 10 ++++++++++ clang/test/Sema/attr-ownership.c | 4 ++++ 3 files changed, 18 insertions(+) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b8d97a6b14fe6b..bdb7fadda7743f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3331,6 +3331,10 @@ def err_ownership_returns_index_mismatch : Error< "'ownership_returns' attribute index does not match; here it is %0">; def note_ownership_returns_index_mismatch : Note< "declared with index %0 here">; +def err_ownership_takes_class_mismatch : Error< + "'ownership_takes' attribute class does not match; here it is '%0'">; +def note_ownership_takes_class_mismatch : Note< + "declared with class '%0' here">; def err_format_strftime_third_parameter : Error< "strftime format attribute requires 3rd parameter to be 0">; def err_format_attribute_not : Error<"format argument not a string type">; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 5fd8622c90dd8e..39675422e3f9fd 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -1537,6 +1537,16 @@ static void handleOwnershipAttr(Sema &S, Decl *D, const ParsedAttr &AL) { << Idx.getSourceIndex() << Ex->getSourceRange(); return; } + } else if (K == OwnershipAttr::Takes && + I->getOwnKind() == OwnershipAttr::Takes) { + if (I->getModule()->getName() != ModuleName) { + S.Diag(I->getLocation(), diag::err_ownership_takes_class_mismatch) + << I->getModule()->getName(); + S.Diag(AL.getLoc(), diag::note_ownership_takes_class_mismatch) + << ModuleName << Ex->getSourceRange(); + + return; + } } } OwnershipArgs.push_back(Idx); diff --git a/clang/test/Sema/attr-ownership.c b/clang/test/Sema/attr-ownership.c index 8157ba7145a24d..084624353315ca 100644 --- a/clang/test/Sema/attr-ownership.c +++ b/clang/test/Sema/attr-ownership.c @@ -24,3 +24,7 @@ void f15(int, int) void f16(int *i, int *j) __attribute__((ownership_holds(foo, 1))) __attribute__((ownership_holds(foo, 1))); // OK, same index void f17(void*) __attribute__((ownership_takes(__, 1))); void f18() __attribute__((ownership_takes(foo, 1))); // expected-warning {{'ownership_takes' attribute only applies to non-K&R-style functions}} + +int f19(void *) + __attribute__((ownership_takes(foo, 1))) // expected-error {{'ownership_takes' attribute class does not match; here it is 'foo'}} + __attribute__((ownership_takes(foo1, 1))); // expected-note {{declared with class 'foo1' here}} From 47793876e74c5773ae6c8464fddb95c760d84507 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Wed, 17 Jul 2024 18:38:28 +0300 Subject: [PATCH 2/9] csa/MallocChecker: turn llvm_unreachable into asserts --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index fe202c79ed6209..8ff71318073ce8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1293,7 +1293,8 @@ void MallocChecker::checkCXXNewOrCXXDelete(const CallEvent &Call, AF_CXXNewArray); break; default: - llvm_unreachable("not a new/delete operator"); + assert(false && "not a new/delete operator"); + return; } C.addTransition(State); @@ -1489,8 +1490,10 @@ ProgramStateRef MallocChecker::ProcessZeroAllocCheck( } else { return State; } - } else - llvm_unreachable("not a CallExpr or CXXNewExpr"); + } else { + assert(false && "not a CallExpr or CXXNewExpr"); + return nullptr; + } assert(Arg); @@ -1925,7 +1928,7 @@ static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) { case AF_IfNameIndex: os << "'if_nameindex()'"; return; case AF_InnerBuffer: os << "container-specific allocator"; return; case AF_Alloca: - case AF_None: llvm_unreachable("not a deallocation expression"); + case AF_None: assert(false && "not a deallocation expression"); } } @@ -1937,7 +1940,7 @@ static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) { case AF_IfNameIndex: os << "'if_freenameindex()'"; return; case AF_InnerBuffer: os << "container-specific deallocator"; return; case AF_Alloca: - case AF_None: llvm_unreachable("suspicious argument"); + case AF_None: assert(false && "suspicious argument"); } } @@ -2145,10 +2148,12 @@ MallocChecker::getCheckIfTracked(AllocationFamily Family, return std::nullopt; } case AF_None: { - llvm_unreachable("no family"); + assert(false && "no family"); + return std::nullopt; } } - llvm_unreachable("unhandled family"); + assert(false && "unhandled family"); + return std::nullopt; } std::optional @@ -3528,7 +3533,8 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N, break; } case AF_None: - llvm_unreachable("Unhandled allocation family!"); + assert(false && "Unhandled allocation family!"); + return nullptr; } // See if we're releasing memory while inlining a destructor From c33b9ea439be0a0e10e0adc06f50820add764e5a Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Wed, 17 Jul 2024 18:43:50 +0300 Subject: [PATCH 3/9] csa/MallocChecker: add support for custom allocation classes Add support for custom allocation classes that could be specified by ownership_returns attribute. This patch adds basic support, so new class is not contructed anywhere and handled as an error everywhere. --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 295 +++++++++++------- 1 file changed, 180 insertions(+), 115 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 8ff71318073ce8..d6db1d2f0649c4 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -103,14 +103,41 @@ using namespace std::placeholders; namespace { // Used to check correspondence between allocators and deallocators. -enum AllocationFamily { +enum AllocationFamilyKind { AF_None, AF_Malloc, AF_CXXNew, AF_CXXNewArray, AF_IfNameIndex, AF_Alloca, - AF_InnerBuffer + AF_InnerBuffer, + AF_Custom, +}; + +struct AllocationFamily { + AllocationFamilyKind Kind; + std::optional CustomName; + + explicit AllocationFamily(AllocationFamilyKind kind, + std::optional name = std::nullopt) + : Kind(kind), CustomName(name) { + assert(kind != AF_Custom || name != std::nullopt); + } + + bool operator==(const AllocationFamily &Other) const { + return std::tie(Kind, CustomName) == std::tie(Other.Kind, Other.CustomName); + } + + bool operator!=(const AllocationFamily &Other) const { + return !(*this == Other); + } + + void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddInteger(Kind); + + if (Kind == AF_Custom) + ID.AddString(CustomName.value()); + } }; } // end of anonymous namespace @@ -158,7 +185,7 @@ class RefState { RefState(Kind k, const Stmt *s, AllocationFamily family) : S(s), K(k), Family(family) { - assert(family != AF_None); + assert(family.Kind != AF_None); } public: @@ -194,7 +221,7 @@ class RefState { void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); ID.AddPointer(S); - ID.AddInteger(Family); + Family.Profile(ID); } LLVM_DUMP_METHOD void dump(raw_ostream &OS) const { @@ -899,7 +926,7 @@ class MallocBugVisitor final : public BugReporterVisitor { bool IsReleased = (RSCurr && RSCurr->isReleased()) && (!RSPrev || !RSPrev->isReleased()); assert(!IsReleased || (isa_and_nonnull(Stmt)) || - (!Stmt && RSCurr->getAllocationFamily() == AF_InnerBuffer)); + (!Stmt && RSCurr->getAllocationFamily().Kind == AF_InnerBuffer)); return IsReleased; } @@ -1122,7 +1149,7 @@ MallocChecker::performKernelMalloc(const CallEvent &Call, CheckerContext &C, if (TrueState && !FalseState) { SVal ZeroVal = C.getSValBuilder().makeZeroVal(Ctx.CharTy); return MallocMemAux(C, Call, Call.getArgExpr(0), ZeroVal, TrueState, - AF_Malloc); + AllocationFamily(AF_Malloc)); } return std::nullopt; @@ -1143,7 +1170,7 @@ void MallocChecker::checkBasicAlloc(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State, - AF_Malloc); + AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(Call, 0, State); C.addTransition(State); } @@ -1157,7 +1184,7 @@ void MallocChecker::checkKernelMalloc(const CallEvent &Call, State = *MaybeState; else State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State, - AF_Malloc); + AllocationFamily(AF_Malloc)); C.addTransition(State); } @@ -1194,7 +1221,8 @@ void MallocChecker::checkRealloc(const CallEvent &Call, CheckerContext &C, return; ProgramStateRef State = C.getState(); - State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, AF_Malloc); + State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, + AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(Call, 1, State); C.addTransition(State); } @@ -1214,7 +1242,7 @@ void MallocChecker::checkFree(const CallEvent &Call, CheckerContext &C) const { if (suppressDeallocationsInSuspiciousContexts(Call, C)) return; State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory, - AF_Malloc); + AllocationFamily(AF_Malloc)); C.addTransition(State); } @@ -1222,7 +1250,7 @@ void MallocChecker::checkAlloca(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State, - AF_Alloca); + AllocationFamily(AF_Alloca)); State = ProcessZeroAllocCheck(Call, 0, State); C.addTransition(State); } @@ -1233,7 +1261,7 @@ void MallocChecker::checkStrdup(const CallEvent &Call, const auto *CE = dyn_cast_or_null(Call.getOriginExpr()); if (!CE) return; - State = MallocUpdateRefState(C, CE, State, AF_Malloc); + State = MallocUpdateRefState(C, CE, State, AllocationFamily(AF_Malloc)); C.addTransition(State); } @@ -1243,8 +1271,8 @@ void MallocChecker::checkIfNameIndex(const CallEvent &Call, ProgramStateRef State = C.getState(); // 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, AF_IfNameIndex); + State = MallocMemAux(C, Call, UnknownVal(), UnknownVal(), State, + AllocationFamily(AF_IfNameIndex)); C.addTransition(State); } @@ -1254,7 +1282,7 @@ void MallocChecker::checkIfFreeNameIndex(const CallEvent &Call, ProgramStateRef State = C.getState(); bool IsKnownToBeAllocatedMemory = false; State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory, - AF_IfNameIndex); + AllocationFamily(AF_IfNameIndex)); C.addTransition(State); } @@ -1275,22 +1303,22 @@ void MallocChecker::checkCXXNewOrCXXDelete(const CallEvent &Call, const FunctionDecl *FD = C.getCalleeDecl(CE); switch (FD->getOverloadedOperator()) { case OO_New: - State = - MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State, AF_CXXNew); + State = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State, + AllocationFamily(AF_CXXNew)); State = ProcessZeroAllocCheck(Call, 0, State); break; case OO_Array_New: State = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State, - AF_CXXNewArray); + AllocationFamily(AF_CXXNewArray)); State = ProcessZeroAllocCheck(Call, 0, State); break; case OO_Delete: State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory, - AF_CXXNew); + AllocationFamily(AF_CXXNew)); break; case OO_Array_Delete: State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory, - AF_CXXNewArray); + AllocationFamily(AF_CXXNewArray)); break; default: assert(false && "not a new/delete operator"); @@ -1305,7 +1333,8 @@ void MallocChecker::checkGMalloc0(const CallEvent &Call, ProgramStateRef State = C.getState(); SValBuilder &svalBuilder = C.getSValBuilder(); SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy); - State = MallocMemAux(C, Call, Call.getArgExpr(0), zeroVal, State, AF_Malloc); + State = MallocMemAux(C, Call, Call.getArgExpr(0), zeroVal, State, + AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(Call, 0, State); C.addTransition(State); } @@ -1313,8 +1342,8 @@ void MallocChecker::checkGMalloc0(const CallEvent &Call, void MallocChecker::checkGMemdup(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); - State = - MallocMemAux(C, Call, Call.getArgExpr(1), UnknownVal(), State, AF_Malloc); + State = MallocMemAux(C, Call, Call.getArgExpr(1), UnknownVal(), State, + AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(Call, 1, State); C.addTransition(State); } @@ -1324,7 +1353,8 @@ void MallocChecker::checkGMallocN(const CallEvent &Call, ProgramStateRef State = C.getState(); SVal Init = UndefinedVal(); SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1)); - State = MallocMemAux(C, Call, TotalSize, Init, State, AF_Malloc); + State = MallocMemAux(C, Call, TotalSize, Init, State, + AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(Call, 0, State); State = ProcessZeroAllocCheck(Call, 1, State); C.addTransition(State); @@ -1336,7 +1366,8 @@ void MallocChecker::checkGMallocN0(const CallEvent &Call, SValBuilder &SB = C.getSValBuilder(); SVal Init = SB.makeZeroVal(SB.getContext().CharTy); SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1)); - State = MallocMemAux(C, Call, TotalSize, Init, State, AF_Malloc); + State = MallocMemAux(C, Call, TotalSize, Init, State, + AllocationFamily(AF_Malloc)); State = ProcessZeroAllocCheck(Call, 0, State); State = ProcessZeroAllocCheck(Call, 1, State); C.addTransition(State); @@ -1366,7 +1397,8 @@ void MallocChecker::preGetdelim(const CallEvent &Call, // of reporting any violation of the preconditions. bool IsKnownToBeAllocated = false; State = FreeMemAux(C, Call.getArgExpr(0), Call, State, false, - IsKnownToBeAllocated, AF_Malloc, false, LinePtr); + IsKnownToBeAllocated, AllocationFamily(AF_Malloc), false, + LinePtr); if (State) C.addTransition(State); } @@ -1395,13 +1427,15 @@ void MallocChecker::checkGetdelim(const CallEvent &Call, return; State = setDynamicExtent(State, LinePtr->getAsRegion(), *Size, SVB); - C.addTransition(MallocUpdateRefState(C, CE, State, AF_Malloc, *LinePtr)); + C.addTransition(MallocUpdateRefState(C, CE, State, + AllocationFamily(AF_Malloc), *LinePtr)); } void MallocChecker::checkReallocN(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); - State = ReallocMemAux(C, Call, /*ShouldFreeOnFail=*/false, State, AF_Malloc, + State = ReallocMemAux(C, Call, /*ShouldFreeOnFail=*/false, State, + AllocationFamily(AF_Malloc), /*SuffixWithN=*/true); State = ProcessZeroAllocCheck(Call, 1, State); State = ProcessZeroAllocCheck(Call, 2, State); @@ -1601,7 +1635,8 @@ MallocChecker::processNewAllocation(const CXXAllocatorCall &Call, SVal Target = Call.getObjectUnderConstruction(); if (Call.getOriginExpr()->isArray()) { if (auto SizeEx = NE->getArraySize()) - checkTaintedness(C, Call, C.getSVal(*SizeEx), State, AF_CXXNewArray); + checkTaintedness(C, Call, C.getSVal(*SizeEx), State, + AllocationFamily(AF_CXXNewArray)); } State = MallocUpdateRefState(C, NE, State, Family, Target); @@ -1614,7 +1649,8 @@ void MallocChecker::checkNewAllocator(const CXXAllocatorCall &Call, if (!C.wasInlined) { ProgramStateRef State = processNewAllocation( Call, C, - (Call.getOriginExpr()->isArray() ? AF_CXXNewArray : AF_CXXNew)); + AllocationFamily(Call.getOriginExpr()->isArray() ? AF_CXXNewArray + : AF_CXXNew)); C.addTransition(State); } } @@ -1658,10 +1694,10 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call, return; bool IsKnownToBeAllocatedMemory; - ProgramStateRef State = - FreeMemAux(C, Call.getArgExpr(0), Call, C.getState(), - /*Hold=*/true, IsKnownToBeAllocatedMemory, AF_Malloc, - /*ReturnsNullOnFailure=*/true); + ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(0), Call, C.getState(), + /*Hold=*/true, IsKnownToBeAllocatedMemory, + AllocationFamily(AF_Malloc), + /*ReturnsNullOnFailure=*/true); C.addTransition(State); } @@ -1679,9 +1715,10 @@ MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call, if (!Att->args().empty()) { return MallocMemAux(C, Call, Call.getArgExpr(Att->args_begin()->getASTIndex()), - UndefinedVal(), State, AF_Malloc); + UndefinedVal(), State, AllocationFamily(AF_Malloc)); } - return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, AF_Malloc); + return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, + AllocationFamily(AF_Malloc)); } ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, @@ -1771,10 +1808,10 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, unsigned Count = C.blockCount(); SValBuilder &SVB = C.getSValBuilder(); const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); - DefinedSVal RetVal = - ((Family == AF_Alloca) ? SVB.getAllocaRegionVal(CE, LCtx, Count) - : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count) - .castAs()); + DefinedSVal RetVal = ((Family.Kind == AF_Alloca) + ? SVB.getAllocaRegionVal(CE, LCtx, Count) + : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count) + .castAs()); State = State->BindExpr(CE, C.getLocationContext(), RetVal); // Fill the region with the initialization value. @@ -1784,7 +1821,7 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, if (Size.isUndef()) Size = UnknownVal(); - checkTaintedness(C, Call, Size, State, AF_Malloc); + checkTaintedness(C, Call, Size, State, AllocationFamily(AF_Malloc)); // Set the region's extent. State = setDynamicExtent(State, RetVal.getAsRegion(), @@ -1842,7 +1879,7 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, ProgramStateRef StateI = FreeMemAux(C, Call, State, Arg.getASTIndex(), Att->getOwnKind() == OwnershipAttr::Holds, - IsKnownToBeAllocated, AF_Malloc); + IsKnownToBeAllocated, AllocationFamily(AF_Malloc)); if (StateI) State = StateI; } @@ -1921,26 +1958,50 @@ static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E) { static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) { - switch(Family) { - case AF_Malloc: os << "malloc()"; return; - case AF_CXXNew: os << "'new'"; return; - case AF_CXXNewArray: os << "'new[]'"; return; - case AF_IfNameIndex: os << "'if_nameindex()'"; return; - case AF_InnerBuffer: os << "container-specific allocator"; return; - case AF_Alloca: - case AF_None: assert(false && "not a deallocation expression"); + switch (Family.Kind) { + case AF_Malloc: + os << "malloc()"; + return; + case AF_CXXNew: + os << "'new'"; + return; + case AF_CXXNewArray: + os << "'new[]'"; + return; + case AF_IfNameIndex: + os << "'if_nameindex()'"; + return; + case AF_InnerBuffer: + os << "container-specific allocator"; + return; + case AF_Alloca: + case AF_Custom: + case AF_None: + assert(false && "not a deallocation expression"); } } static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) { - switch(Family) { - case AF_Malloc: os << "free()"; return; - case AF_CXXNew: os << "'delete'"; return; - case AF_CXXNewArray: os << "'delete[]'"; return; - case AF_IfNameIndex: os << "'if_freenameindex()'"; return; - case AF_InnerBuffer: os << "container-specific deallocator"; return; - case AF_Alloca: - case AF_None: assert(false && "suspicious argument"); + switch (Family.Kind) { + case AF_Malloc: + os << "free()"; + return; + case AF_CXXNew: + os << "'delete'"; + return; + case AF_CXXNewArray: + os << "'delete[]'"; + return; + case AF_IfNameIndex: + os << "'if_freenameindex()'"; + return; + case AF_InnerBuffer: + os << "container-specific deallocator"; + return; + case AF_Alloca: + case AF_Custom: + case AF_None: + assert(false && "not a deallocation expression"); } } @@ -1994,7 +2055,7 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr, // code. In that case, the ZERO_SIZE_PTR defines a special value used for a // zero-sized memory block which is allowed to be freed, despite not being a // null pointer. - if (Family != AF_Malloc || !isArgZERO_SIZE_PTR(State, C, ArgVal)) + if (Family.Kind != AF_Malloc || !isArgZERO_SIZE_PTR(State, C, ArgVal)) HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family); return nullptr; @@ -2044,7 +2105,7 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr, if (RsBase) { // Memory returned by alloca() shouldn't be freed. - if (RsBase->getAllocationFamily() == AF_Alloca) { + if (RsBase->getAllocationFamily().Kind == AF_Alloca) { HandleFreeAlloca(C, ArgVal, ArgExpr->getSourceRange()); return nullptr; } @@ -2122,7 +2183,7 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr, std::optional MallocChecker::getCheckIfTracked(AllocationFamily Family, bool IsALeakCheck) const { - switch (Family) { + switch (Family.Kind) { case AF_Malloc: case AF_Alloca: case AF_IfNameIndex: { @@ -2147,6 +2208,7 @@ MallocChecker::getCheckIfTracked(AllocationFamily Family, return CK_InnerPointerChecker; return std::nullopt; } + case AF_Custom: case AF_None: { assert(false && "no family"); return std::nullopt; @@ -2470,7 +2532,7 @@ void MallocChecker::HandleUseAfterFree(CheckerContext &C, SourceRange Range, auto R = std::make_unique( *BT_UseFree[*CheckKind], - AF == AF_InnerBuffer + AF.Kind == AF_InnerBuffer ? "Inner pointer of container used after re/deallocation" : "Use of memory after it is freed", N); @@ -2479,7 +2541,7 @@ void MallocChecker::HandleUseAfterFree(CheckerContext &C, SourceRange Range, R->addRange(Range); R->addVisitor(Sym); - if (AF == AF_InnerBuffer) + if (AF.Kind == AF_InnerBuffer) R->addVisitor(allocation_state::getInnerPointerBRVisitor(Sym)); C.emitReport(std::move(R)); @@ -2738,7 +2800,8 @@ ProgramStateRef MallocChecker::CallocMem(CheckerContext &C, SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1)); - return MallocMemAux(C, Call, TotalSize, zeroVal, State, AF_Malloc); + return MallocMemAux(C, Call, TotalSize, zeroVal, State, + AllocationFamily(AF_Malloc)); } MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N, @@ -2793,7 +2856,7 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N, assert(RS && "cannot leak an untracked symbol"); AllocationFamily Family = RS->getAllocationFamily(); - if (Family == AF_Alloca) + if (Family.Kind == AF_Alloca) return; std::optional CheckKind = @@ -2920,9 +2983,10 @@ void MallocChecker::checkPreCall(const CallEvent &Call, ProgramStateRef State = C.getState(); bool IsKnownToBeAllocated; - State = FreeMemAux(C, DE->getArgument(), Call, State, - /*Hold*/ false, IsKnownToBeAllocated, - (DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew)); + State = FreeMemAux( + C, DE->getArgument(), Call, State, + /*Hold*/ false, IsKnownToBeAllocated, + AllocationFamily(DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew)); C.addTransition(State); return; @@ -3354,8 +3418,8 @@ ProgramStateRef MallocChecker::checkConstPointerEscape(ProgramStateRef State, } static bool checkIfNewOrNewArrayFamily(const RefState *RS) { - return (RS->getAllocationFamily() == AF_CXXNewArray || - RS->getAllocationFamily() == AF_CXXNew); + return (RS->getAllocationFamily().Kind == AF_CXXNewArray || + RS->getAllocationFamily().Kind == AF_CXXNew); } ProgramStateRef MallocChecker::checkPointerEscapeAux( @@ -3436,7 +3500,7 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N, const Stmt *S = N->getStmtForDiagnostics(); // When dealing with containers, we sometimes want to give a note // even if the statement is missing. - if (!S && (!RSCurr || RSCurr->getAllocationFamily() != AF_InnerBuffer)) + if (!S && (!RSCurr || RSCurr->getAllocationFamily().Kind != AF_InnerBuffer)) return nullptr; const LocationContext *CurrentLC = N->getLocationContext(); @@ -3488,54 +3552,55 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N, Sym, "Returned allocated memory"); } else if (isReleased(RSCurr, RSPrev, S)) { const auto Family = RSCurr->getAllocationFamily(); - switch (Family) { - case AF_Alloca: - case AF_Malloc: - case AF_CXXNew: - case AF_CXXNewArray: - case AF_IfNameIndex: - Msg = "Memory is released"; + switch (Family.Kind) { + case AF_Alloca: + case AF_Malloc: + case AF_CXXNew: + case AF_CXXNewArray: + case AF_IfNameIndex: + Msg = "Memory is released"; + StackHint = std::make_unique( + Sym, "Returning; memory was released"); + break; + case AF_InnerBuffer: { + const MemRegion *ObjRegion = + allocation_state::getContainerObjRegion(statePrev, Sym); + const auto *TypedRegion = cast(ObjRegion); + QualType ObjTy = TypedRegion->getValueType(); + OS << "Inner buffer of '" << ObjTy << "' "; + + if (N->getLocation().getKind() == ProgramPoint::PostImplicitCallKind) { + OS << "deallocated by call to destructor"; StackHint = std::make_unique( - Sym, "Returning; memory was released"); - break; - case AF_InnerBuffer: { - const MemRegion *ObjRegion = - allocation_state::getContainerObjRegion(statePrev, Sym); - const auto *TypedRegion = cast(ObjRegion); - QualType ObjTy = TypedRegion->getValueType(); - OS << "Inner buffer of '" << ObjTy << "' "; - - if (N->getLocation().getKind() == ProgramPoint::PostImplicitCallKind) { - OS << "deallocated by call to destructor"; - StackHint = std::make_unique( - Sym, "Returning; inner buffer was deallocated"); - } else { - OS << "reallocated by call to '"; - const Stmt *S = RSCurr->getStmt(); - if (const auto *MemCallE = dyn_cast(S)) { - OS << MemCallE->getMethodDecl()->getDeclName(); - } else if (const auto *OpCallE = dyn_cast(S)) { - OS << OpCallE->getDirectCallee()->getDeclName(); - } else if (const auto *CallE = dyn_cast(S)) { - auto &CEMgr = BRC.getStateManager().getCallEventManager(); - CallEventRef<> Call = - CEMgr.getSimpleCall(CallE, state, CurrentLC, {nullptr, 0}); - if (const auto *D = dyn_cast_or_null(Call->getDecl())) - OS << D->getDeclName(); - else - OS << "unknown"; - } - OS << "'"; - StackHint = std::make_unique( - Sym, "Returning; inner buffer was reallocated"); + Sym, "Returning; inner buffer was deallocated"); + } else { + OS << "reallocated by call to '"; + const Stmt *S = RSCurr->getStmt(); + if (const auto *MemCallE = dyn_cast(S)) { + OS << MemCallE->getMethodDecl()->getDeclName(); + } else if (const auto *OpCallE = dyn_cast(S)) { + OS << OpCallE->getDirectCallee()->getDeclName(); + } else if (const auto *CallE = dyn_cast(S)) { + auto &CEMgr = BRC.getStateManager().getCallEventManager(); + CallEventRef<> Call = + CEMgr.getSimpleCall(CallE, state, CurrentLC, {nullptr, 0}); + if (const auto *D = dyn_cast_or_null(Call->getDecl())) + OS << D->getDeclName(); + else + OS << "unknown"; } - Msg = OS.str(); - break; + OS << "'"; + StackHint = std::make_unique( + Sym, "Returning; inner buffer was reallocated"); + } + Msg = OS.str(); + break; } + case AF_Custom: case AF_None: assert(false && "Unhandled allocation family!"); return nullptr; - } + } // See if we're releasing memory while inlining a destructor // (or one of its callees). This turns on various common @@ -3609,7 +3674,7 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N, // Generate the extra diagnostic. PathDiagnosticLocation Pos; if (!S) { - assert(RSCurr->getAllocationFamily() == AF_InnerBuffer); + assert(RSCurr->getAllocationFamily().Kind == AF_InnerBuffer); auto PostImplCall = N->getLocation().getAs(); if (!PostImplCall) return nullptr; @@ -3656,7 +3721,7 @@ namespace allocation_state { ProgramStateRef markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) { - AllocationFamily Family = AF_InnerBuffer; + AllocationFamily Family(AF_InnerBuffer); return State->set(Sym, RefState::getReleased(Family, Origin)); } From 13cfbc949e2d177566c3cb7f781036708e1bf04a Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Wed, 17 Jul 2024 18:49:13 +0300 Subject: [PATCH 4/9] csa/MismatchedDeallocator: handle custom allocation clasess Patch introduces support for custom allocation classes in MismatchedDeallocator. Patch preserves previous behavior, in which 'malloc' class was handled as AF_Malloc type. --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 69 ++++++++++++++----- 1 file changed, 53 insertions(+), 16 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index d6db1d2f0649c4..b5209fc21ffaff 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -118,10 +118,17 @@ struct AllocationFamily { AllocationFamilyKind Kind; std::optional CustomName; - explicit AllocationFamily(AllocationFamilyKind kind, - std::optional name = std::nullopt) - : Kind(kind), CustomName(name) { - assert(kind != AF_Custom || name != std::nullopt); + explicit AllocationFamily(AllocationFamilyKind Kind, + std::optional Name = std::nullopt) + : Kind(Kind), CustomName(Name) { + assert((Kind != AF_Custom || Name.has_value()) && + "Custom family must specify also the name"); + + // Preseve previous behavior when "malloc" class means AF_Malloc + if (Kind == AF_Custom && CustomName.value() == "malloc") { + Kind = AF_Malloc; + CustomName = std::nullopt; + } } bool operator==(const AllocationFamily &Other) const { @@ -1709,16 +1716,15 @@ MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call, if (!State) return nullptr; - if (Att->getModule()->getName() != "malloc") - return nullptr; + auto attrClassName = Att->getModule()->getName(); + auto Family = AllocationFamily(AF_Custom, attrClassName); if (!Att->args().empty()) { return MallocMemAux(C, Call, Call.getArgExpr(Att->args_begin()->getASTIndex()), - UndefinedVal(), State, AllocationFamily(AF_Malloc)); + UndefinedVal(), State, Family); } - return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, - AllocationFamily(AF_Malloc)); + return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, Family); } ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, @@ -1870,8 +1876,8 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, if (!State) return nullptr; - if (Att->getModule()->getName() != "malloc") - return nullptr; + auto attrClassName = Att->getModule()->getName(); + auto Family = AllocationFamily(AF_Custom, attrClassName); bool IsKnownToBeAllocated = false; @@ -1879,7 +1885,7 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, ProgramStateRef StateI = FreeMemAux(C, Call, State, Arg.getASTIndex(), Att->getOwnKind() == OwnershipAttr::Holds, - IsKnownToBeAllocated, AllocationFamily(AF_Malloc)); + IsKnownToBeAllocated, Family); if (StateI) State = StateI; } @@ -1917,6 +1923,30 @@ static bool didPreviousFreeFail(ProgramStateRef State, return false; } +static void printOwnershipTakesList(raw_ostream &os, CheckerContext &C, + const Expr *E) { + if (const CallExpr *CE = dyn_cast(E)) { + const FunctionDecl *FD = CE->getDirectCallee(); + if (!FD) + return; + + if (!FD->hasAttrs()) + return; + + // Only one ownership_takes attribute is allowed + for (const auto *I : FD->specific_attrs()) { + OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind(); + + if (OwnKind != OwnershipAttr::Takes) + continue; + + os << ", which takes ownership of " << '\'' << I->getModule()->getName() + << '\''; + break; + } + } +} + static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E) { if (const CallExpr *CE = dyn_cast(E)) { // FIXME: This doesn't handle indirect calls. @@ -1974,8 +2004,10 @@ static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) { case AF_InnerBuffer: os << "container-specific allocator"; return; - case AF_Alloca: case AF_Custom: + os << Family.CustomName.value(); + return; + case AF_Alloca: case AF_None: assert(false && "not a deallocation expression"); } @@ -1998,8 +2030,11 @@ static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) { case AF_InnerBuffer: os << "container-specific deallocator"; return; - case AF_Alloca: case AF_Custom: + os << "function that takes ownership of '" << Family.CustomName.value() + << "\'"; + return; + case AF_Alloca: case AF_None: assert(false && "not a deallocation expression"); } @@ -2186,6 +2221,7 @@ MallocChecker::getCheckIfTracked(AllocationFamily Family, switch (Family.Kind) { case AF_Malloc: case AF_Alloca: + case AF_Custom: case AF_IfNameIndex: { if (ChecksEnabled[CK_MallocChecker]) return CK_MallocChecker; @@ -2208,7 +2244,6 @@ MallocChecker::getCheckIfTracked(AllocationFamily Family, return CK_InnerPointerChecker; return std::nullopt; } - case AF_Custom: case AF_None: { assert(false && "no family"); return std::nullopt; @@ -2440,6 +2475,8 @@ void MallocChecker::HandleMismatchedDealloc(CheckerContext &C, if (printMemFnName(DeallocOs, C, DeallocExpr)) os << ", not " << DeallocOs.str(); + + printOwnershipTakesList(os, C, DeallocExpr); } auto R = std::make_unique(*BT_MismatchedDealloc, @@ -3555,6 +3592,7 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N, switch (Family.Kind) { case AF_Alloca: case AF_Malloc: + case AF_Custom: case AF_CXXNew: case AF_CXXNewArray: case AF_IfNameIndex: @@ -3596,7 +3634,6 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N, Msg = OS.str(); break; } - case AF_Custom: case AF_None: assert(false && "Unhandled allocation family!"); return nullptr; From 7ff2b93f285944e9f5024897c57484d4c58a69c2 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Wed, 17 Jul 2024 16:52:41 +0300 Subject: [PATCH 5/9] csa/MallocChecker: quote funtion names in error output --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 13 +++-- .../expected-plists/plist-macros.cpp.plist | 6 +-- ...Malloc+MismatchedDeallocator+NewDelete.cpp | 10 ++-- .../MismatchedDeallocator-checker-test.mm | 36 +++++++------- .../test/Analysis/NewDelete-intersections.mm | 6 +-- clang/test/Analysis/free.c | 24 +++++----- clang/test/Analysis/free.cpp | 48 +++++++++---------- clang/test/Analysis/getline-alloc.c | 8 ++-- clang/test/Analysis/kmalloc-linux.c | 2 +- clang/test/Analysis/malloc-fnptr-plist.c | 2 +- clang/test/Analysis/malloc-std-namespace.cpp | 4 +- clang/test/Analysis/malloc.c | 36 +++++++------- clang/test/Analysis/malloc.mm | 2 +- clang/test/Analysis/plist-macros.cpp | 2 +- clang/test/Analysis/weak-functions.c | 2 +- 15 files changed, 102 insertions(+), 99 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index b5209fc21ffaff..0db0e622d2b0d2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1954,9 +1954,12 @@ static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E) { if (!FD) return false; - os << *FD; + os << '\'' << *FD; + if (!FD->isOverloadedOperator()) os << "()"; + + os << '\''; return true; } @@ -1990,7 +1993,7 @@ static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) { switch (Family.Kind) { case AF_Malloc: - os << "malloc()"; + os << "'malloc()'"; return; case AF_CXXNew: os << "'new'"; @@ -2016,7 +2019,7 @@ static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) { static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) { switch (Family.Kind) { case AF_Malloc: - os << "free()"; + os << "'free()'"; return; case AF_CXXNew: os << "'delete'"; @@ -2418,11 +2421,11 @@ void MallocChecker::HandleFreeAlloca(CheckerContext &C, SVal ArgVal, if (ExplodedNode *N = C.generateErrorNode()) { if (!BT_FreeAlloca[*CheckKind]) BT_FreeAlloca[*CheckKind].reset(new BugType( - CheckNames[*CheckKind], "Free alloca()", categories::MemoryError)); + CheckNames[*CheckKind], "Free 'alloca()'", categories::MemoryError)); auto R = std::make_unique( *BT_FreeAlloca[*CheckKind], - "Memory allocated by alloca() should not be deallocated", N); + "Memory allocated by 'alloca()' should not be deallocated", N); R->markInteresting(ArgVal.getAsRegion()); R->addRange(Range); C.emitReport(std::move(R)); diff --git a/clang/test/Analysis/Inputs/expected-plists/plist-macros.cpp.plist b/clang/test/Analysis/Inputs/expected-plists/plist-macros.cpp.plist index 9981fc6a1f5179..13645d47f8cdcb 100644 --- a/clang/test/Analysis/Inputs/expected-plists/plist-macros.cpp.plist +++ b/clang/test/Analysis/Inputs/expected-plists/plist-macros.cpp.plist @@ -130,12 +130,12 @@ depth0 extended_message - Memory allocated by malloc() should be deallocated by free(), not 'delete' + Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete' message - Memory allocated by malloc() should be deallocated by free(), not 'delete' + Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete' - descriptionMemory allocated by malloc() should be deallocated by free(), not 'delete' + descriptionMemory allocated by 'malloc()' should be deallocated by 'free()', not 'delete' categoryMemory error typeBad deallocator check_nameunix.MismatchedDeallocator diff --git a/clang/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp b/clang/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp index b5e47b3355da3d..6c20b4ba53dae7 100644 --- a/clang/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp +++ b/clang/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp @@ -24,12 +24,12 @@ void testMallocUseAfterFree() { void testMallocBadFree() { int i; - free(&i); // expected-warning{{Argument to free() is the address of the local variable 'i', which is not memory allocated by malloc()}} + free(&i); // expected-warning{{Argument to 'free()' is the address of the local variable 'i', which is not memory allocated by 'malloc()'}} } void testMallocOffsetFree() { int *p = (int *)malloc(sizeof(int)); - free(++p); // expected-warning{{Argument to free() is offset by 4 bytes from the start of memory allocated by malloc()}} + free(++p); // expected-warning{{Argument to 'free()' is offset by 4 bytes from the start of memory allocated by 'malloc()'}} } //----------------------------------------------------------------- @@ -37,7 +37,7 @@ void testMallocOffsetFree() { //----------------------------------------------------------------- void testMismatchedDeallocator() { int *x = (int *)malloc(sizeof(int)); - delete x; // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} + delete x; // expected-warning{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete'}} } //---------------------------------------------------------------- @@ -69,7 +69,7 @@ void testNewBadFree() { void testNewOffsetFree() { int *p = new int; - operator delete(++p); // expected-warning{{Argument to operator delete is offset by 4 bytes from the start of memory allocated by 'new'}} + operator delete(++p); // expected-warning{{Argument to 'operator delete' is offset by 4 bytes from the start of memory allocated by 'new'}} } //---------------------------------------------------------------- @@ -88,7 +88,7 @@ void testMismatchedChangePtrThroughCall() { void testMismatchedChangePointeeThroughCall() { int *p = (int*)malloc(sizeof(int)*4); changePointee(p); - delete p; // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} + delete p; // expected-warning{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete'}} } void testShouldReportDoubleFreeNotMismatched() { diff --git a/clang/test/Analysis/MismatchedDeallocator-checker-test.mm b/clang/test/Analysis/MismatchedDeallocator-checker-test.mm index 013d677e515cf1..bbf24837f9da82 100644 --- a/clang/test/Analysis/MismatchedDeallocator-checker-test.mm +++ b/clang/test/Analysis/MismatchedDeallocator-checker-test.mm @@ -21,79 +21,79 @@ //--------------- test malloc family void testMalloc1() { int *p = (int *)malloc(sizeof(int)); - delete p; // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} + delete p; // expected-warning{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete'}} } void testMalloc2() { int *p = (int *)malloc(8); int *q = (int *)realloc(p, 16); - delete q; // expected-warning{{Memory allocated by realloc() should be deallocated by free(), not 'delete'}} + delete q; // expected-warning{{Memory allocated by 'realloc()' should be deallocated by 'free()', not 'delete'}} } void testMalloc3() { int *p = (int *)calloc(1, sizeof(int)); - delete p; // expected-warning{{Memory allocated by calloc() should be deallocated by free(), not 'delete'}} + delete p; // expected-warning{{Memory allocated by 'calloc()' should be deallocated by 'free()', not 'delete'}} } void testMalloc4(const char *s) { char *p = strdup(s); - delete p; // expected-warning{{Memory allocated by strdup() should be deallocated by free(), not 'delete'}} + delete p; // expected-warning{{Memory allocated by 'strdup()' should be deallocated by 'free()', not 'delete'}} } void testMalloc5() { int *p = (int *)my_malloc(sizeof(int)); - delete p; // expected-warning{{Memory allocated by my_malloc() should be deallocated by free(), not 'delete'}} + delete p; // expected-warning{{Memory allocated by 'my_malloc()' should be deallocated by 'free()', not 'delete'}} } void testMalloc6() { int *p = (int *)malloc(sizeof(int)); - operator delete(p); // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not operator delete}} + operator delete(p); // expected-warning{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'operator delete'}} } void testMalloc7() { int *p = (int *)malloc(sizeof(int)); - delete[] p; // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not 'delete[]'}} + delete[] p; // expected-warning{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete[]'}} } void testMalloc8() { int *p = (int *)malloc(sizeof(int)); - operator delete[](p); // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not operator delete[]}} + operator delete[](p); // expected-warning{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'operator delete[]'}} } void testAlloca() { int *p = (int *)__builtin_alloca(sizeof(int)); - delete p; // expected-warning{{Memory allocated by alloca() should not be deallocated}} + delete p; // expected-warning{{Memory allocated by 'alloca()' should not be deallocated}} } //--------------- test new family void testNew1() { int *p = new int; - free(p); // expected-warning{{Memory allocated by 'new' should be deallocated by 'delete', not free()}} + free(p); // expected-warning{{Memory allocated by 'new' should be deallocated by 'delete', not 'free()'}} } void testNew2() { int *p = (int *)operator new(0); - free(p); // expected-warning{{Memory allocated by operator new should be deallocated by 'delete', not free()}} + free(p); // expected-warning{{Memory allocated by 'operator new' should be deallocated by 'delete', not 'free()'}} } void testNew3() { int *p = new int[1]; - free(p); // expected-warning{{Memory allocated by 'new[]' should be deallocated by 'delete[]', not free()}} + free(p); // expected-warning{{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'free()'}} } void testNew4() { int *p = new int; - realloc(p, sizeof(long)); // expected-warning{{Memory allocated by 'new' should be deallocated by 'delete', not realloc()}} + realloc(p, sizeof(long)); // expected-warning{{Memory allocated by 'new' should be deallocated by 'delete', not 'realloc()'}} } void testNew5() { int *p = (int *)operator new(0); - realloc(p, sizeof(long)); // expected-warning{{Memory allocated by operator new should be deallocated by 'delete', not realloc()}} + realloc(p, sizeof(long)); // expected-warning{{Memory allocated by 'operator new' should be deallocated by 'delete', not 'realloc()'}} } void testNew6() { int *p = new int[1]; - realloc(p, sizeof(long)); // expected-warning{{Memory allocated by 'new[]' should be deallocated by 'delete[]', not realloc()}} + realloc(p, sizeof(long)); // expected-warning{{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'realloc()'}} } int *allocInt() { @@ -106,7 +106,7 @@ void testNew7() { void testNew8() { int *p = (int *)operator new(0); - delete[] p; // expected-warning{{Memory allocated by operator new should be deallocated by 'delete', not 'delete[]'}} + delete[] p; // expected-warning{{Memory allocated by 'operator new' should be deallocated by 'delete', not 'delete[]'}} } int *allocIntArray(unsigned c) { @@ -120,7 +120,7 @@ void testNew9() { void testNew10() { int *p = (int *)operator new[](0); - delete p; // expected-warning{{Memory allocated by operator new[] should be deallocated by 'delete[]', not 'delete'}} + delete p; // expected-warning{{Memory allocated by 'operator new[]' should be deallocated by 'delete[]', not 'delete'}} } void testNew11(NSUInteger dataLength) { @@ -208,7 +208,7 @@ explicit SimpleSmartPointer(T *p = 0) : ptr(p) {} ~SimpleSmartPointer() { delete ptr; // expected-warning@-1 {{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'}} - // expected-warning@-2 {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} + // expected-warning@-2 {{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete'}} } }; diff --git a/clang/test/Analysis/NewDelete-intersections.mm b/clang/test/Analysis/NewDelete-intersections.mm index 6f81034ee349fd..9ac471600e8b9b 100644 --- a/clang/test/Analysis/NewDelete-intersections.mm +++ b/clang/test/Analysis/NewDelete-intersections.mm @@ -44,7 +44,7 @@ void testMallocFreeNoWarn() { void testDeleteMalloced() { int *p1 = (int *)malloc(sizeof(int)); delete p1; - // mismatch-warning@-1{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} + // mismatch-warning@-1{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete'}} int *p2 = (int *)__builtin_alloca(sizeof(int)); delete p2; // no warn @@ -59,13 +59,13 @@ void testUseZeroAllocatedMalloced() { void testFreeOpNew() { void *p = operator new(0); free(p); - // mismatch-warning@-1{{Memory allocated by operator new should be deallocated by 'delete', not free()}} + // mismatch-warning@-1{{Memory allocated by 'operator new' should be deallocated by 'delete', not 'free()'}} } void testFreeNewExpr() { int *p = new int; free(p); - // mismatch-warning@-1{{Memory allocated by 'new' should be deallocated by 'delete', not free()}} + // mismatch-warning@-1{{Memory allocated by 'new' should be deallocated by 'delete', not 'free()'}} free(p); } diff --git a/clang/test/Analysis/free.c b/clang/test/Analysis/free.c index 50c1efdfb1309c..5fd956a4ce110d 100644 --- a/clang/test/Analysis/free.c +++ b/clang/test/Analysis/free.c @@ -13,21 +13,21 @@ void *alloca(size_t); void t1 (void) { int a[] = { 1 }; free(a); - // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the local variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } void t2 (void) { int a = 1; free(&a); - // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the local variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } void t3 (void) { static int a[] = { 1 }; free(a); - // expected-warning@-1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the static variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } @@ -42,7 +42,7 @@ void t5 (void) { void t6 (void) { free((void*)1000); - // expected-warning@-1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is a constant address (1000), which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object '(void *)1000'}} } @@ -58,42 +58,42 @@ void t8 (char **x) { void t9 (void) { label: free(&&label); - // expected-warning@-1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the label 'label', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'label'}} } void t10 (void) { free((void*)&t10); - // expected-warning@-1{{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the function 't10', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 't10'}} } void t11 (void) { char *p = (char*)alloca(2); - free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void t12 (void) { char *p = (char*)__builtin_alloca(2); - free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void t13 (void) { free(^{return;}); - // expected-warning@-1{{Argument to free() is a block, which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is a block, which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object: block expression}} } void t14 (char a) { free(&a); - // expected-warning@-1{{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the parameter 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } static int someGlobal[2]; void t15 (void) { free(someGlobal); - // expected-warning@-1{{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the global variable 'someGlobal', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'someGlobal'}} } @@ -105,7 +105,7 @@ void t16 (char **x, int offset) { int *iptr(void); void t17(void) { free(iptr); // Oops, forgot to call iptr(). - // expected-warning@-1{{Argument to free() is the address of the function 'iptr', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the function 'iptr', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'iptr'}} } diff --git a/clang/test/Analysis/free.cpp b/clang/test/Analysis/free.cpp index a812a22c47d396..b7f2e49855cf12 100644 --- a/clang/test/Analysis/free.cpp +++ b/clang/test/Analysis/free.cpp @@ -17,42 +17,42 @@ extern "C" void *alloca(std::size_t); void t1a () { int a[] = { 1 }; free(a); - // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the local variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } void t1b () { int a[] = { 1 }; std::free(a); - // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the local variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}} } void t2a () { int a = 1; free(&a); - // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the local variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } void t2b () { int a = 1; std::free(&a); - // expected-warning@-1{{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the local variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}} } void t3a () { static int a[] = { 1 }; free(a); - // expected-warning@-1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the static variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } void t3b () { static int a[] = { 1 }; std::free(a); - // expected-warning@-1{{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the static variable 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}} } @@ -76,13 +76,13 @@ void t5b () { void t6a () { free((void*)1000); - // expected-warning@-1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is a constant address (1000), which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object '(void *)1000'}} } void t6b () { std::free((void*)1000); - // expected-warning@-1{{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is a constant address (1000), which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object '(void *)1000'}} } @@ -107,95 +107,95 @@ void t8b (char **x) { void t9a () { label: free(&&label); - // expected-warning@-1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the label 'label', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'label'}} } void t9b () { label: std::free(&&label); - // expected-warning@-1{{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the label 'label', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object 'label'}} } void t10a () { free((void*)&t10a); - // expected-warning@-1{{Argument to free() is the address of the function 't10a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the function 't10a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 't10a'}} } void t10b () { std::free((void*)&t10b); - // expected-warning@-1{{Argument to free() is the address of the function 't10b', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the function 't10b', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object 't10b'}} } void t11a () { char *p = (char*)alloca(2); - free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void t11b () { char *p = (char*)alloca(2); - std::free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + std::free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void t12a () { char *p = (char*)__builtin_alloca(2); - free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void t12b () { char *p = (char*)__builtin_alloca(2); - std::free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + std::free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void t13a () { free(^{return;}); - // expected-warning@-1{{Argument to free() is a block, which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is a block, which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object: block expression}} } void t13b () { std::free(^{return;}); - // expected-warning@-1{{Argument to free() is a block, which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is a block, which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object: block expression}} } void t14a () { free((void *)+[]{ return; }); - // expected-warning@-1{{Argument to free() is the address of the function '__invoke', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the function '__invoke', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object: lambda-to-function-pointer conversion}} } void t14b () { std::free((void *)+[]{ return; }); - // expected-warning@-1{{Argument to free() is the address of the function '__invoke', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the function '__invoke', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object: lambda-to-function-pointer conversion}} } void t15a (char a) { free(&a); - // expected-warning@-1{{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the parameter 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'a'}} } void t15b (char a) { std::free(&a); - // expected-warning@-1{{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the parameter 'a', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object 'a'}} } static int someGlobal[2]; void t16a () { free(someGlobal); - // expected-warning@-1{{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the global variable 'someGlobal', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 'someGlobal'}} } void t16b () { std::free(someGlobal); - // expected-warning@-1{{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the global variable 'someGlobal', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call std::free on non-heap object 'someGlobal'}} } diff --git a/clang/test/Analysis/getline-alloc.c b/clang/test/Analysis/getline-alloc.c index 5b5c716cb605a7..74a40a11b97828 100644 --- a/clang/test/Analysis/getline-alloc.c +++ b/clang/test/Analysis/getline-alloc.c @@ -40,7 +40,7 @@ void test_getline_alloca() { return; size_t n = 10; char *buffer = alloca(n); - getline(&buffer, &n, F1); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + getline(&buffer, &n, F1); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} fclose(F1); } @@ -50,7 +50,7 @@ void test_getline_invalid_ptr() { return; size_t n = 10; char *buffer = (char*)test_getline_invalid_ptr; - getline(&buffer, &n, F1); // expected-warning {{Argument to getline() is the address of the function 'test_getline_invalid_ptr', which is not memory allocated by malloc()}} + getline(&buffer, &n, F1); // expected-warning {{Argument to 'getline()' is the address of the function 'test_getline_invalid_ptr', which is not memory allocated by 'malloc()'}} fclose(F1); } @@ -79,7 +79,7 @@ void test_getline_stack() { if (!F1) return; - getline(&ptr, &n, F1); // expected-warning {{Argument to getline() is the address of the local variable 'buffer', which is not memory allocated by malloc()}} + getline(&ptr, &n, F1); // expected-warning {{Argument to 'getline()' is the address of the local variable 'buffer', which is not memory allocated by 'malloc()'}} } void test_getline_static() { @@ -91,5 +91,5 @@ void test_getline_static() { if (!F1) return; - getline(&ptr, &n, F1); // expected-warning {{Argument to getline() is the address of the static variable 'buffer', which is not memory allocated by malloc()}} + getline(&ptr, &n, F1); // expected-warning {{Argument to 'getline()' is the address of the static variable 'buffer', which is not memory allocated by 'malloc()'}} } diff --git a/clang/test/Analysis/kmalloc-linux.c b/clang/test/Analysis/kmalloc-linux.c index af1af24126b6a0..0114c4ef000e28 100644 --- a/clang/test/Analysis/kmalloc-linux.c +++ b/clang/test/Analysis/kmalloc-linux.c @@ -133,5 +133,5 @@ void test_kfree_ZERO_SIZE_PTR(void) { void test_kfree_other_constant_value(void) { void *ptr = (void *)1; - kfree(ptr); // expected-warning{{Argument to kfree() is a constant address (1)}} + kfree(ptr); // expected-warning{{Argument to 'kfree()' is a constant address (1)}} } diff --git a/clang/test/Analysis/malloc-fnptr-plist.c b/clang/test/Analysis/malloc-fnptr-plist.c index e3482980dfbf66..9e824064592344 100644 --- a/clang/test/Analysis/malloc-fnptr-plist.c +++ b/clang/test/Analysis/malloc-fnptr-plist.c @@ -5,7 +5,7 @@ void free(void *); void (*fnptr)(int); void foo(void) { free((void *)fnptr); - // expected-warning@-1{{Argument to free() is a function pointer}} + // expected-warning@-1{{Argument to 'free()' is a function pointer}} // expected-warning@-2{{attempt to call free on non-heap object '(void *)fnptr'}} } diff --git a/clang/test/Analysis/malloc-std-namespace.cpp b/clang/test/Analysis/malloc-std-namespace.cpp index d4e397bb812aa9..21566ad617920b 100644 --- a/clang/test/Analysis/malloc-std-namespace.cpp +++ b/clang/test/Analysis/malloc-std-namespace.cpp @@ -18,7 +18,7 @@ void no_leak() { void invalid_free() { int i; int *p = &i; - //expected-note@+2{{Argument to free() is the address of the local variable 'i', which is not memory allocated by malloc()}} - //expected-warning@+1{{Argument to free() is the address of the local variable 'i', which is not memory allocated by malloc()}} + //expected-note@+2{{Argument to 'free()' is the address of the local variable 'i', which is not memory allocated by 'malloc()'}} + //expected-warning@+1{{Argument to 'free()' is the address of the local variable 'i', which is not memory allocated by 'malloc()'}} std::free(p); } diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index 8ee29aeb324f72..9c7ca43bfbc5af 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -732,7 +732,7 @@ void mallocCastToFP(void) { free(p); } -// This tests that malloc() buffers are undefined by default +// This tests that 'malloc()' buffers are undefined by default char mallocGarbage (void) { char *buf = malloc(2); char result = buf[1]; // expected-warning{{undefined}} @@ -778,17 +778,17 @@ void paramFree(int *p) { void allocaFree(void) { int *p = alloca(sizeof(int)); - free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void allocaFreeBuiltin(void) { int *p = __builtin_alloca(sizeof(int)); - free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } void allocaFreeBuiltinAlign(void) { int *p = __builtin_alloca_with_align(sizeof(int), 64); - free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}} + free(p); // expected-warning {{Memory allocated by 'alloca()' should not be deallocated}} } @@ -1327,7 +1327,7 @@ void radar10978247_positive(int myValueSize) { else return; // expected-warning {{leak}} } -// Previously this triggered a false positive because malloc() is known to +// Previously this triggered a false positive because 'malloc()' is known to // return uninitialized memory and the binding of 'o' to 'p->n' was not getting // propertly handled. Now we report a leak. struct rdar11269741_a_t { @@ -1656,26 +1656,26 @@ void testOffsetDeallocate(int *memoryBlock) { void testOffsetOfRegionFreed(void) { __int64_t * array = malloc(sizeof(__int64_t)*2); array += 1; - free(&array[0]); // expected-warning{{Argument to free() is offset by 8 bytes from the start of memory allocated by malloc()}} + free(&array[0]); // expected-warning{{Argument to 'free()' is offset by 8 bytes from the start of memory allocated by 'malloc()'}} } void testOffsetOfRegionFreed2(void) { __int64_t *p = malloc(sizeof(__int64_t)*2); p += 1; - free(p); // expected-warning{{Argument to free() is offset by 8 bytes from the start of memory allocated by malloc()}} + free(p); // expected-warning{{Argument to 'free()' is offset by 8 bytes from the start of memory allocated by 'malloc()'}} } void testOffsetOfRegionFreed3(void) { char *r = malloc(sizeof(char)); r = r - 10; - free(r); // expected-warning {{Argument to free() is offset by -10 bytes from the start of memory allocated by malloc()}} + free(r); // expected-warning {{Argument to 'free()' is offset by -10 bytes from the start of memory allocated by 'malloc()'}} } void testOffsetOfRegionFreedAfterFunctionCall(void) { int *p = malloc(sizeof(int)*2); p += 1; myfoo(p); - free(p); // expected-warning{{Argument to free() is offset by 4 bytes from the start of memory allocated by malloc()}} + free(p); // expected-warning{{Argument to 'free()' is offset by 4 bytes from the start of memory allocated by 'malloc()'}} } void testFixManipulatedPointerBeforeFree(void) { @@ -1695,7 +1695,7 @@ void freeOffsetPointerPassedToFunction(void) { p[1] = 0; p += 1; myfooint(*p); // not passing the pointer, only a value pointed by pointer - free(p); // expected-warning {{Argument to free() is offset by 8 bytes from the start of memory allocated by malloc()}} + free(p); // expected-warning {{Argument to 'free()' is offset by 8 bytes from the start of memory allocated by 'malloc()'}} } int arbitraryInt(void); @@ -1709,13 +1709,13 @@ void testFreeNonMallocPointerWithNoOffset(void) { char c; char *r = &c; r = r + 10; - free(r-10); // expected-warning {{Argument to free() is the address of the local variable 'c', which is not memory allocated by malloc()}} + free(r-10); // expected-warning {{Argument to 'free()' is the address of the local variable 'c', which is not memory allocated by 'malloc()'}} } void testFreeNonMallocPointerWithOffset(void) { char c; char *r = &c; - free(r+1); // expected-warning {{Argument to free() is the address of the local variable 'c', which is not memory allocated by malloc()}} + free(r+1); // expected-warning {{Argument to 'free()' is the address of the local variable 'c', which is not memory allocated by 'malloc()'}} } void testOffsetZeroDoubleFree(void) { @@ -1735,14 +1735,14 @@ void testOffsetPassedToStrlenThenFree(void) { char * string = malloc(sizeof(char)*10); string += 1; int length = strlen(string); - free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}} + free(string); // expected-warning {{Argument to 'free()' is offset by 1 byte from the start of memory allocated by 'malloc()'}} } void testOffsetPassedAsConst(void) { char * string = malloc(sizeof(char)*10); string += 1; passConstPtr(string); - free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}} + free(string); // expected-warning {{Argument to 'free()' is offset by 1 byte from the start of memory allocated by 'malloc()'}} } char **_vectorSegments; @@ -1842,12 +1842,12 @@ int testNoCheckerDataPropogationFromLogicalOpOperandToOpResult(void) { void (*fnptr)(int); void freeIndirectFunctionPtr(void) { void *p = (void *)fnptr; - free(p); // expected-warning {{Argument to free() is a function pointer}} + free(p); // expected-warning {{Argument to 'free()' is a function pointer}} } void freeFunctionPtr(void) { free((void *)fnptr); - // expected-warning@-1{{Argument to free() is a function pointer}} + // expected-warning@-1{{Argument to 'free()' is a function pointer}} // expected-warning@-2{{attempt to call free on non-heap object '(void *)fnptr'}} } @@ -1905,8 +1905,8 @@ enum { BUFSIZE = 256 }; void MEM34_C(void) { char buf[BUFSIZE]; char *p = (char *)realloc(buf, 2 * BUFSIZE); - // expected-warning@-1{{Argument to realloc() is the address of the local \ -variable 'buf', which is not memory allocated by malloc() [unix.Malloc]}} + // expected-warning@-1{{Argument to 'realloc()' is the address of the local \ +variable 'buf', which is not memory allocated by 'malloc()' [unix.Malloc]}} if (p == NULL) { /* Handle error */ } diff --git a/clang/test/Analysis/malloc.mm b/clang/test/Analysis/malloc.mm index 94a46d731090b3..5b816a1524aec7 100644 --- a/clang/test/Analysis/malloc.mm +++ b/clang/test/Analysis/malloc.mm @@ -89,7 +89,7 @@ void testNSStringFreeWhenDoneNO2(NSUInteger dataLength) { void testOffsetFree() { int *p = (int *)malloc(sizeof(int)); - NSData *nsdata = [NSData dataWithBytesNoCopy:++p length:sizeof(int) freeWhenDone:1]; // expected-warning{{Argument to +dataWithBytesNoCopy:length:freeWhenDone: is offset by 4 bytes from the start of memory allocated by malloc()}} + NSData *nsdata = [NSData dataWithBytesNoCopy:++p length:sizeof(int) freeWhenDone:1]; // expected-warning{{Argument to +dataWithBytesNoCopy:length:freeWhenDone: is offset by 4 bytes from the start of memory allocated by 'malloc()'}} } void testRelinquished1() { diff --git a/clang/test/Analysis/plist-macros.cpp b/clang/test/Analysis/plist-macros.cpp index 94f8e514c8b39a..d7b3c7cc1c86f2 100644 --- a/clang/test/Analysis/plist-macros.cpp +++ b/clang/test/Analysis/plist-macros.cpp @@ -13,7 +13,7 @@ void noteOnMacro(int y) { mallocmemory y++; y++; - delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} + delete x; // expected-warning {{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'delete'}} } void macroIsFirstInFunction(int y) { diff --git a/clang/test/Analysis/weak-functions.c b/clang/test/Analysis/weak-functions.c index 26cbfb3523a927..5bdb411fcbf4fb 100644 --- a/clang/test/Analysis/weak-functions.c +++ b/clang/test/Analysis/weak-functions.c @@ -72,7 +72,7 @@ void free(void *) __attribute__((weak_import)); void t10 (void) { free((void*)&t10); - // expected-warning@-1{{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}} + // expected-warning@-1{{Argument to 'free()' is the address of the function 't10', which is not memory allocated by 'malloc()'}} // expected-warning@-2{{attempt to call free on non-heap object 't10'}} } From 5316802a40cf98ca74b3f2e9da4e4117bf0e760f Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Mon, 15 Jul 2024 00:57:21 +0300 Subject: [PATCH 6/9] csa/tests: add test suite for custom allocation classes --- .../MismatchedDeallocator-checker-test.mm | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/clang/test/Analysis/MismatchedDeallocator-checker-test.mm b/clang/test/Analysis/MismatchedDeallocator-checker-test.mm index bbf24837f9da82..ef8b24ba8de32e 100644 --- a/clang/test/Analysis/MismatchedDeallocator-checker-test.mm +++ b/clang/test/Analysis/MismatchedDeallocator-checker-test.mm @@ -14,6 +14,19 @@ void free(void *); void __attribute((ownership_takes(malloc, 1))) my_free(void *); +void __attribute((ownership_returns(malloc1))) *my_malloc1(size_t); +void __attribute((ownership_takes(malloc1, 1))) my_free1(void *); + +void __attribute((ownership_returns(malloc2))) *my_malloc2(size_t); + +// The order of these declarations are important to verify that analisys still works even +// if there are less specific declarations of the same functions +void __attribute((ownership_returns(malloc3))) *my_malloc3(size_t); +void *my_malloc3(size_t); + +void *my_malloc4(size_t); +void __attribute((ownership_returns(malloc4))) *my_malloc4(size_t); + //--------------------------------------------------------------- // Test if an allocation function matches deallocation function //--------------------------------------------------------------- @@ -60,6 +73,41 @@ void testMalloc8() { operator delete[](p); // expected-warning{{Memory allocated by 'malloc()' should be deallocated by 'free()', not 'operator delete[]'}} } +void testMalloc9() { + int *p = (int *)my_malloc(sizeof(int)); + my_free(p); // no warning +} + +void testMalloc10() { + int *p = (int *)my_malloc1(sizeof(int)); + my_free1(p); // no warning +} + +void testMalloc11() { + int *p = (int *)my_malloc1(sizeof(int)); + my_free(p); // expected-warning{{Memory allocated by 'my_malloc1()' should be deallocated by function that takes ownership of 'malloc1', not 'my_free()', which takes ownership of 'malloc'}} +} + +void testMalloc12() { + int *p = (int *)my_malloc2(sizeof(int)); + my_free1(p); // expected-warning{{Memory allocated by 'my_malloc2()' should be deallocated by function that takes ownership of 'malloc2', not 'my_free1()', which takes ownership of 'malloc1'}} +} + +void testMalloc13() { + int *p = (int *)my_malloc1(sizeof(int)); + free(p); // expected-warning{{Memory allocated by 'my_malloc1()' should be deallocated by function that takes ownership of 'malloc1', not 'free()'}} +} + +void testMalloc14() { + int *p = (int *)my_malloc3(sizeof(int)); + free(p); // expected-warning{{Memory allocated by 'my_malloc3()' should be deallocated by function that takes ownership of 'malloc3', not 'free()'}} +} + +void testMalloc15() { + int *p = (int *)my_malloc4(sizeof(int)); + free(p); // expected-warning{{Memory allocated by 'my_malloc4()' should be deallocated by function that takes ownership of 'malloc4', not 'free()'}} +} + void testAlloca() { int *p = (int *)__builtin_alloca(sizeof(int)); delete p; // expected-warning{{Memory allocated by 'alloca()' should not be deallocated}} From 40b2207072fa0e082116a644abecfcda948a5e45 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Thu, 18 Jul 2024 10:23:57 +0300 Subject: [PATCH 7/9] csa/MallocChecker: fix style and unexpected shadowing --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 0db0e622d2b0d2..95ec28bfd20da1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -118,10 +118,10 @@ struct AllocationFamily { AllocationFamilyKind Kind; std::optional CustomName; - explicit AllocationFamily(AllocationFamilyKind Kind, + explicit AllocationFamily(AllocationFamilyKind AKind, std::optional Name = std::nullopt) - : Kind(Kind), CustomName(Name) { - assert((Kind != AF_Custom || Name.has_value()) && + : Kind(AKind), CustomName(Name) { + assert((Kind != AF_Custom || CustomName.has_value()) && "Custom family must specify also the name"); // Preseve previous behavior when "malloc" class means AF_Malloc @@ -1925,25 +1925,22 @@ static bool didPreviousFreeFail(ProgramStateRef State, static void printOwnershipTakesList(raw_ostream &os, CheckerContext &C, const Expr *E) { - if (const CallExpr *CE = dyn_cast(E)) { - const FunctionDecl *FD = CE->getDirectCallee(); - if (!FD) - return; + const CallExpr *CE = dyn_cast(E); - if (!FD->hasAttrs()) - return; + if (!CE) + return; - // Only one ownership_takes attribute is allowed - for (const auto *I : FD->specific_attrs()) { - OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind(); + const FunctionDecl *FD = CE->getDirectCallee(); + if (!FD) + return; - if (OwnKind != OwnershipAttr::Takes) - continue; + // Only one ownership_takes attribute is allowed. + for (const auto *I : FD->specific_attrs()) { + if (I->getOwnKind() != OwnershipAttr::Takes) + continue; - os << ", which takes ownership of " << '\'' << I->getModule()->getName() - << '\''; - break; - } + os << ", which takes ownership of '" << I->getModule()->getName() << '\''; + break; } } From 0392ef99301bf9fee7de3fccabda5dffcdc3db75 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Thu, 18 Jul 2024 17:19:36 +0300 Subject: [PATCH 8/9] add release note --- clang/docs/ReleaseNotes.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 83fd5dc31547eb..bd879277fd5bfb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -107,6 +107,9 @@ Removed Compiler Flags Attribute Changes in Clang -------------------------- +- Clang now disallows more than one ``__attribute__((ownership_returns(class, idx)))`` with + different class names attached to one function. + Improvements to Clang's diagnostics ----------------------------------- @@ -206,6 +209,11 @@ Static Analyzer New features ^^^^^^^^^^^^ +- MallocChecker now checks for ``ownership_returns(class, idx)`` and ``ownership_takes(class, idx)`` + attributes with class names different from "malloc". Clang static analyzer now reports an error + if class of allocation and deallocation function mismatches. + `Documentation `__. + Crash and bug fixes ^^^^^^^^^^^^^^^^^^^ From ca8dfa9572ea8552d4b43d7ebbdd0d8313a29850 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Thu, 18 Jul 2024 18:52:38 +0300 Subject: [PATCH 9/9] add example for mismatched deallocation class in documentation --- .../checkers/mismatched_deallocator_example.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/clang/docs/analyzer/checkers/mismatched_deallocator_example.cpp b/clang/docs/analyzer/checkers/mismatched_deallocator_example.cpp index 2a4103240fe81a..3b7c7d99faef3c 100644 --- a/clang/docs/analyzer/checkers/mismatched_deallocator_example.cpp +++ b/clang/docs/analyzer/checkers/mismatched_deallocator_example.cpp @@ -6,6 +6,10 @@ void test() { // C, C++ void __attribute((ownership_returns(malloc))) *user_malloc(size_t); +void __attribute((ownership_takes(malloc, 1))) *user_free(void *); + +void __attribute((ownership_returns(malloc1))) *user_malloc1(size_t); +void __attribute((ownership_takes(malloc1, 1))) *user_free1(void *); void test() { int *p = (int *)user_malloc(sizeof(int)); @@ -24,6 +28,12 @@ void test() { realloc(p, sizeof(long)); // warn } +// C, C++ +void test() { + int *p = user_malloc(10); + user_free1(p); // warn +} + // C, C++ template struct SimpleSmartPointer {