From 30e5c7e82fa1c5318540feb83d54757c632e2599 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Thu, 9 Apr 2020 16:06:32 +0200 Subject: [PATCH] [analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API Summary: I wanted to extend the diagnostics of the CStringChecker with taintedness. This requires the CStringChecker to be refactored to support a more flexible reporting mechanism. This patch does only refactorings, such: - eliminates always false parameters (like WarnAboutSize) - reduces the number of parameters - makes strong types differentiating *source* and *destination* buffers (same with size expressions) - binds the argument expression and the index, making diagnostics accurate and easy to emit - removes a bunch of default parameters to make it more readable - remove random const char* warning message parameters, making clear where and what is going to be emitted Note that: - CheckBufferAccess now checks *only* one buffer, this removed about 100 LOC code duplication - not every function was refactored to use the /new/ strongly typed API, since the CString related functions are really closely coupled monolithic beasts, I will refactor them separately - all tests are preserved and passing; only the message changed at some places. In my opinion, these messages are holding the same information. I would also highlight that this refactoring caught a bug in clang/test/Analysis/string.c:454 where the diagnostic did not reflect reality. This catch backs my effort on simplifying this monolithic CStringChecker. Reviewers: NoQ, baloghadamsoftware, Szelethus, rengolin, Charusso Reviewed By: NoQ Subscribers: whisperity, xazax.hun, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, martong, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D74806 --- .../lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 584 +++++++++++---------- clang/test/Analysis/bsd-string.c | 8 +- clang/test/Analysis/bstring.c | 12 +- clang/test/Analysis/null-deref-ps-region.c | 7 +- clang/test/Analysis/string.c | 46 +- 5 files changed, 339 insertions(+), 318 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 4558b69..40467b3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -30,6 +30,47 @@ using namespace clang; using namespace ento; namespace { +struct AnyArgExpr { + // FIXME: Remove constructor in C++17 to turn it into an aggregate. + AnyArgExpr(const Expr *Expression, unsigned ArgumentIndex) + : Expression{Expression}, ArgumentIndex{ArgumentIndex} {} + const Expr *Expression; + unsigned ArgumentIndex; +}; + +struct SourceArgExpr : AnyArgExpr { + using AnyArgExpr::AnyArgExpr; // FIXME: Remove using in C++17. +}; + +struct DestinationArgExpr : AnyArgExpr { + using AnyArgExpr::AnyArgExpr; // FIXME: Same. +}; + +struct SizeArgExpr : AnyArgExpr { + using AnyArgExpr::AnyArgExpr; // FIXME: Same. +}; + +using ErrorMessage = SmallString<128>; +enum class AccessKind { write, read }; + +static ErrorMessage createOutOfBoundErrorMsg(StringRef FunctionDescription, + AccessKind Access) { + ErrorMessage Message; + llvm::raw_svector_ostream Os(Message); + + // Function classification like: Memory copy function + Os << toUppercase(FunctionDescription.front()) + << &FunctionDescription.data()[1]; + + if (Access == AccessKind::write) { + Os << " overflows the destination buffer"; + } else { // read access + Os << " accesses out-of-bound array element"; + } + + return Message; +} + enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 }; class CStringChecker : public Checker< eval::Call, check::PreStmt, @@ -113,12 +154,9 @@ public: void evalMemmove(CheckerContext &C, const CallExpr *CE) const; void evalBcopy(CheckerContext &C, const CallExpr *CE) const; void evalCopyCommon(CheckerContext &C, const CallExpr *CE, - ProgramStateRef state, - const Expr *Size, - const Expr *Source, - const Expr *Dest, - bool Restricted = false, - bool IsMempcpy = false) const; + ProgramStateRef state, SizeArgExpr Size, + DestinationArgExpr Dest, SourceArgExpr Source, + bool Restricted, bool IsMempcpy) const; void evalMemcmp(CheckerContext &C, const CallExpr *CE) const; @@ -195,40 +233,17 @@ public: ProgramStateRef &State); // Re-usable checks - ProgramStateRef checkNonNull(CheckerContext &C, - ProgramStateRef state, - const Expr *S, - SVal l, - unsigned IdxOfArg) const; - ProgramStateRef CheckLocation(CheckerContext &C, - ProgramStateRef state, - const Expr *S, - SVal l, - const char *message = nullptr) const; - ProgramStateRef CheckBufferAccess(CheckerContext &C, - ProgramStateRef state, - const Expr *Size, - const Expr *FirstBuf, - const Expr *SecondBuf, - const char *firstMessage = nullptr, - const char *secondMessage = nullptr, - bool WarnAboutSize = false) const; - - ProgramStateRef CheckBufferAccess(CheckerContext &C, - ProgramStateRef state, - const Expr *Size, - const Expr *Buf, - const char *message = nullptr, - bool WarnAboutSize = false) const { - // This is a convenience overload. - return CheckBufferAccess(C, state, Size, Buf, nullptr, message, nullptr, - WarnAboutSize); - } - ProgramStateRef CheckOverlap(CheckerContext &C, - ProgramStateRef state, - const Expr *Size, - const Expr *First, - const Expr *Second) const; + ProgramStateRef checkNonNull(CheckerContext &C, ProgramStateRef State, + AnyArgExpr Arg, SVal l) const; + ProgramStateRef CheckLocation(CheckerContext &C, ProgramStateRef state, + AnyArgExpr Buffer, SVal Element, + AccessKind Access) const; + ProgramStateRef CheckBufferAccess(CheckerContext &C, ProgramStateRef State, + AnyArgExpr Buffer, SizeArgExpr Size, + AccessKind Access) const; + ProgramStateRef CheckOverlap(CheckerContext &C, ProgramStateRef state, + SizeArgExpr Size, AnyArgExpr First, + AnyArgExpr Second) const; void emitOverlapBug(CheckerContext &C, ProgramStateRef state, const Stmt *First, @@ -277,26 +292,26 @@ CStringChecker::assumeZero(CheckerContext &C, ProgramStateRef state, SVal V, } ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, - ProgramStateRef state, - const Expr *S, SVal l, - unsigned IdxOfArg) const { + ProgramStateRef State, + AnyArgExpr Arg, SVal l) const { // If a previous check has failed, propagate the failure. - if (!state) + if (!State) return nullptr; ProgramStateRef stateNull, stateNonNull; - std::tie(stateNull, stateNonNull) = assumeZero(C, state, l, S->getType()); + std::tie(stateNull, stateNonNull) = + assumeZero(C, State, l, Arg.Expression->getType()); if (stateNull && !stateNonNull) { if (Filter.CheckCStringNullArg) { SmallString<80> buf; llvm::raw_svector_ostream OS(buf); assert(CurrentFunctionDescription); - OS << "Null pointer passed as " << IdxOfArg - << llvm::getOrdinalSuffix(IdxOfArg) << " argument to " + OS << "Null pointer passed as " << (Arg.ArgumentIndex + 1) + << llvm::getOrdinalSuffix(Arg.ArgumentIndex + 1) << " argument to " << CurrentFunctionDescription; - emitNullArgBug(C, stateNull, S, OS.str()); + emitNullArgBug(C, stateNull, Arg.Expression, OS.str()); } return nullptr; } @@ -308,19 +323,20 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, // FIXME: This was originally copied from ArrayBoundChecker.cpp. Refactor? ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, - ProgramStateRef state, - const Expr *S, SVal l, - const char *warningMsg) const { + ProgramStateRef state, + AnyArgExpr Buffer, SVal Element, + AccessKind Access) const { + // If a previous check has failed, propagate the failure. if (!state) return nullptr; // Check for out of bound array element access. - const MemRegion *R = l.getAsRegion(); + const MemRegion *R = Element.getAsRegion(); if (!R) return state; - const ElementRegion *ER = dyn_cast(R); + const auto *ER = dyn_cast(R); if (!ER) return state; @@ -328,7 +344,7 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, return state; // Get the size of the array. - const SubRegion *superReg = cast(ER->getSuperRegion()); + const auto *superReg = cast(ER->getSuperRegion()); DefinedOrUnknownSVal Size = getDynamicSize(state, superReg, C.getSValBuilder()); @@ -343,20 +359,11 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, // In the latter case we only do modeling but do not emit warning. if (!Filter.CheckCStringOutOfBounds) return nullptr; - // Emit a bug report. - if (warningMsg) { - emitOutOfBoundsBug(C, StOutBound, S, warningMsg); - } else { - assert(CurrentFunctionDescription); - assert(CurrentFunctionDescription[0] != '\0'); - SmallString<80> buf; - llvm::raw_svector_ostream os(buf); - os << toUppercase(CurrentFunctionDescription[0]) - << &CurrentFunctionDescription[1] - << " accesses out-of-bound array element"; - emitOutOfBoundsBug(C, StOutBound, S, os.str()); - } + // Emit a bug report. + ErrorMessage Message = + createOutOfBoundErrorMsg(CurrentFunctionDescription, Access); + emitOutOfBoundsBug(C, StOutBound, Buffer.Expression, Message); return nullptr; } @@ -366,89 +373,68 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, } ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C, - ProgramStateRef state, - const Expr *Size, - const Expr *FirstBuf, - const Expr *SecondBuf, - const char *firstMessage, - const char *secondMessage, - bool WarnAboutSize) const { + ProgramStateRef State, + AnyArgExpr Buffer, + SizeArgExpr Size, + AccessKind Access) const { // If a previous check has failed, propagate the failure. - if (!state) + if (!State) return nullptr; SValBuilder &svalBuilder = C.getSValBuilder(); ASTContext &Ctx = svalBuilder.getContext(); - const LocationContext *LCtx = C.getLocationContext(); - QualType sizeTy = Size->getType(); + QualType SizeTy = Size.Expression->getType(); QualType PtrTy = Ctx.getPointerType(Ctx.CharTy); // Check that the first buffer is non-null. - SVal BufVal = C.getSVal(FirstBuf); - state = checkNonNull(C, state, FirstBuf, BufVal, 1); - if (!state) + SVal BufVal = C.getSVal(Buffer.Expression); + State = checkNonNull(C, State, Buffer, BufVal); + if (!State) return nullptr; // If out-of-bounds checking is turned off, skip the rest. if (!Filter.CheckCStringOutOfBounds) - return state; + return State; // Get the access length and make sure it is known. // FIXME: This assumes the caller has already checked that the access length // is positive. And that it's unsigned. - SVal LengthVal = C.getSVal(Size); + SVal LengthVal = C.getSVal(Size.Expression); Optional Length = LengthVal.getAs(); if (!Length) - return state; + return State; // Compute the offset of the last element to be accessed: size-1. - NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs(); - SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy); + NonLoc One = svalBuilder.makeIntVal(1, SizeTy).castAs(); + SVal Offset = svalBuilder.evalBinOpNN(State, BO_Sub, *Length, One, SizeTy); if (Offset.isUnknown()) return nullptr; NonLoc LastOffset = Offset.castAs(); // Check that the first buffer is sufficiently long. - SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType()); + SVal BufStart = + svalBuilder.evalCast(BufVal, PtrTy, Buffer.Expression->getType()); if (Optional BufLoc = BufStart.getAs()) { - const Expr *warningExpr = (WarnAboutSize ? Size : FirstBuf); - SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc, - LastOffset, PtrTy); - state = CheckLocation(C, state, warningExpr, BufEnd, firstMessage); + SVal BufEnd = + svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy); - // If the buffer isn't large enough, abort. - if (!state) - return nullptr; - } + State = CheckLocation(C, State, Buffer, BufEnd, Access); - // If there's a second buffer, check it as well. - if (SecondBuf) { - BufVal = state->getSVal(SecondBuf, LCtx); - state = checkNonNull(C, state, SecondBuf, BufVal, 2); - if (!state) + // If the buffer isn't large enough, abort. + if (!State) return nullptr; - - BufStart = svalBuilder.evalCast(BufVal, PtrTy, SecondBuf->getType()); - if (Optional BufLoc = BufStart.getAs()) { - const Expr *warningExpr = (WarnAboutSize ? Size : SecondBuf); - - SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc, - LastOffset, PtrTy); - state = CheckLocation(C, state, warningExpr, BufEnd, secondMessage); - } } // Large enough or not, return this state! - return state; + return State; } ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, - ProgramStateRef state, - const Expr *Size, - const Expr *First, - const Expr *Second) const { + ProgramStateRef state, + SizeArgExpr Size, AnyArgExpr First, + AnyArgExpr Second) const { if (!Filter.CheckCStringBufferOverlap) return state; @@ -464,8 +450,8 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, // Get the buffer values and make sure they're known locations. const LocationContext *LCtx = C.getLocationContext(); - SVal firstVal = state->getSVal(First, LCtx); - SVal secondVal = state->getSVal(Second, LCtx); + SVal firstVal = state->getSVal(First.Expression, LCtx); + SVal secondVal = state->getSVal(Second.Expression, LCtx); Optional firstLoc = firstVal.getAs(); if (!firstLoc) @@ -478,11 +464,11 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, // Are the two values the same? SValBuilder &svalBuilder = C.getSValBuilder(); std::tie(stateTrue, stateFalse) = - state->assume(svalBuilder.evalEQ(state, *firstLoc, *secondLoc)); + state->assume(svalBuilder.evalEQ(state, *firstLoc, *secondLoc)); if (stateTrue && !stateFalse) { // If the values are known to be equal, that's automatically an overlap. - emitOverlapBug(C, stateTrue, First, Second); + emitOverlapBug(C, stateTrue, First.Expression, Second.Expression); return nullptr; } @@ -492,8 +478,8 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, // Which value comes first? QualType cmpTy = svalBuilder.getConditionType(); - SVal reverse = svalBuilder.evalBinOpLL(state, BO_GT, - *firstLoc, *secondLoc, cmpTy); + SVal reverse = + svalBuilder.evalBinOpLL(state, BO_GT, *firstLoc, *secondLoc, cmpTy); Optional reverseTest = reverse.getAs(); if (!reverseTest) @@ -514,7 +500,7 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, } // Get the length, and make sure it too is known. - SVal LengthVal = state->getSVal(Size, LCtx); + SVal LengthVal = state->getSVal(Size.Expression, LCtx); Optional Length = LengthVal.getAs(); if (!Length) return state; @@ -523,22 +509,22 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, // Bail out if the cast fails. ASTContext &Ctx = svalBuilder.getContext(); QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy); - SVal FirstStart = svalBuilder.evalCast(*firstLoc, CharPtrTy, - First->getType()); + SVal FirstStart = + svalBuilder.evalCast(*firstLoc, CharPtrTy, First.Expression->getType()); Optional FirstStartLoc = FirstStart.getAs(); if (!FirstStartLoc) return state; // Compute the end of the first buffer. Bail out if THAT fails. - SVal FirstEnd = svalBuilder.evalBinOpLN(state, BO_Add, - *FirstStartLoc, *Length, CharPtrTy); + SVal FirstEnd = svalBuilder.evalBinOpLN(state, BO_Add, *FirstStartLoc, + *Length, CharPtrTy); Optional FirstEndLoc = FirstEnd.getAs(); if (!FirstEndLoc) return state; // Is the end of the first buffer past the start of the second buffer? - SVal Overlap = svalBuilder.evalBinOpLL(state, BO_GT, - *FirstEndLoc, *secondLoc, cmpTy); + SVal Overlap = + svalBuilder.evalBinOpLL(state, BO_GT, *FirstEndLoc, *secondLoc, cmpTy); Optional OverlapTest = Overlap.getAs(); if (!OverlapTest) @@ -548,7 +534,7 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, if (stateTrue && !stateFalse) { // Overlap! - emitOverlapBug(C, stateTrue, First, Second); + emitOverlapBug(C, stateTrue, First.Expression, Second.Expression); return nullptr; } @@ -1131,25 +1117,24 @@ bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal, // evaluation of individual function calls. //===----------------------------------------------------------------------===// -void CStringChecker::evalCopyCommon(CheckerContext &C, - const CallExpr *CE, - ProgramStateRef state, - const Expr *Size, const Expr *Dest, - const Expr *Source, bool Restricted, +void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE, + ProgramStateRef state, SizeArgExpr Size, + DestinationArgExpr Dest, + SourceArgExpr Source, bool Restricted, bool IsMempcpy) const { CurrentFunctionDescription = "memory copy function"; // See if the size argument is zero. const LocationContext *LCtx = C.getLocationContext(); - SVal sizeVal = state->getSVal(Size, LCtx); - QualType sizeTy = Size->getType(); + SVal sizeVal = state->getSVal(Size.Expression, LCtx); + QualType sizeTy = Size.Expression->getType(); ProgramStateRef stateZeroSize, stateNonZeroSize; std::tie(stateZeroSize, stateNonZeroSize) = - assumeZero(C, state, sizeVal, sizeTy); + assumeZero(C, state, sizeVal, sizeTy); // Get the value of the Dest. - SVal destVal = state->getSVal(Dest, LCtx); + SVal destVal = state->getSVal(Dest.Expression, LCtx); // If the size is zero, there won't be any actual memory access, so // just bind the return value to the destination buffer and return. @@ -1165,24 +1150,23 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, // Ensure the destination is not null. If it is NULL there will be a // NULL pointer dereference. - state = checkNonNull(C, state, Dest, destVal, 1); + state = checkNonNull(C, state, Dest, destVal); if (!state) return; // Get the value of the Src. - SVal srcVal = state->getSVal(Source, LCtx); + SVal srcVal = state->getSVal(Source.Expression, LCtx); // Ensure the source is not null. If it is NULL there will be a // NULL pointer dereference. - state = checkNonNull(C, state, Source, srcVal, 2); + state = checkNonNull(C, state, Source, srcVal); if (!state) return; // Ensure the accesses are valid and that the buffers do not overlap. - const char * const writeWarning = - "Memory copy function overflows destination buffer"; - state = CheckBufferAccess(C, state, Size, Dest, Source, - writeWarning, /* sourceWarning = */ nullptr); + state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write); + state = CheckBufferAccess(C, state, Source, Size, AccessKind::read); + if (Restricted) state = CheckOverlap(C, state, Size, Dest, Source); @@ -1197,9 +1181,9 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, ASTContext &Ctx = SvalBuilder.getContext(); QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy); SVal DestRegCharVal = - SvalBuilder.evalCast(destVal, CharPtrTy, Dest->getType()); + SvalBuilder.evalCast(destVal, CharPtrTy, Dest.Expression->getType()); SVal lastElement = C.getSValBuilder().evalBinOp( - state, BO_Add, DestRegCharVal, sizeVal, Dest->getType()); + state, BO_Add, DestRegCharVal, sizeVal, Dest.Expression->getType()); // If we don't know how much we copied, we can at least // conjure a return value for later. if (lastElement.isUnknown()) @@ -1220,120 +1204,136 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, // can use LazyCompoundVals to copy the source values into the destination. // This would probably remove any existing bindings past the end of the // copied region, but that's still an improvement over blank invalidation. - state = InvalidateBuffer(C, state, Dest, C.getSVal(Dest), - /*IsSourceBuffer*/false, Size); + state = + InvalidateBuffer(C, state, Dest.Expression, C.getSVal(Dest.Expression), + /*IsSourceBuffer*/ false, Size.Expression); // Invalidate the source (const-invalidation without const-pointer-escaping // the address of the top-level region). - state = InvalidateBuffer(C, state, Source, C.getSVal(Source), - /*IsSourceBuffer*/true, nullptr); + state = InvalidateBuffer(C, state, Source.Expression, + C.getSVal(Source.Expression), + /*IsSourceBuffer*/ true, nullptr); C.addTransition(state); } } - void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const { // void *memcpy(void *restrict dst, const void *restrict src, size_t n); // The return value is the address of the destination buffer. - const Expr *Dest = CE->getArg(0); - ProgramStateRef state = C.getState(); + DestinationArgExpr Dest = {CE->getArg(0), 0}; + SourceArgExpr Src = {CE->getArg(1), 1}; + SizeArgExpr Size = {CE->getArg(2), 2}; - evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1), true); + ProgramStateRef State = C.getState(); + + constexpr bool IsRestricted = true; + constexpr bool IsMempcpy = false; + evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy); } void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const { // void *mempcpy(void *restrict dst, const void *restrict src, size_t n); // The return value is a pointer to the byte following the last written byte. - const Expr *Dest = CE->getArg(0); - ProgramStateRef state = C.getState(); + DestinationArgExpr Dest = {CE->getArg(0), 0}; + SourceArgExpr Src = {CE->getArg(1), 1}; + SizeArgExpr Size = {CE->getArg(2), 2}; - evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1), true, true); + constexpr bool IsRestricted = true; + constexpr bool IsMempcpy = true; + evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy); } void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const { // void *memmove(void *dst, const void *src, size_t n); // The return value is the address of the destination buffer. - const Expr *Dest = CE->getArg(0); - ProgramStateRef state = C.getState(); + DestinationArgExpr Dest = {CE->getArg(0), 0}; + SourceArgExpr Src = {CE->getArg(1), 1}; + SizeArgExpr Size = {CE->getArg(2), 2}; - evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1)); + constexpr bool IsRestricted = false; + constexpr bool IsMempcpy = false; + evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy); } void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const { // void bcopy(const void *src, void *dst, size_t n); - evalCopyCommon(C, CE, C.getState(), - CE->getArg(2), CE->getArg(1), CE->getArg(0)); + SourceArgExpr Src(CE->getArg(0), 0); + DestinationArgExpr Dest = {CE->getArg(1), 1}; + SizeArgExpr Size = {CE->getArg(2), 2}; + + constexpr bool IsRestricted = false; + constexpr bool IsMempcpy = false; + evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy); } void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const { // int memcmp(const void *s1, const void *s2, size_t n); CurrentFunctionDescription = "memory comparison function"; - const Expr *Left = CE->getArg(0); - const Expr *Right = CE->getArg(1); - const Expr *Size = CE->getArg(2); + AnyArgExpr Left = {CE->getArg(0), 0}; + AnyArgExpr Right = {CE->getArg(1), 1}; + SizeArgExpr Size = {CE->getArg(2), 2}; - ProgramStateRef state = C.getState(); - SValBuilder &svalBuilder = C.getSValBuilder(); + ProgramStateRef State = C.getState(); + SValBuilder &Builder = C.getSValBuilder(); + const LocationContext *LCtx = C.getLocationContext(); // See if the size argument is zero. - const LocationContext *LCtx = C.getLocationContext(); - SVal sizeVal = state->getSVal(Size, LCtx); - QualType sizeTy = Size->getType(); + SVal sizeVal = State->getSVal(Size.Expression, LCtx); + QualType sizeTy = Size.Expression->getType(); ProgramStateRef stateZeroSize, stateNonZeroSize; std::tie(stateZeroSize, stateNonZeroSize) = - assumeZero(C, state, sizeVal, sizeTy); + assumeZero(C, State, sizeVal, sizeTy); // If the size can be zero, the result will be 0 in that case, and we don't // have to check either of the buffers. if (stateZeroSize) { - state = stateZeroSize; - state = state->BindExpr(CE, LCtx, - svalBuilder.makeZeroVal(CE->getType())); - C.addTransition(state); + State = stateZeroSize; + State = State->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType())); + C.addTransition(State); } // If the size can be nonzero, we have to check the other arguments. if (stateNonZeroSize) { - state = stateNonZeroSize; + State = stateNonZeroSize; // If we know the two buffers are the same, we know the result is 0. // First, get the two buffers' addresses. Another checker will have already // made sure they're not undefined. DefinedOrUnknownSVal LV = - state->getSVal(Left, LCtx).castAs(); + State->getSVal(Left.Expression, LCtx).castAs(); DefinedOrUnknownSVal RV = - state->getSVal(Right, LCtx).castAs(); + State->getSVal(Right.Expression, LCtx).castAs(); // See if they are the same. - DefinedOrUnknownSVal SameBuf = svalBuilder.evalEQ(state, LV, RV); - ProgramStateRef StSameBuf, StNotSameBuf; - std::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf); + ProgramStateRef SameBuffer, NotSameBuffer; + std::tie(SameBuffer, NotSameBuffer) = + State->assume(Builder.evalEQ(State, LV, RV)); // If the two arguments are the same buffer, we know the result is 0, // and we only need to check one size. - if (StSameBuf && !StNotSameBuf) { - state = StSameBuf; - state = CheckBufferAccess(C, state, Size, Left); - if (state) { - state = StSameBuf->BindExpr(CE, LCtx, - svalBuilder.makeZeroVal(CE->getType())); - C.addTransition(state); + if (SameBuffer && !NotSameBuffer) { + State = SameBuffer; + State = CheckBufferAccess(C, State, Left, Size, AccessKind::read); + if (State) { + State = + SameBuffer->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType())); + C.addTransition(State); } return; } // If the two arguments might be different buffers, we have to check // the size of both of them. - assert(StNotSameBuf); - state = CheckBufferAccess(C, state, Size, Left, Right); - if (state) { + assert(NotSameBuffer); + State = CheckBufferAccess(C, State, Right, Size, AccessKind::read); + State = CheckBufferAccess(C, State, Left, Size, AccessKind::read); + if (State) { // The return value is the comparison result, which we don't know. - SVal CmpV = - svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()); - state = state->BindExpr(CE, LCtx, CmpV); - C.addTransition(state); + SVal CmpV = Builder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()); + State = State->BindExpr(CE, LCtx, CmpV); + C.addTransition(State); } } } @@ -1381,15 +1381,14 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE, } // Check that the string argument is non-null. - const Expr *Arg = CE->getArg(0); - SVal ArgVal = state->getSVal(Arg, LCtx); - - state = checkNonNull(C, state, Arg, ArgVal, 1); + AnyArgExpr Arg = {CE->getArg(0), 0}; + SVal ArgVal = state->getSVal(Arg.Expression, LCtx); + state = checkNonNull(C, state, Arg, ArgVal); if (!state) return; - SVal strLength = getCStringLength(C, state, Arg, ArgVal); + SVal strLength = getCStringLength(C, state, Arg.Expression, ArgVal); // If the argument isn't a valid C string, there's no valid state to // transition to. @@ -1537,30 +1536,30 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, CurrentFunctionDescription = "string copy function"; else CurrentFunctionDescription = "string concatenation function"; + ProgramStateRef state = C.getState(); const LocationContext *LCtx = C.getLocationContext(); // Check that the destination is non-null. - const Expr *Dst = CE->getArg(0); - SVal DstVal = state->getSVal(Dst, LCtx); - - state = checkNonNull(C, state, Dst, DstVal, 1); + DestinationArgExpr Dst = {CE->getArg(0), 0}; + SVal DstVal = state->getSVal(Dst.Expression, LCtx); + state = checkNonNull(C, state, Dst, DstVal); if (!state) return; // Check that the source is non-null. - const Expr *srcExpr = CE->getArg(1); - SVal srcVal = state->getSVal(srcExpr, LCtx); - state = checkNonNull(C, state, srcExpr, srcVal, 2); + SourceArgExpr srcExpr = {CE->getArg(1), 1}; + SVal srcVal = state->getSVal(srcExpr.Expression, LCtx); + state = checkNonNull(C, state, srcExpr, srcVal); if (!state) return; // Get the string length of the source. - SVal strLength = getCStringLength(C, state, srcExpr, srcVal); + SVal strLength = getCStringLength(C, state, srcExpr.Expression, srcVal); Optional strLengthNL = strLength.getAs(); // Get the string length of the destination buffer. - SVal dstStrLength = getCStringLength(C, state, Dst, DstVal); + SVal dstStrLength = getCStringLength(C, state, Dst.Expression, DstVal); Optional dstStrLengthNL = dstStrLength.getAs(); // If the source isn't a valid C string, give up. @@ -1578,8 +1577,13 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, SVal maxLastElementIndex = UnknownVal(); const char *boundWarning = nullptr; - state = CheckOverlap(C, state, IsBounded ? CE->getArg(2) : CE->getArg(1), Dst, - srcExpr); + // FIXME: Why do we choose the srcExpr if the access has no size? + // Note that the 3rd argument of the call would be the size parameter. + SizeArgExpr SrcExprAsSizeDummy = {srcExpr.Expression, srcExpr.ArgumentIndex}; + state = CheckOverlap( + C, state, + (IsBounded ? SizeArgExpr{CE->getArg(2), 2} : SrcExprAsSizeDummy), Dst, + srcExpr); if (!state) return; @@ -1587,11 +1591,12 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, // If the function is strncpy, strncat, etc... it is bounded. if (IsBounded) { // Get the max number of characters to copy. - const Expr *lenExpr = CE->getArg(2); - SVal lenVal = state->getSVal(lenExpr, LCtx); + SizeArgExpr lenExpr = {CE->getArg(2), 2}; + SVal lenVal = state->getSVal(lenExpr.Expression, LCtx); // Protect against misdeclared strncpy(). - lenVal = svalBuilder.evalCast(lenVal, sizeTy, lenExpr->getType()); + lenVal = + svalBuilder.evalCast(lenVal, sizeTy, lenExpr.Expression->getType()); Optional lenValNL = lenVal.getAs(); @@ -1834,19 +1839,17 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, // record the new string length. if (Optional dstRegVal = DstVal.getAs()) { - QualType ptrTy = Dst->getType(); + QualType ptrTy = Dst.Expression->getType(); // If we have an exact value on a bounded copy, use that to check for // overflows, rather than our estimate about how much is actually copied. - if (boundWarning) { - if (Optional maxLastNL = maxLastElementIndex.getAs()) { - SVal maxLastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal, - *maxLastNL, ptrTy); - state = CheckLocation(C, state, CE->getArg(2), maxLastElement, - boundWarning); - if (!state) - return; - } + if (Optional maxLastNL = maxLastElementIndex.getAs()) { + SVal maxLastElement = + svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal, *maxLastNL, ptrTy); + + state = CheckLocation(C, state, Dst, maxLastElement, AccessKind::write); + if (!state) + return; } // Then, if the final length is known... @@ -1856,9 +1859,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, // ...and we haven't checked the bound, we'll check the actual copy. if (!boundWarning) { - const char * const warningMsg = - "String copy function overflows destination buffer"; - state = CheckLocation(C, state, Dst, lastElement, warningMsg); + state = CheckLocation(C, state, Dst, lastElement, AccessKind::write); if (!state) return; } @@ -1875,13 +1876,13 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, // can use LazyCompoundVals to copy the source values into the destination. // This would probably remove any existing bindings past the end of the // string, but that's still an improvement over blank invalidation. - state = InvalidateBuffer(C, state, Dst, *dstRegVal, - /*IsSourceBuffer*/false, nullptr); + state = InvalidateBuffer(C, state, Dst.Expression, *dstRegVal, + /*IsSourceBuffer*/ false, nullptr); // Invalidate the source (const-invalidation without const-pointer-escaping // the address of the top-level region). - state = InvalidateBuffer(C, state, srcExpr, srcVal, /*IsSourceBuffer*/true, - nullptr); + state = InvalidateBuffer(C, state, srcExpr.Expression, srcVal, + /*IsSourceBuffer*/ true, nullptr); // Set the C string length of the destination, if we know it. if (IsBounded && (appendK == ConcatFnKind::none)) { @@ -1938,34 +1939,34 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, const LocationContext *LCtx = C.getLocationContext(); // Check that the first string is non-null - const Expr *s1 = CE->getArg(0); - SVal s1Val = state->getSVal(s1, LCtx); - state = checkNonNull(C, state, s1, s1Val, 1); + AnyArgExpr Left = {CE->getArg(0), 0}; + SVal LeftVal = state->getSVal(Left.Expression, LCtx); + state = checkNonNull(C, state, Left, LeftVal); if (!state) return; // Check that the second string is non-null. - const Expr *s2 = CE->getArg(1); - SVal s2Val = state->getSVal(s2, LCtx); - state = checkNonNull(C, state, s2, s2Val, 2); + AnyArgExpr Right = {CE->getArg(1), 1}; + SVal RightVal = state->getSVal(Right.Expression, LCtx); + state = checkNonNull(C, state, Right, RightVal); if (!state) return; // Get the string length of the first string or give up. - SVal s1Length = getCStringLength(C, state, s1, s1Val); - if (s1Length.isUndef()) + SVal LeftLength = getCStringLength(C, state, Left.Expression, LeftVal); + if (LeftLength.isUndef()) return; // Get the string length of the second string or give up. - SVal s2Length = getCStringLength(C, state, s2, s2Val); - if (s2Length.isUndef()) + SVal RightLength = getCStringLength(C, state, Right.Expression, RightVal); + if (RightLength.isUndef()) return; // If we know the two buffers are the same, we know the result is 0. // First, get the two buffers' addresses. Another checker will have already // made sure they're not undefined. - DefinedOrUnknownSVal LV = s1Val.castAs(); - DefinedOrUnknownSVal RV = s2Val.castAs(); + DefinedOrUnknownSVal LV = LeftVal.castAs(); + DefinedOrUnknownSVal RV = RightVal.castAs(); // See if they are the same. SValBuilder &svalBuilder = C.getSValBuilder(); @@ -1992,15 +1993,17 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, // For now, we only do this if they're both known string literals. // Attempt to extract string literals from both expressions. - const StringLiteral *s1StrLiteral = getCStringLiteral(C, state, s1, s1Val); - const StringLiteral *s2StrLiteral = getCStringLiteral(C, state, s2, s2Val); + const StringLiteral *LeftStrLiteral = + getCStringLiteral(C, state, Left.Expression, LeftVal); + const StringLiteral *RightStrLiteral = + getCStringLiteral(C, state, Right.Expression, RightVal); bool canComputeResult = false; SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()); - if (s1StrLiteral && s2StrLiteral) { - StringRef s1StrRef = s1StrLiteral->getString(); - StringRef s2StrRef = s2StrLiteral->getString(); + if (LeftStrLiteral && RightStrLiteral) { + StringRef LeftStrRef = LeftStrLiteral->getString(); + StringRef RightStrRef = RightStrLiteral->getString(); if (IsBounded) { // Get the max number of characters to compare. @@ -2010,8 +2013,8 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, // If the length is known, we can get the right substrings. if (const llvm::APSInt *len = svalBuilder.getKnownValue(state, lenVal)) { // Create substrings of each to compare the prefix. - s1StrRef = s1StrRef.substr(0, (size_t)len->getZExtValue()); - s2StrRef = s2StrRef.substr(0, (size_t)len->getZExtValue()); + LeftStrRef = LeftStrRef.substr(0, (size_t)len->getZExtValue()); + RightStrRef = RightStrRef.substr(0, (size_t)len->getZExtValue()); canComputeResult = true; } } else { @@ -2021,17 +2024,17 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, if (canComputeResult) { // Real strcmp stops at null characters. - size_t s1Term = s1StrRef.find('\0'); + size_t s1Term = LeftStrRef.find('\0'); if (s1Term != StringRef::npos) - s1StrRef = s1StrRef.substr(0, s1Term); + LeftStrRef = LeftStrRef.substr(0, s1Term); - size_t s2Term = s2StrRef.find('\0'); + size_t s2Term = RightStrRef.find('\0'); if (s2Term != StringRef::npos) - s2StrRef = s2StrRef.substr(0, s2Term); + RightStrRef = RightStrRef.substr(0, s2Term); // Use StringRef's comparison methods to compute the actual result. - int compareRes = IgnoreCase ? s1StrRef.compare_lower(s2StrRef) - : s1StrRef.compare(s2StrRef); + int compareRes = IgnoreCase ? LeftStrRef.compare_lower(RightStrRef) + : LeftStrRef.compare(RightStrRef); // The strcmp function returns an integer greater than, equal to, or less // than zero, [c11, p7.24.4.2]. @@ -2061,8 +2064,9 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { //char *strsep(char **stringp, const char *delim); // Sanity: does the search string parameter match the return type? - const Expr *SearchStrPtr = CE->getArg(0); - QualType CharPtrTy = SearchStrPtr->getType()->getPointeeType(); + SourceArgExpr SearchStrPtr = {CE->getArg(0), 0}; + + QualType CharPtrTy = SearchStrPtr.Expression->getType()->getPointeeType(); if (CharPtrTy.isNull() || CE->getType().getUnqualifiedType() != CharPtrTy.getUnqualifiedType()) return; @@ -2073,15 +2077,15 @@ void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { // Check that the search string pointer is non-null (though it may point to // a null string). - SVal SearchStrVal = State->getSVal(SearchStrPtr, LCtx); - State = checkNonNull(C, State, SearchStrPtr, SearchStrVal, 1); + SVal SearchStrVal = State->getSVal(SearchStrPtr.Expression, LCtx); + State = checkNonNull(C, State, SearchStrPtr, SearchStrVal); if (!State) return; // Check that the delimiter string is non-null. - const Expr *DelimStr = CE->getArg(1); - SVal DelimStrVal = State->getSVal(DelimStr, LCtx); - State = checkNonNull(C, State, DelimStr, DelimStrVal, 2); + AnyArgExpr DelimStr = {CE->getArg(1), 1}; + SVal DelimStrVal = State->getSVal(DelimStr.Expression, LCtx); + State = checkNonNull(C, State, DelimStr, DelimStrVal); if (!State) return; @@ -2093,8 +2097,8 @@ void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { // Invalidate the search string, representing the change of one delimiter // character to NUL. - State = InvalidateBuffer(C, State, SearchStrPtr, Result, - /*IsSourceBuffer*/false, nullptr); + State = InvalidateBuffer(C, State, SearchStrPtr.Expression, Result, + /*IsSourceBuffer*/ false, nullptr); // Overwrite the search string pointer. The new value is either an address // further along in the same string, or NULL if there are no more tokens. @@ -2155,65 +2159,67 @@ void CStringChecker::evalStdCopyCommon(CheckerContext &C, } void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const { + // void *memset(void *s, int c, size_t n); CurrentFunctionDescription = "memory set function"; - const Expr *Mem = CE->getArg(0); - const Expr *CharE = CE->getArg(1); - const Expr *Size = CE->getArg(2); + DestinationArgExpr Buffer = {CE->getArg(0), 0}; + AnyArgExpr CharE = {CE->getArg(1), 1}; + SizeArgExpr Size = {CE->getArg(2), 2}; + ProgramStateRef State = C.getState(); // See if the size argument is zero. const LocationContext *LCtx = C.getLocationContext(); - SVal SizeVal = State->getSVal(Size, LCtx); - QualType SizeTy = Size->getType(); + SVal SizeVal = C.getSVal(Size.Expression); + QualType SizeTy = Size.Expression->getType(); - ProgramStateRef StateZeroSize, StateNonZeroSize; - std::tie(StateZeroSize, StateNonZeroSize) = - assumeZero(C, State, SizeVal, SizeTy); + ProgramStateRef ZeroSize, NonZeroSize; + std::tie(ZeroSize, NonZeroSize) = assumeZero(C, State, SizeVal, SizeTy); // Get the value of the memory area. - SVal MemVal = State->getSVal(Mem, LCtx); + SVal BufferPtrVal = C.getSVal(Buffer.Expression); // If the size is zero, there won't be any actual memory access, so - // just bind the return value to the Mem buffer and return. - if (StateZeroSize && !StateNonZeroSize) { - StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, MemVal); - C.addTransition(StateZeroSize); + // just bind the return value to the buffer and return. + if (ZeroSize && !NonZeroSize) { + ZeroSize = ZeroSize->BindExpr(CE, LCtx, BufferPtrVal); + C.addTransition(ZeroSize); return; } // Ensure the memory area is not null. // If it is NULL there will be a NULL pointer dereference. - State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1); + State = checkNonNull(C, NonZeroSize, Buffer, BufferPtrVal); if (!State) return; - State = CheckBufferAccess(C, State, Size, Mem); + State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write); if (!State) return; // According to the values of the arguments, bind the value of the second // argument to the destination buffer and set string length, or just // invalidate the destination buffer. - if (!memsetAux(Mem, C.getSVal(CharE), Size, C, State)) + if (!memsetAux(Buffer.Expression, C.getSVal(CharE.Expression), + Size.Expression, C, State)) return; - State = State->BindExpr(CE, LCtx, MemVal); + State = State->BindExpr(CE, LCtx, BufferPtrVal); C.addTransition(State); } void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const { CurrentFunctionDescription = "memory clearance function"; - const Expr *Mem = CE->getArg(0); - const Expr *Size = CE->getArg(1); + DestinationArgExpr Buffer = {CE->getArg(0), 0}; + SizeArgExpr Size = {CE->getArg(1), 1}; SVal Zero = C.getSValBuilder().makeZeroVal(C.getASTContext().IntTy); ProgramStateRef State = C.getState(); // See if the size argument is zero. - SVal SizeVal = C.getSVal(Size); - QualType SizeTy = Size->getType(); + SVal SizeVal = C.getSVal(Size.Expression); + QualType SizeTy = Size.Expression->getType(); ProgramStateRef StateZeroSize, StateNonZeroSize; std::tie(StateZeroSize, StateNonZeroSize) = @@ -2227,19 +2233,19 @@ void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const { } // Get the value of the memory area. - SVal MemVal = C.getSVal(Mem); + SVal MemVal = C.getSVal(Buffer.Expression); // Ensure the memory area is not null. // If it is NULL there will be a NULL pointer dereference. - State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1); + State = checkNonNull(C, StateNonZeroSize, Buffer, MemVal); if (!State) return; - State = CheckBufferAccess(C, State, Size, Mem); + State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write); if (!State) return; - if (!memsetAux(Mem, Zero, Size, C, State)) + if (!memsetAux(Buffer.Expression, Zero, Size.Expression, C, State)) return; C.addTransition(State); diff --git a/clang/test/Analysis/bsd-string.c b/clang/test/Analysis/bsd-string.c index adb8721..e411905 100644 --- a/clang/test/Analysis/bsd-string.c +++ b/clang/test/Analysis/bsd-string.c @@ -29,7 +29,7 @@ void f2() { void f3() { char dst[2]; const char *src = "abdef"; - strlcpy(dst, src, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}} + strlcpy(dst, src, 5); // expected-warning{{String copy function overflows the destination buffer}} } void f4() { @@ -112,7 +112,8 @@ void f9(int unknown_size, char* unknown_src, char* unknown_dst){ clang_analyzer_eval(strlen(unknown_dst));// expected-warning{{UNKNOWN}} //size is unknown - len = strlcat(buf+2,unknown_src+1, sizeof(buf));// expected-warning{{Size argument is greater than the length of the destination buffer}}; + len = strlcat(buf + 2, unknown_src + 1, sizeof(buf)); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } void f10(){ @@ -121,7 +122,8 @@ void f10(){ len = strlcpy(buf,"abba",sizeof(buf)); clang_analyzer_eval(len==4);// expected-warning{{TRUE}} - strlcat(buf, "efghi",9);// expected-warning{{Size argument is greater than the length of the destination buffer}} + strlcat(buf, "efghi", 9); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } void f11() { diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c index 0c512ec..0deb475 100644 --- a/clang/test/Analysis/bstring.c +++ b/clang/test/Analysis/bstring.c @@ -95,9 +95,9 @@ void memcpy2 () { char src[] = {1, 2, 3, 4}; char dst[1]; - memcpy(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}} + memcpy(dst, src, 4); // expected-warning {{Memory copy function overflows the destination buffer}} #ifndef VARIANT - // expected-warning@-2{{memcpy' will always overflow; destination buffer has size 1, but size argument is 4}} + // expected-warning@-2 {{memcpy' will always overflow; destination buffer has size 1, but size argument is 4}} #endif } @@ -119,7 +119,7 @@ void memcpy5() { char src[] = {1, 2, 3, 4}; char dst[3]; - memcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}} + memcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}} #ifndef VARIANT // expected-warning@-2{{memcpy' will always overflow; destination buffer has size 1, but size argument is 2}} #endif @@ -221,7 +221,7 @@ void mempcpy2 () { char src[] = {1, 2, 3, 4}; char dst[1]; - mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}} + mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}} #ifndef VARIANT // expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 4}} #endif @@ -245,7 +245,7 @@ void mempcpy5() { char src[] = {1, 2, 3, 4}; char dst[3]; - mempcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}} + mempcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}} #ifndef VARIANT // expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 2}} #endif @@ -386,7 +386,7 @@ void memmove2 () { char src[] = {1, 2, 3, 4}; char dst[1]; - memmove(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}} + memmove(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}} #ifndef VARIANT // expected-warning@-2{{memmove' will always overflow; destination buffer has size 1, but size argument is 4}} #endif diff --git a/clang/test/Analysis/null-deref-ps-region.c b/clang/test/Analysis/null-deref-ps-region.c index 58fcbb6..bb35944 100644 --- a/clang/test/Analysis/null-deref-ps-region.c +++ b/clang/test/Analysis/null-deref-ps-region.c @@ -55,12 +55,15 @@ void testHeapSymbol() { void testStackArrayOutOfBound() { char buf[1]; - memset(buf, 0, 1024); // expected-warning {{Memory set function accesses out-of-bound array element}} expected-warning {{'memset' will always overflow; destination buffer has size 1, but size argument is 1024}} + memset(buf, 0, 1024); + // expected-warning@-1 {{Memory set function overflows the destination buffer}} + // expected-warning@-2 {{'memset' will always overflow; destination buffer has size 1, but size argument is 1024}} } void testHeapSymbolOutOfBound() { char *buf = (char *)malloc(1); - memset(buf, 0, 1024); // expected-warning {{Memory set function accesses out-of-bound array element}} + memset(buf, 0, 1024); + // expected-warning@-1 {{Memory set function overflows the destination buffer}} free(buf); } diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c index 7612614..debcea48 100644 --- a/clang/test/Analysis/string.c +++ b/clang/test/Analysis/string.c @@ -353,7 +353,7 @@ void strcpy_effects(char *x, char *y) { void strcpy_overflow(char *y) { char x[4]; if (strlen(y) == 4) - strcpy(x, y); // expected-warning{{String copy function overflows destination buffer}} + strcpy(x, y); // expected-warning{{String copy function overflows the destination buffer}} } #endif @@ -394,7 +394,7 @@ void stpcpy_effect(char *x, char *y) { void stpcpy_overflow(char *y) { char x[4]; if (strlen(y) == 4) - stpcpy(x, y); // expected-warning{{String copy function overflows destination buffer}} + stpcpy(x, y); // expected-warning{{String copy function overflows the destination buffer}} } #endif @@ -451,19 +451,19 @@ void strcat_effects(char *y) { void strcat_overflow_0(char *y) { char x[4] = "12"; if (strlen(y) == 4) - strcat(x, y); // expected-warning{{String copy function overflows destination buffer}} + strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}} } void strcat_overflow_1(char *y) { char x[4] = "12"; if (strlen(y) == 3) - strcat(x, y); // expected-warning{{String copy function overflows destination buffer}} + strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}} } void strcat_overflow_2(char *y) { char x[4] = "12"; if (strlen(y) == 2) - strcat(x, y); // expected-warning{{String copy function overflows destination buffer}} + strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}} } #endif @@ -547,25 +547,28 @@ void strncpy_effects(char *x, char *y) { // of the C-string checker. void cstringchecker_bounds_nocrash() { char *p = malloc(2); - strncpy(p, "AAA", sizeof("AAA")); // expected-warning {{Size argument is greater than the length of the destination buffer}} + strncpy(p, "AAA", sizeof("AAA")); + // expected-warning@-1 {{String copy function overflows the destination buffer}} free(p); } void strncpy_overflow(char *y) { char x[4]; if (strlen(y) == 4) - strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}} + strncpy(x, y, 5); + // expected-warning@-1 {{String copy function overflows the destination buffer}} #ifndef VARIANT - // expected-warning@-2{{size argument is too large; destination buffer has size 4, but size argument is 5}} + // expected-warning@-3 {{size argument is too large; destination buffer has size 4, but size argument is 5}} #endif } void strncpy_no_overflow(char *y) { char x[4]; if (strlen(y) == 3) - strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}} + strncpy(x, y, 5); + // expected-warning@-1 {{String copy function overflows the destination buffer}} #ifndef VARIANT - // expected-warning@-2{{size argument is too large; destination buffer has size 4, but size argument is 5}} + // expected-warning@-3 {{size argument is too large; destination buffer has size 4, but size argument is 5}} #endif } @@ -575,7 +578,8 @@ void strncpy_no_overflow2(char *y, int n) { char x[4]; if (strlen(y) == 3) - strncpy(x, y, n); // expected-warning{{Size argument is greater than the length of the destination buffer}} + strncpy(x, y, n); + // expected-warning@-1 {{String copy function overflows the destination buffer}} } #endif @@ -658,25 +662,29 @@ void strncat_effects(char *y) { void strncat_overflow_0(char *y) { char x[4] = "12"; if (strlen(y) == 4) - strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}} + strncat(x, y, strlen(y)); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } void strncat_overflow_1(char *y) { char x[4] = "12"; if (strlen(y) == 3) - strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}} + strncat(x, y, strlen(y)); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } void strncat_overflow_2(char *y) { char x[4] = "12"; if (strlen(y) == 2) - strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}} + strncat(x, y, strlen(y)); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } void strncat_overflow_3(char *y) { char x[4] = "12"; if (strlen(y) == 4) - strncat(x, y, 2); // expected-warning{{Size argument is greater than the free space in the destination buffer}} + strncat(x, y, 2); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } #endif @@ -704,7 +712,8 @@ void strncat_symbolic_src_length(char *src) { clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}} char dst2[8] = "1234"; - strncat(dst2, src, 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}} + strncat(dst2, src, 4); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } void strncat_unknown_src_length(char *src, int offset) { @@ -713,7 +722,8 @@ void strncat_unknown_src_length(char *src, int offset) { clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}} char dst2[8] = "1234"; - strncat(dst2, &src[offset], 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}} + strncat(dst2, &src[offset], 4); + // expected-warning@-1 {{String concatenation function overflows the destination buffer}} } #endif @@ -1496,7 +1506,7 @@ void explicit_bzero3_out_ofbound() { strcpy(privkey, "random"); explicit_bzero(privkey, sizeof(newprivkey)); #ifndef SUPPRESS_OUT_OF_BOUND - // expected-warning@-2 {{Memory clearance function accesses out-of-bound array element}} + // expected-warning@-2 {{Memory clearance function overflows the destination buffer}} #endif clang_analyzer_eval(privkey[0] == '\0'); #ifdef SUPPRESS_OUT_OF_BOUND -- 2.7.4