github-actions[bot] wrote: <!--LLVM CODE FORMAT COMMENT: {clang-format}-->
:warning: C/C++ code formatter, clang-format found issues in your code. :warning: <details> <summary> You can test this locally with the following command: </summary> ``````````bash git-clang-format --diff 1cde1240ed6e45012d7510f4aa39badbdb4a4721 b31ec694c88635404b252f00472140e83083fd02 -- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/malloc.c clang/test/Analysis/taint-diagnostic-visitor.c `````````` </details> <details> <summary> View the diff from clang-format here. </summary> ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 0167dd1336..84fc6700d2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1824,219 +1824,232 @@ void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State, } } -void MallocChecker::CheckTaintedness( - CheckerContext & C, const CallEvent &Call, const SVal SizeSVal, - ProgramStateRef State, AllocationFamily Family) const { - std::vector<SymbolRef> TaintedSyms = - taint::getTaintedSymbols(State, SizeSVal); - if (TaintedSyms.empty()) - return; - - SValBuilder &SVB = C.getSValBuilder(); - QualType SizeTy = SVB.getContext().getSizeType(); - QualType CmpTy = SVB.getConditionType(); - // In case the symbol is tainted, we give a warning if the - // size is larger than SIZE_MAX/4 - BasicValueFactory &BVF = SVB.getBasicValueFactory(); - const llvm::APSInt MaxValInt = BVF.getMaxValue(SizeTy); - NonLoc MaxLength = - SVB.makeIntVal(MaxValInt / APSIntType(MaxValInt).getValue(4)); - std::optional<NonLoc> SizeNL = SizeSVal.getAs<NonLoc>(); - auto Cmp = SVB.evalBinOpNN(State, BO_GE, *SizeNL, MaxLength, CmpTy) - .getAs<DefinedOrUnknownSVal>(); - if (!Cmp) - return; - auto [StateTooLarge, StateNotTooLarge] = State->assume(*Cmp); - if (!StateTooLarge && StateNotTooLarge) { - // we can prove that size is not too large so ok. - return; - } +void MallocChecker::CheckTaintedness(CheckerContext &C, const CallEvent &Call, + const SVal SizeSVal, ProgramStateRef State, + AllocationFamily Family) const { + std::vector<SymbolRef> TaintedSyms = + taint::getTaintedSymbols(State, SizeSVal); + if (TaintedSyms.empty()) + return; - std::string Callee = "Memory allocation function"; - if (Call.getCalleeIdentifier()) - Callee = Call.getCalleeIdentifier()->getName().str(); - reportTaintBug( - Callee + " is called with a tainted (potentially attacker controlled) " - "value. Make sure the value is bound checked.", - State, C, TaintedSyms, Family, Call.getArgExpr(0)); + SValBuilder &SVB = C.getSValBuilder(); + QualType SizeTy = SVB.getContext().getSizeType(); + QualType CmpTy = SVB.getConditionType(); + // In case the symbol is tainted, we give a warning if the + // size is larger than SIZE_MAX/4 + BasicValueFactory &BVF = SVB.getBasicValueFactory(); + const llvm::APSInt MaxValInt = BVF.getMaxValue(SizeTy); + NonLoc MaxLength = + SVB.makeIntVal(MaxValInt / APSIntType(MaxValInt).getValue(4)); + std::optional<NonLoc> SizeNL = SizeSVal.getAs<NonLoc>(); + auto Cmp = SVB.evalBinOpNN(State, BO_GE, *SizeNL, MaxLength, CmpTy) + .getAs<DefinedOrUnknownSVal>(); + if (!Cmp) + return; + auto [StateTooLarge, StateNotTooLarge] = State->assume(*Cmp); + if (!StateTooLarge && StateNotTooLarge) { + // we can prove that size is not too large so ok. + return; } - ProgramStateRef MallocChecker::MallocMemAux( - CheckerContext & C, const CallEvent &Call, SVal Size, SVal Init, - ProgramStateRef State, AllocationFamily Family) const { - if (!State) - return nullptr; + std::string Callee = "Memory allocation function"; + if (Call.getCalleeIdentifier()) + Callee = Call.getCalleeIdentifier()->getName().str(); + reportTaintBug( + Callee + " is called with a tainted (potentially attacker controlled) " + "value. Make sure the value is bound checked.", + State, C, TaintedSyms, Family, Call.getArgExpr(0)); +} - const Expr *CE = Call.getOriginExpr(); +ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, + const CallEvent &Call, SVal Size, + SVal Init, ProgramStateRef State, + AllocationFamily Family) const { + if (!State) + return nullptr; - // We expect the malloc functions to return a pointer. - if (!Loc::isLocType(CE->getType())) - return nullptr; + const Expr *CE = Call.getOriginExpr(); - // Bind the return value to the symbolic value from the heap region. - // TODO: move use of this functions to an EvalCall callback, becasue - // BindExpr() should'nt be used elsewhere. - 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>()); - State = State->BindExpr(CE, C.getLocationContext(), RetVal); + // We expect the malloc functions to return a pointer. + if (!Loc::isLocType(CE->getType())) + return nullptr; - // Fill the region with the initialization value. - State = State->bindDefaultInitial(RetVal, Init, LCtx); + // Bind the return value to the symbolic value from the heap region. + // TODO: move use of this functions to an EvalCall callback, becasue + // BindExpr() should'nt be used elsewhere. + 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>()); + State = State->BindExpr(CE, C.getLocationContext(), RetVal); - // If Size is somehow undefined at this point, this line prevents a crash. - if (Size.isUndef()) - Size = UnknownVal(); + // Fill the region with the initialization value. + State = State->bindDefaultInitial(RetVal, Init, LCtx); - CheckTaintedness(C, Call, Size, State, AF_Malloc); + // If Size is somehow undefined at this point, this line prevents a crash. + if (Size.isUndef()) + Size = UnknownVal(); - // Set the region's extent. - State = setDynamicExtent(State, RetVal.getAsRegion(), - Size.castAs<DefinedOrUnknownSVal>(), SVB); + CheckTaintedness(C, Call, Size, State, AF_Malloc); - return MallocUpdateRefState(C, CE, State, Family); - } + // Set the region's extent. + State = setDynamicExtent(State, RetVal.getAsRegion(), + Size.castAs<DefinedOrUnknownSVal>(), SVB); - static ProgramStateRef MallocUpdateRefState( - CheckerContext & C, const Expr *E, ProgramStateRef State, - AllocationFamily Family, std::optional<SVal> RetVal) { - if (!State) - return nullptr; + return MallocUpdateRefState(C, CE, State, Family); +} + +static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E, + ProgramStateRef State, + AllocationFamily Family, + std::optional<SVal> RetVal) { + if (!State) + return nullptr; - // Get the return value. - if (!RetVal) - RetVal = C.getSVal(E); + // Get the return value. + if (!RetVal) + RetVal = C.getSVal(E); - // We expect the malloc functions to return a pointer. - if (!RetVal->getAs<Loc>()) - return nullptr; + // We expect the malloc functions to return a pointer. + if (!RetVal->getAs<Loc>()) + return nullptr; - SymbolRef Sym = RetVal->getAsLocSymbol(); + SymbolRef Sym = RetVal->getAsLocSymbol(); - // This is a return value of a function that was not inlined, such as - // malloc() or new(). We've checked that in the caller. Therefore, it must - // be a symbol. - assert(Sym); - // FIXME: In theory this assertion should fail for `alloca()` calls (because - // `AllocaRegion`s are not symbolic); but in practice this does not happen. - // As the current code appears to work correctly, I'm not touching this - // issue now, but it would be good to investigate and clarify this. Also - // note that perhaps the special `AllocaRegion` should be replaced by - // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to - // enable proper tracking of memory allocated by `alloca()` -- and after - // that change this assertion would become valid again. - - // Set the symbol's state to Allocated. - return State->set<RegionState>(Sym, RefState::getAllocated(Family, E)); - } - - ProgramStateRef MallocChecker::FreeMemAttr( - CheckerContext & C, const CallEvent &Call, const OwnershipAttr *Att, - ProgramStateRef State) const { - if (!State) - return nullptr; + // This is a return value of a function that was not inlined, such as + // malloc() or new(). We've checked that in the caller. Therefore, it must + // be a symbol. + assert(Sym); + // FIXME: In theory this assertion should fail for `alloca()` calls (because + // `AllocaRegion`s are not symbolic); but in practice this does not happen. + // As the current code appears to work correctly, I'm not touching this + // issue now, but it would be good to investigate and clarify this. Also + // note that perhaps the special `AllocaRegion` should be replaced by + // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to + // enable proper tracking of memory allocated by `alloca()` -- and after + // that change this assertion would become valid again. + + // Set the symbol's state to Allocated. + return State->set<RegionState>(Sym, RefState::getAllocated(Family, E)); +} + +ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, + const CallEvent &Call, + const OwnershipAttr *Att, + ProgramStateRef State) const { + if (!State) + return nullptr; - if (Att->getModule()->getName() != "malloc") - return nullptr; + if (Att->getModule()->getName() != "malloc") + return nullptr; - bool IsKnownToBeAllocated = false; + bool IsKnownToBeAllocated = false; - for (const auto &Arg : Att->args()) { - ProgramStateRef StateI = - FreeMemAux(C, Call, State, Arg.getASTIndex(), - Att->getOwnKind() == OwnershipAttr::Holds, - IsKnownToBeAllocated, AF_Malloc); - if (StateI) - State = StateI; - } - return State; + for (const auto &Arg : Att->args()) { + ProgramStateRef StateI = + FreeMemAux(C, Call, State, Arg.getASTIndex(), + Att->getOwnKind() == OwnershipAttr::Holds, + IsKnownToBeAllocated, AF_Malloc); + if (StateI) + State = StateI; } + return State; +} - ProgramStateRef MallocChecker::FreeMemAux( - CheckerContext & C, const CallEvent &Call, ProgramStateRef State, - unsigned Num, bool Hold, bool &IsKnownToBeAllocated, - AllocationFamily Family, bool ReturnsNullOnFailure) const { - if (!State) - return nullptr; +ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, + const CallEvent &Call, + ProgramStateRef State, unsigned Num, + bool Hold, bool &IsKnownToBeAllocated, + AllocationFamily Family, + bool ReturnsNullOnFailure) const { + if (!State) + return nullptr; - if (Call.getNumArgs() < (Num + 1)) - return nullptr; + if (Call.getNumArgs() < (Num + 1)) + return nullptr; - return FreeMemAux(C, Call.getArgExpr(Num), Call, State, Hold, - IsKnownToBeAllocated, Family, ReturnsNullOnFailure); - } - - /// Checks if the previous call to free on the given symbol failed - if free - /// failed, returns true. Also, returns the corresponding return value symbol. - static bool didPreviousFreeFail(ProgramStateRef State, SymbolRef Sym, - SymbolRef & RetStatusSymbol) { - const SymbolRef *Ret = State->get<FreeReturnValue>(Sym); - if (Ret) { - assert(*Ret && "We should not store the null return symbol"); - ConstraintManager &CMgr = State->getConstraintManager(); - ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret); - RetStatusSymbol = *Ret; - return FreeFailed.isConstrainedTrue(); - } - return false; - } + return FreeMemAux(C, Call.getArgExpr(Num), Call, State, Hold, + IsKnownToBeAllocated, Family, ReturnsNullOnFailure); +} - static bool printMemFnName(raw_ostream & os, CheckerContext & C, - const Expr *E) { - if (const CallExpr *CE = dyn_cast<CallExpr>(E)) { - // FIXME: This doesn't handle indirect calls. - const FunctionDecl *FD = CE->getDirectCallee(); - if (!FD) - return false; +/// Checks if the previous call to free on the given symbol failed - if free +/// failed, returns true. Also, returns the corresponding return value symbol. +static bool didPreviousFreeFail(ProgramStateRef State, SymbolRef Sym, + SymbolRef &RetStatusSymbol) { + const SymbolRef *Ret = State->get<FreeReturnValue>(Sym); + if (Ret) { + assert(*Ret && "We should not store the null return symbol"); + ConstraintManager &CMgr = State->getConstraintManager(); + ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret); + RetStatusSymbol = *Ret; + return FreeFailed.isConstrainedTrue(); + } + return false; +} - os << *FD; - if (!FD->isOverloadedOperator()) - os << "()"; - return true; - } +static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E) { + if (const CallExpr *CE = dyn_cast<CallExpr>(E)) { + // FIXME: This doesn't handle indirect calls. + const FunctionDecl *FD = CE->getDirectCallee(); + if (!FD) + return false; - if (const ObjCMessageExpr *Msg = dyn_cast<ObjCMessageExpr>(E)) { - if (Msg->isInstanceMessage()) - os << "-"; - else - os << "+"; - Msg->getSelector().print(os); - return true; - } + os << *FD; + if (!FD->isOverloadedOperator()) + os << "()"; + return true; + } - if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) { - os << "'" - << getOperatorSpelling(NE->getOperatorNew()->getOverloadedOperator()) - << "'"; - return true; - } + if (const ObjCMessageExpr *Msg = dyn_cast<ObjCMessageExpr>(E)) { + if (Msg->isInstanceMessage()) + os << "-"; + else + os << "+"; + Msg->getSelector().print(os); + return true; + } - if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) { - os << "'" - << getOperatorSpelling( - DE->getOperatorDelete()->getOverloadedOperator()) - << "'"; - return true; - } + if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) { + os << "'" + << getOperatorSpelling(NE->getOperatorNew()->getOverloadedOperator()) + << "'"; + return true; + } - return false; + if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) { + os << "'" + << getOperatorSpelling(DE->getOperatorDelete()->getOverloadedOperator()) + << "'"; + return true; } - static void printExpectedAllocName(raw_ostream & os, - AllocationFamily Family) { + return false; +} - 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: llvm_unreachable("not a deallocation expression"); +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: + llvm_unreachable("not a deallocation expression"); } } `````````` </details> https://github.com/llvm/llvm-project/pull/92420 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits