Index: test/Analysis/string.c =================================================================== --- test/Analysis/string.c (revision 121776) +++ test/Analysis/string.c (working copy) @@ -196,6 +196,74 @@ } //===----------------------------------------------------------------------=== +// strncpy() +//===----------------------------------------------------------------------=== + +#ifdef VARIANT + +#define __strncpy_chk BUILTIN(__strncpy_chk) +char *__strncpy_chk(char *restrict s1, const char *restrict s2, size_t n, size_t destlen); + +#define strncpy(a,b,c) __strncpy_chk(a,b,c, (size_t)-1) + +#else /* VARIANT */ + +#define strncpy BUILTIN(strncpy) +char *strncpy(char *restrict s1, const char *restrict s2, size_t n); + +#endif /* VARIANT */ + + +void strncpy_null_dst(char *x) { + strncpy(NULL, x, 1); // expected-warning{{Null pointer argument in call to byte string function}} +} + +void strncpy_null_src(char *x) { + strncpy(x, NULL, 1); // expected-warning{{Null pointer argument in call to byte string function}} +} + +void strncpy_fn(char *x) { + strncpy(x, (char*)&strncpy_fn, 1); // expected-warning{{Argument to byte string function is the address of the function 'strncpy_fn', which is not a null-terminated string}} +} + +void strncpy_effects(char *x, char *y) { + char a = x[0]; + + if (strncpy(x, y, strlen(y)) != x) + (void)*(char*)0; // no-warning + + if (strlen(x) != strlen(y)) + (void)*(char*)0; // no-warning + + if (a != x[0]) + (void)*(char*)0; // expected-warning{{null}} +} + +void strncpy_overflow(char *y) { + char x[4]; + if (strlen(y) == 4) + strncpy(x, y, strlen(y)); // expected-warning{{Byte string function length parameter overflows destination buffer}} +} + +void strncpy_len_overflow(char *y) { + char x[4]; + if (strlen(y) == 3) + strncpy(x, y, sizeof(x)); // expected-warning{{Byte string function length parameter overflows destination buffer}} +} + +void strncpy_no_overflow(char *y) { + char x[4]; + if (strlen(y) == 3) + strncpy(x, y, strlen(y)); // no-warning +} + +void strncpy_no_len_overflow(char *y) { + char x[4]; + if (strlen(y) == 4) + strncpy(x, y, sizeof(x)-1); // no-warning +} + +//===----------------------------------------------------------------------=== // stpcpy() //===----------------------------------------------------------------------=== Index: lib/Checker/CStringChecker.cpp =================================================================== --- lib/Checker/CStringChecker.cpp (revision 121776) +++ lib/Checker/CStringChecker.cpp (working copy) @@ -54,8 +54,10 @@ void evalstrLength(CheckerContext &C, const CallExpr *CE); void evalStrcpy(CheckerContext &C, const CallExpr *CE); + void evalStrncpy(CheckerContext &C, const CallExpr *CE); void evalStpcpy(CheckerContext &C, const CallExpr *CE); - void evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, bool returnEnd); + void evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, bool returnEnd, + bool specifiedLen); // Utility methods std::pair @@ -79,7 +81,8 @@ const Expr *S, SVal l); const GRState *CheckLocation(CheckerContext &C, const GRState *state, const Expr *S, SVal l, - bool IsDestination = false); + bool IsDestination = false, + bool IsDefinedLength = false); const GRState *CheckBufferAccess(CheckerContext &C, const GRState *state, const Expr *Size, const Expr *FirstBuf, @@ -165,7 +168,8 @@ const GRState *CStringChecker::CheckLocation(CheckerContext &C, const GRState *state, const Expr *S, SVal l, - bool IsDestination) { + bool IsDestination, + bool IsDefinedLength) { // If a previous check has failed, propagate the failure. if (!state) return NULL; @@ -201,14 +205,24 @@ BuiltinBug *BT; if (IsDestination) { if (!BT_BoundsWrite) { - BT_BoundsWrite = new BuiltinBug("Out-of-bound array access", - "Byte string function overflows destination buffer"); + if (IsDefinedLength) { + BT_BoundsWrite = new BuiltinBug("Out-of-bound array access", + "Byte string function length parameter overflows destination buffer"); + } else { + BT_BoundsWrite = new BuiltinBug("Out-of-bound array access", + "Byte string function overflows destination buffer"); + } } BT = static_cast(BT_BoundsWrite); } else { if (!BT_Bounds) { - BT_Bounds = new BuiltinBug("Out-of-bound array access", - "Byte string function accesses out-of-bound array element"); + if (IsDefinedLength) { + BT_BoundsWrite = new BuiltinBug("Out-of-bound array access", + "Byte string function length parameter accesses out-of-bound element"); + } else { + BT_Bounds = new BuiltinBug("Out-of-bound array access", + "Byte string function accesses out-of-bound array element"); + } } BT = static_cast(BT_Bounds); } @@ -798,16 +812,21 @@ void CStringChecker::evalStrcpy(CheckerContext &C, const CallExpr *CE) { // char *strcpy(char *restrict dst, const char *restrict src); - evalStrcpyCommon(C, CE, /* returnEnd = */ false); + evalStrcpyCommon(C, CE, /* returnEnd = */ false, /* specifiedLen */ false); } +void CStringChecker::evalStrncpy(CheckerContext &C, const CallExpr *CE) { + // char *strcpy(char *restrict dst, const char *restrict src); + evalStrcpyCommon(C, CE, /* returnEnd = */ false, /* specifiedLen */ true); +} + void CStringChecker::evalStpcpy(CheckerContext &C, const CallExpr *CE) { // char *stpcpy(char *restrict dst, const char *restrict src); - evalStrcpyCommon(C, CE, /* returnEnd = */ true); + evalStrcpyCommon(C, CE, /* returnEnd = */ true, /* specifiedLen */ false); } void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, - bool returnEnd) { + bool returnEnd, bool specifiedLen) { const GRState *state = C.getState(); // Check that the destination is non-null @@ -828,6 +847,12 @@ // Get the string length of the source. SVal strLength = getCStringLength(C, state, srcExpr, srcVal); + if (specifiedLen) { + // Check if the number of bytes to copy is less than the size of the src + const Expr *lenExpr = CE->getArg(2); + strLength = state->getSVal(lenExpr); + } + // If the source isn't a valid C string, give up. if (strLength.isUndef()) return; @@ -843,7 +868,8 @@ C.getSValBuilder().evalBinOpLN(state, BO_Add, *dstRegVal, *knownStrLength, Dst->getType()); - state = CheckLocation(C, state, Dst, lastElement, /* IsDst = */ true); + state = CheckLocation(C, state, Dst, lastElement, /* IsDst = */ true, + specifiedLen); if (!state) return; @@ -852,6 +878,7 @@ Result = lastElement; } + // Invalidate the destination. This must happen before we set the C string // length because invalidation will clear the length. // FIXME: Even if we can't perfectly model the copy, we should see if we @@ -904,6 +931,7 @@ .Cases("memcmp", "bcmp", &CStringChecker::evalMemcmp) .Cases("memmove", "__memmove_chk", &CStringChecker::evalMemmove) .Cases("strcpy", "__strcpy_chk", &CStringChecker::evalStrcpy) + .Cases("strncpy", "__strncpy_chk", &CStringChecker::evalStrncpy) .Cases("stpcpy", "__stpcpy_chk", &CStringChecker::evalStpcpy) .Case("strlen", &CStringChecker::evalstrLength) .Case("bcopy", &CStringChecker::evalBcopy)