[clang] 30e5c7e - [analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 9 07:07:16 PDT 2020
Author: Balazs Benics
Date: 2020-04-09T16:06:32+02:00
New Revision: 30e5c7e82fa1c5318540feb83d54757c632e2599
URL: https://github.com/llvm/llvm-project/commit/30e5c7e82fa1c5318540feb83d54757c632e2599
DIFF: https://github.com/llvm/llvm-project/commit/30e5c7e82fa1c5318540feb83d54757c632e2599.diff
LOG: [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
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/test/Analysis/bsd-string.c
clang/test/Analysis/bstring.c
clang/test/Analysis/null-deref-ps-region.c
clang/test/Analysis/string.c
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 4558b6936e7f..40467b346f69 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<DeclStmt>,
@@ -113,12 +154,9 @@ class CStringChecker : public Checker< eval::Call,
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 @@ class CStringChecker : public Checker< eval::Call,
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<ElementRegion>(R);
+ const auto *ER = dyn_cast<ElementRegion>(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<SubRegion>(ER->getSuperRegion());
+ const auto *superReg = cast<SubRegion>(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<NonLoc> Length = LengthVal.getAs<NonLoc>();
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<NonLoc>();
- SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+ NonLoc One = svalBuilder.makeIntVal(1, SizeTy).castAs<NonLoc>();
+ SVal Offset = svalBuilder.evalBinOpNN(State, BO_Sub, *Length, One, SizeTy);
if (Offset.isUnknown())
return nullptr;
NonLoc LastOffset = Offset.castAs<NonLoc>();
// 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<Loc> BufLoc = BufStart.getAs<Loc>()) {
- 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<Loc> BufLoc = BufStart.getAs<Loc>()) {
- 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<Loc> firstLoc = firstVal.getAs<Loc>();
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<DefinedOrUnknownSVal> reverseTest =
reverse.getAs<DefinedOrUnknownSVal>();
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<NonLoc> Length = LengthVal.getAs<NonLoc>();
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<Loc> FirstStartLoc = FirstStart.getAs<Loc>();
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<Loc> FirstEndLoc = FirstEnd.getAs<Loc>();
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<DefinedOrUnknownSVal> OverlapTest =
Overlap.getAs<DefinedOrUnknownSVal>();
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<DefinedOrUnknownSVal>();
+ State->getSVal(Left.Expression, LCtx).castAs<DefinedOrUnknownSVal>();
DefinedOrUnknownSVal RV =
- state->getSVal(Right, LCtx).castAs<DefinedOrUnknownSVal>();
+ State->getSVal(Right.Expression, LCtx).castAs<DefinedOrUnknownSVal>();
// 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
diff erent 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<NonLoc> strLengthNL = strLength.getAs<NonLoc>();
// Get the string length of the destination buffer.
- SVal dstStrLength = getCStringLength(C, state, Dst, DstVal);
+ SVal dstStrLength = getCStringLength(C, state, Dst.Expression, DstVal);
Optional<NonLoc> dstStrLengthNL = dstStrLength.getAs<NonLoc>();
// 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<NonLoc> lenValNL = lenVal.getAs<NonLoc>();
@@ -1834,19 +1839,17 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
// record the new string length.
if (Optional<loc::MemRegionVal> dstRegVal =
DstVal.getAs<loc::MemRegionVal>()) {
- 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<NonLoc> maxLastNL = maxLastElementIndex.getAs<NonLoc>()) {
- SVal maxLastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
- *maxLastNL, ptrTy);
- state = CheckLocation(C, state, CE->getArg(2), maxLastElement,
- boundWarning);
- if (!state)
- return;
- }
+ if (Optional<NonLoc> maxLastNL = maxLastElementIndex.getAs<NonLoc>()) {
+ 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>();
- DefinedOrUnknownSVal RV = s2Val.castAs<DefinedOrUnknownSVal>();
+ DefinedOrUnknownSVal LV = LeftVal.castAs<DefinedOrUnknownSVal>();
+ DefinedOrUnknownSVal RV = RightVal.castAs<DefinedOrUnknownSVal>();
// 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 adb8721c3fa2..e4119058507f 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 at -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 at -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 0c512ec6fd91..0deb4754c3b2 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 at -2{{memcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
+ // expected-warning at -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 at -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 at -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 at -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 at -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 58fcbb65f151..bb35944411bb 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 at -1 {{Memory set function overflows the destination buffer}}
+ // expected-warning at -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 at -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 7612614bc091..debcea481adb 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 at -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 at -1 {{String copy function overflows the destination buffer}}
#ifndef VARIANT
- // expected-warning at -2{{size argument is too large; destination buffer has size 4, but size argument is 5}}
+ // expected-warning at -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 at -1 {{String copy function overflows the destination buffer}}
#ifndef VARIANT
- // expected-warning at -2{{size argument is too large; destination buffer has size 4, but size argument is 5}}
+ // expected-warning at -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 at -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 at -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 at -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 at -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 at -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 at -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 at -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 at -2 {{Memory clearance function accesses out-of-bound array element}}
+ // expected-warning at -2 {{Memory clearance function overflows the destination buffer}}
#endif
clang_analyzer_eval(privkey[0] == '\0');
#ifdef SUPPRESS_OUT_OF_BOUND
More information about the cfe-commits
mailing list