diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2699,7 +2699,7 @@ alpha.unix.cstring.BufferOverlap (C) """""""""""""""""""""""""""""""""""" -Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy, wmemcpy``. +Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy, wmemcpy, wmempcpy``. .. code-block:: c diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -75,6 +75,16 @@ } enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 }; + +enum class CharKind { Regular = 0, Wide }; +constexpr CharKind CK_Regular = CharKind::Regular; +constexpr CharKind CK_Wide = CharKind::Wide; + +static QualType getCharPtrType(ASTContext &Ctx, CharKind CK) { + return Ctx.getPointerType(CK == CharKind::Regular ? Ctx.CharTy + : Ctx.WideCharTy); +} + class CStringChecker : public Checker< eval::Call, check::PreStmt, check::LiveSymbols, @@ -125,12 +135,21 @@ CallDescriptionMap Callbacks = { {{CDF_MaybeBuiltin, "memcpy", 3}, - std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, false)}, + std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Regular)}, {{CDF_MaybeBuiltin, "wmemcpy", 3}, - std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, true)}, - {{CDF_MaybeBuiltin, "mempcpy", 3}, &CStringChecker::evalMempcpy}, - {{CDF_MaybeBuiltin, "memcmp", 3}, &CStringChecker::evalMemcmp}, - {{CDF_MaybeBuiltin, "memmove", 3}, &CStringChecker::evalMemmove}, + std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Wide)}, + {{CDF_MaybeBuiltin, "mempcpy", 3}, + std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Regular)}, + {{CDF_None, "wmempcpy", 3}, + std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Wide)}, + {{CDF_MaybeBuiltin, "memcmp", 3}, + std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)}, + {{CDF_MaybeBuiltin, "wmemcmp", 3}, + std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Wide)}, + {{CDF_MaybeBuiltin, "memmove", 3}, + std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Regular)}, + {{CDF_MaybeBuiltin, "wmemmove", 3}, + std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Wide)}, {{CDF_MaybeBuiltin, "memset", 3}, &CStringChecker::evalMemset}, {{CDF_MaybeBuiltin, "explicit_memset", 3}, &CStringChecker::evalMemset}, {{CDF_MaybeBuiltin, "strcpy", 2}, &CStringChecker::evalStrcpy}, @@ -150,7 +169,8 @@ {{CDF_MaybeBuiltin, "strncasecmp", 3}, &CStringChecker::evalStrncasecmp}, {{CDF_MaybeBuiltin, "strsep", 2}, &CStringChecker::evalStrsep}, {{CDF_MaybeBuiltin, "bcopy", 3}, &CStringChecker::evalBcopy}, - {{CDF_MaybeBuiltin, "bcmp", 3}, &CStringChecker::evalMemcmp}, + {{CDF_MaybeBuiltin, "bcmp", 3}, + std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)}, {{CDF_MaybeBuiltin, "bzero", 2}, &CStringChecker::evalBzero}, {{CDF_MaybeBuiltin, "explicit_bzero", 2}, &CStringChecker::evalBzero}, }; @@ -160,16 +180,16 @@ StdCopyBackward{{"std", "copy_backward"}, 3}; FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const; - void evalMemcpy(CheckerContext &C, const CallExpr *CE, bool IsWide) const; - void evalMempcpy(CheckerContext &C, const CallExpr *CE) const; - void evalMemmove(CheckerContext &C, const CallExpr *CE) const; + void evalMemcpy(CheckerContext &C, const CallExpr *CE, CharKind CK) const; + void evalMempcpy(CheckerContext &C, const CallExpr *CE, CharKind CK) const; + void evalMemmove(CheckerContext &C, const CallExpr *CE, CharKind CK) const; void evalBcopy(CheckerContext &C, const CallExpr *CE) const; void evalCopyCommon(CheckerContext &C, const CallExpr *CE, ProgramStateRef state, SizeArgExpr Size, DestinationArgExpr Dest, SourceArgExpr Source, - bool Restricted, bool IsMempcpy, bool IsWide) const; + bool Restricted, bool IsMempcpy, CharKind CK) const; - void evalMemcmp(CheckerContext &C, const CallExpr *CE) const; + void evalMemcmp(CheckerContext &C, const CallExpr *CE, CharKind CK) const; void evalstrLength(CheckerContext &C, const CallExpr *CE) const; void evalstrnLength(CheckerContext &C, const CallExpr *CE) const; @@ -248,14 +268,16 @@ AnyArgExpr Arg, SVal l) const; ProgramStateRef CheckLocation(CheckerContext &C, ProgramStateRef state, AnyArgExpr Buffer, SVal Element, - AccessKind Access, bool IsWide = false) const; + AccessKind Access, + CharKind CK = CharKind::Regular) const; ProgramStateRef CheckBufferAccess(CheckerContext &C, ProgramStateRef State, AnyArgExpr Buffer, SizeArgExpr Size, AccessKind Access, - bool IsWide = false) const; + CharKind CK = CharKind::Regular) const; ProgramStateRef CheckOverlap(CheckerContext &C, ProgramStateRef state, SizeArgExpr Size, AnyArgExpr First, - AnyArgExpr Second, bool IsWide = false) const; + AnyArgExpr Second, + CharKind CK = CharKind::Regular) const; void emitOverlapBug(CheckerContext &C, ProgramStateRef state, const Stmt *First, @@ -339,7 +361,7 @@ ProgramStateRef state, AnyArgExpr Buffer, SVal Element, AccessKind Access, - bool IsWide) const { + CharKind CK) const { // If a previous check has failed, propagate the failure. if (!state) @@ -360,7 +382,7 @@ // Get the index of the accessed element. NonLoc Idx = ER->getIndex(); - if (!IsWide) { + if (CK == CharKind::Regular) { if (ER->getValueType() != Ctx.CharTy) return state; } else { @@ -417,7 +439,7 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C, ProgramStateRef State, AnyArgExpr Buffer, SizeArgExpr Size, - AccessKind Access, bool IsWide) const { + AccessKind Access, CharKind CK) const { // If a previous check has failed, propagate the failure. if (!State) return nullptr; @@ -426,7 +448,7 @@ ASTContext &Ctx = svalBuilder.getContext(); QualType SizeTy = Size.Expression->getType(); - QualType PtrTy = Ctx.getPointerType(IsWide ? Ctx.WideCharTy : Ctx.CharTy); + QualType PtrTy = getCharPtrType(Ctx, CK); // Check that the first buffer is non-null. SVal BufVal = C.getSVal(Buffer.Expression); @@ -460,7 +482,7 @@ SVal BufEnd = svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy); - State = CheckLocation(C, State, Buffer, BufEnd, Access, IsWide); + State = CheckLocation(C, State, Buffer, BufEnd, Access, CK); // If the buffer isn't large enough, abort. if (!State) @@ -475,7 +497,7 @@ ProgramStateRef state, SizeArgExpr Size, AnyArgExpr First, AnyArgExpr Second, - bool IsWide) const { + CharKind CK) const { if (!Filter.CheckCStringBufferOverlap) return state; @@ -554,7 +576,7 @@ // Convert the first buffer's start address to char*. // Bail out if the cast fails. ASTContext &Ctx = svalBuilder.getContext(); - QualType CharPtrTy = Ctx.getPointerType(IsWide ? Ctx.WideCharTy : Ctx.CharTy); + QualType CharPtrTy = getCharPtrType(Ctx, CK); SVal FirstStart = svalBuilder.evalCast(*firstLoc, CharPtrTy, First.Expression->getType()); Optional FirstStartLoc = FirstStart.getAs(); @@ -1190,7 +1212,7 @@ ProgramStateRef state, SizeArgExpr Size, DestinationArgExpr Dest, SourceArgExpr Source, bool Restricted, - bool IsMempcpy, bool IsWide) const { + bool IsMempcpy, CharKind CK) const { CurrentFunctionDescription = "memory copy function"; // See if the size argument is zero. @@ -1233,11 +1255,11 @@ return; // Ensure the accesses are valid and that the buffers do not overlap. - state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write, IsWide); - state = CheckBufferAccess(C, state, Source, Size, AccessKind::read, IsWide); + state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write, CK); + state = CheckBufferAccess(C, state, Source, Size, AccessKind::read, CK); if (Restricted) - state = CheckOverlap(C, state, Size, Dest, Source, IsWide); + state = CheckOverlap(C, state, Size, Dest, Source, CK); if (!state) return; @@ -1248,7 +1270,7 @@ // Get the byte after the last byte copied. SValBuilder &SvalBuilder = C.getSValBuilder(); ASTContext &Ctx = SvalBuilder.getContext(); - QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy); + QualType CharPtrTy = getCharPtrType(Ctx, CK); SVal DestRegCharVal = SvalBuilder.evalCast(destVal, CharPtrTy, Dest.Expression->getType()); SVal lastElement = C.getSValBuilder().evalBinOp( @@ -1288,7 +1310,7 @@ } void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE, - bool IsWide) const { + CharKind CK) const { // void *memcpy(void *restrict dst, const void *restrict src, size_t n); // The return value is the address of the destination buffer. DestinationArgExpr Dest = {CE->getArg(0), 0}; @@ -1299,11 +1321,11 @@ constexpr bool IsRestricted = true; constexpr bool IsMempcpy = false; - evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy, - IsWide); + evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy, CK); } -void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const { +void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE, + CharKind CK) 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. DestinationArgExpr Dest = {CE->getArg(0), 0}; @@ -1313,10 +1335,11 @@ constexpr bool IsRestricted = true; constexpr bool IsMempcpy = true; evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy, - false); + CK); } -void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const { +void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE, + CharKind CK) const { // void *memmove(void *dst, const void *src, size_t n); // The return value is the address of the destination buffer. DestinationArgExpr Dest = {CE->getArg(0), 0}; @@ -1326,7 +1349,7 @@ constexpr bool IsRestricted = false; constexpr bool IsMempcpy = false; evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy, - false); + CK); } void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const { @@ -1338,10 +1361,11 @@ constexpr bool IsRestricted = false; constexpr bool IsMempcpy = false; evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy, - false); + CharKind::Regular); } -void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const { +void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE, + CharKind CK) const { // int memcmp(const void *s1, const void *s2, size_t n); CurrentFunctionDescription = "memory comparison function"; @@ -1401,8 +1425,8 @@ // If the two arguments might be different buffers, we have to check // the size of both of them. assert(NotSameBuffer); - State = CheckBufferAccess(C, State, Right, Size, AccessKind::read); - State = CheckBufferAccess(C, State, Left, Size, AccessKind::read); + State = CheckBufferAccess(C, State, Right, Size, AccessKind::read, CK); + State = CheckBufferAccess(C, State, Left, Size, AccessKind::read, CK); if (State) { // The return value is the comparison result, which we don't know. SVal CmpV = Builder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()); diff --git a/clang/test/Analysis/wstring.c b/clang/test/Analysis/wstring.c --- a/clang/test/Analysis/wstring.c +++ b/clang/test/Analysis/wstring.c @@ -33,7 +33,7 @@ void clang_analyzer_eval(int); //===----------------------------------------------------------------------=== -// wwmemcpy() +// wmemcpy() //===----------------------------------------------------------------------=== #define wmemcpy BUILTIN(wmemcpy) @@ -139,6 +139,255 @@ clang_analyzer_eval(result == a); // no-warning (above is fatal) } +//===----------------------------------------------------------------------=== +// wmempcpy() +//===----------------------------------------------------------------------=== + +wchar_t *wmempcpy(wchar_t *restrict s1, const wchar_t *restrict s2, size_t n); + +void wmempcpy0 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[5] = {0}; + + wmempcpy(dst, src, 4); // no-warning + + clang_analyzer_eval(wmempcpy(dst, src, 4) == &dst[4]); // expected-warning{{TRUE}} + + // If we actually model the copy, we can make this known. + // The important thing for now is that the old value has been invalidated. + clang_analyzer_eval(dst[0] != 0); // expected-warning{{UNKNOWN}} +} + +void wmempcpy1 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[10]; + + wmempcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}} +} + +void wmempcpy2 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[1]; + + wmempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}} +} + +void wmempcpy3 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[3]; + + wmempcpy(dst+1, src+2, 2); // no-warning +} + +void wmempcpy4 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[10]; + + wmempcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}} +} + +void wmempcpy5(void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[3]; + + wmempcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}} +} + +void wmempcpy6(void) { + wchar_t a[4] = {0}; + wmempcpy(a, a, 2); // expected-warning{{overlapping}} +} + +void wmempcpy7(void) { + wchar_t a[4] = {0}; + wmempcpy(a+2, a+1, 2); // expected-warning{{overlapping}} +} + +void wmempcpy8(void) { + wchar_t a[4] = {0}; + wmempcpy(a+1, a+2, 2); // expected-warning{{overlapping}} +} + +void wmempcpy9(void) { + wchar_t a[4] = {0}; + wmempcpy(a+2, a+1, 1); // no-warning + wmempcpy(a+1, a+2, 1); // no-warning +} + +void wmempcpy10(void) { + wchar_t a[4] = {0}; + wmempcpy(0, a, 1); // expected-warning{{Null pointer passed as 1st argument to memory copy function}} +} + +void wmempcpy11(void) { + wchar_t a[4] = {0}; + wmempcpy(a, 0, 1); // expected-warning{{Null pointer passed as 2nd argument to memory copy function}} +} + +void wmempcpy12(void) { + wchar_t a[4] = {0}; + wmempcpy(0, a, 0); // no-warning +} + +void wmempcpy13(void) { + wchar_t a[4] = {0}; + wmempcpy(a, 0, 0); // no-warning +} + +void wmempcpy14(void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[5] = {0}; + wchar_t *p; + + p = wmempcpy(dst, src, 4); + + clang_analyzer_eval(p == &dst[4]); // expected-warning{{TRUE}} +} + +struct st { + wchar_t i; + wchar_t j; +}; + +void wmempcpy15(void) { + struct st s1 = {0}; + struct st s2; + struct st *p1; + struct st *p2; + + p1 = (&s2) + 1; + p2 = (struct st *)wmempcpy((wchar_t *)&s2, (wchar_t *)&s1, 2); + + clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}} +} + +void wmempcpy16(void) { + struct st s1[10] = {{0}}; + struct st s2[10]; + struct st *p1; + struct st *p2; + + p1 = (&s2[0]) + 5; + p2 = (struct st *)wmempcpy((wchar_t *)&s2[0], (wchar_t *)&s1[0], 5 * 2); + + clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}} +} + +void wmempcpy_unknown_size_warn (size_t n) { + wchar_t a[4]; + void *result = wmempcpy(a, 0, n); // expected-warning{{Null pointer passed as 2nd argument to memory copy function}} + clang_analyzer_eval(result == a); // no-warning (above is fatal) +} + +void wmempcpy_unknownable_size (wchar_t *src, float n) { + wchar_t a[4]; + // This used to crash because we don't model floats. + wmempcpy(a, src, (size_t)n); +} + +//===----------------------------------------------------------------------=== +// wmemmove() +//===----------------------------------------------------------------------=== + +#define wmemmove BUILTIN(wmemmove) +wchar_t *wmemmove(wchar_t *s1, const wchar_t *s2, size_t n); + +void wmemmove0 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[4] = {0}; + + wmemmove(dst, src, 4); // no-warning + + clang_analyzer_eval(wmemmove(dst, src, 4) == dst); // expected-warning{{TRUE}} + + clang_analyzer_eval(dst[0] != 0); // expected-warning{{UNKNOWN}} +} + +void wmemmove1 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[10]; + + wmemmove(dst, src, 5); // expected-warning{{out-of-bound}} +} + +void wmemmove2 (void) { + wchar_t src[] = {1, 2, 3, 4}; + wchar_t dst[1]; + + wmemmove(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}} +} + +//===----------------------------------------------------------------------=== +// wmemcmp() +//===----------------------------------------------------------------------=== + +#define wmemcmp BUILTIN(wmemcmp) +int wmemcmp(const wchar_t *s1, const wchar_t *s2, size_t n); + +void wmemcmp0 (void) { + wchar_t a[] = {1, 2, 3, 4}; + wchar_t b[4] = { 0 }; + + wmemcmp(a, b, 4); // no-warning +} + +void wmemcmp1 (void) { + wchar_t a[] = {1, 2, 3, 4}; + wchar_t b[10] = { 0 }; + + wmemcmp(a, b, 5); // expected-warning{{out-of-bound}} +} + +void wmemcmp2 (void) { + wchar_t a[] = {1, 2, 3, 4}; + wchar_t b[1] = { 0 }; + + wmemcmp(a, b, 4); // expected-warning{{out-of-bound}} +} + +void wmemcmp3 (void) { + wchar_t a[] = {1, 2, 3, 4}; + + clang_analyzer_eval(wmemcmp(a, a, 4) == 0); // expected-warning{{TRUE}} +} + +void wmemcmp4 (wchar_t *input) { + wchar_t a[] = {1, 2, 3, 4}; + + clang_analyzer_eval(wmemcmp(a, input, 4) == 0); // expected-warning{{UNKNOWN}} +} + +void wmemcmp5 (wchar_t *input) { + wchar_t a[] = {1, 2, 3, 4}; + + clang_analyzer_eval(wmemcmp(a, 0, 0) == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(wmemcmp(0, a, 0) == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(wmemcmp(a, input, 0) == 0); // expected-warning{{TRUE}} +} + +void wmemcmp6 (wchar_t *a, wchar_t *b, size_t n) { + int result = wmemcmp(a, b, n); + if (result != 0) + clang_analyzer_eval(n != 0); // expected-warning{{TRUE}} + // else + // analyzer_assert_unknown(n == 0); + + // We can't do the above comparison because n has already been constrained. + // On one path n == 0, on the other n != 0. +} + +int wmemcmp7 (wchar_t *a, size_t x, size_t y, size_t n) { + // We used to crash when either of the arguments was unknown. + return wmemcmp(a, &a[x*y], n) + + wmemcmp(&a[x*y], a, n); +} + +int wmemcmp8(wchar_t *a, size_t n) { + wchar_t *b = 0; + // Do not warn about the first argument! + return wmemcmp(a, b, n); // expected-warning{{Null pointer passed as 2nd argument to memory comparison function}} +} + //===----------------------------------------------------------------------=== // wcslen() //===----------------------------------------------------------------------===