From: Jordy Rose Date: Wed, 7 Jul 2010 08:15:01 +0000 (+0000) Subject: Add memcmp() and bcmp() to CStringChecker. These check for valid access to the buffer... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bc56d1f6e2288aea9546b2380c71288939d688ca;p=clang Add memcmp() and bcmp() to CStringChecker. These check for valid access to the buffer arguments and have a special-case for when the buffer arguments are known to be the same address, or when the size is zero. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@107761 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Checker/CStringChecker.cpp b/lib/Checker/CStringChecker.cpp index cb30c1ba05..3c20baa93c 100644 --- a/lib/Checker/CStringChecker.cpp +++ b/lib/Checker/CStringChecker.cpp @@ -35,6 +35,7 @@ public: const GRState *EvalMemcpy(CheckerContext &C, const CallExpr *CE); const GRState *EvalMemmove(CheckerContext &C, const CallExpr *CE); + const GRState *EvalMemcmp(CheckerContext &C, const CallExpr *CE); const GRState *EvalBcopy(CheckerContext &C, const CallExpr *CE); // Utility methods @@ -387,6 +388,70 @@ CStringChecker::EvalMemmove(CheckerContext &C, const CallExpr *CE) { return state->BindExpr(CE, state->getSVal(Dest)); } +const GRState * +CStringChecker::EvalMemcmp(CheckerContext &C, const CallExpr *CE) { + // int memcmp(const void *s1, const void *s2, size_t n); + const Expr *Left = CE->getArg(0); + const Expr *Right = CE->getArg(1); + const Expr *Size = CE->getArg(2); + + const GRState *state = C.getState(); + ValueManager &ValMgr = C.getValueManager(); + SValuator &SV = ValMgr.getSValuator(); + const GRState *stateTrue, *stateFalse; + + // If we know the size argument is 0, we know the result is 0, and we don't + // have to check either of the buffers. (Another checker will have already + // made sure the size isn't undefined, so we can cast it safely.) + DefinedOrUnknownSVal SizeV = cast(state->getSVal(Size)); + DefinedOrUnknownSVal Zero = ValMgr.makeZeroVal(Size->getType()); + + DefinedOrUnknownSVal SizeIsZero = SV.EvalEQ(state, SizeV, Zero); + llvm::tie(stateTrue, stateFalse) = state->Assume(SizeIsZero); + + // FIXME: This should really cause a bifurcation of the state, but that would + // require changing the contract to allow the various Eval* methods to add + // transitions themselves. Currently that isn't the case because some of these + // functions are "basically" like another function, but with one or two + // additional restrictions (like memcpy and memmove). + + if (stateTrue && !stateFalse) + return stateTrue->BindExpr(CE, ValMgr.makeZeroVal(CE->getType())); + + // At this point, we still don't know that the size is nonzero, only that it + // might be. + + // 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 LBuf = cast(state->getSVal(Left)); + DefinedOrUnknownSVal RBuf = cast(state->getSVal(Right)); + + // See if they are the same. + DefinedOrUnknownSVal SameBuf = SV.EvalEQ(state, LBuf, RBuf); + llvm::tie(stateTrue, stateFalse) = state->Assume(SameBuf); + + // FIXME: This should also bifurcate the state (as above). + + // If the two arguments are known to be the same buffer, we know the result is + // zero, and we only need to check one size. + if (stateTrue && !stateFalse) { + state = CheckBufferAccess(C, stateTrue, Size, Left); + return state->BindExpr(CE, ValMgr.makeZeroVal(CE->getType())); + } + + // At this point, we don't know if the arguments are the same or not -- we + // only know that they *might* be different. We can't make any assumptions. + + // The return value is the comparison result, which we don't know. + unsigned Count = C.getNodeBuilder().getCurrentBlockCount(); + SVal RetVal = ValMgr.getConjuredSymbolVal(NULL, CE, CE->getType(), Count); + state = state->BindExpr(CE, RetVal); + + // Check that the accesses will stay in bounds. + return CheckBufferAccess(C, state, Size, Left, Right); +} + const GRState * CStringChecker::EvalBcopy(CheckerContext &C, const CallExpr *CE) { // void bcopy(const void *src, void *dst, size_t n); @@ -411,6 +476,7 @@ bool CStringChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) { FnCheck EvalFunction = llvm::StringSwitch(Name) .Cases("memcpy", "__memcpy_chk", &CStringChecker::EvalMemcpy) + .Cases("memcmp", "bcmp", &CStringChecker::EvalMemcmp) .Cases("memmove", "__memmove_chk", &CStringChecker::EvalMemmove) .Case("bcopy", &CStringChecker::EvalBcopy) .Default(NULL); diff --git a/test/Analysis/bstring.c b/test/Analysis/bstring.c index 467b87b878..59d6318db3 100644 --- a/test/Analysis/bstring.c +++ b/test/Analysis/bstring.c @@ -8,8 +8,9 @@ //===----------------------------------------------------------------------=== // Some functions are so similar to each other that they follow the same code -// path, such as memcpy and __memcpy_chk. If VARIANT is defined, make sure to -// use the variants instead to make sure they are still checked by the analyzer. +// path, such as memcpy and __memcpy_chk, or memcmp and bcmp. If VARIANT is +// defined, make sure to use the variants instead to make sure they are still +// checked by the analyzer. // Some functions are implemented as builtins. These should be #defined as // BUILTIN(f), which will prepend "__builtin_" if USE_BUILTINS is defined. @@ -172,6 +173,71 @@ void memmove2 () { memmove(dst, src, 4); // expected-warning{{out-of-bound}} } +//===----------------------------------------------------------------------=== +// memcmp() +//===----------------------------------------------------------------------=== + +#ifdef VARIANT + +#define bcmp BUILTIN(bcmp) +// __builtin_bcmp is not defined with const in Builtins.def. +int bcmp(/*const*/ void *s1, /*const*/ void *s2, size_t n); +#define memcmp bcmp + +#else /* VARIANT */ + +#define memcmp BUILTIN(memcmp) +int memcmp(const void *s1, const void *s2, size_t n); + +#endif /* VARIANT */ + + +void memcmp0 () { + char a[] = {1, 2, 3, 4}; + char b[4] = { 0 }; + + memcmp(a, b, 4); // no-warning +} + +void memcmp1 () { + char a[] = {1, 2, 3, 4}; + char b[10] = { 0 }; + + memcmp(a, b, 5); // expected-warning{{out-of-bound}} +} + +void memcmp2 () { + char a[] = {1, 2, 3, 4}; + char b[1] = { 0 }; + + memcmp(a, b, 4); // expected-warning{{out-of-bound}} +} + +void memcmp3 () { + char a[] = {1, 2, 3, 4}; + + if (memcmp(a, a, 4)) + (void)*(char*)0; // no-warning +} + +void memcmp4 (char *input) { + char a[] = {1, 2, 3, 4}; + + if (memcmp(a, input, 4)) + (void)*(char*)0; // expected-warning{{null}} +} + +void memcmp5 (char *input) { + char a[] = {1, 2, 3, 4}; + + if (memcmp(a, 0, 0)) // no-warning + (void)*(char*)0; // no-warning + if (memcmp(0, a, 0)) // no-warning + (void)*(char*)0; // no-warning + if (memcmp(a, input, 0)) // no-warning + (void)*(char*)0; // no-warning +} + //===----------------------------------------------------------------------=== // bcopy() //===----------------------------------------------------------------------===