[cfe-commits] r86834 - in /cfe/trunk: lib/Analysis/ArrayBoundChecker.cpp lib/Analysis/CMakeLists.txt lib/Analysis/GRExprEngineInternalChecks.cpp lib/Analysis/GRExprEngineInternalChecks.h test/Analysis/no-outofbounds.c test/Analysis/outofbound.c test/Analysis/rdar-6541136-region.c
Ted Kremenek
kremenek at apple.com
Wed Nov 11 09:24:04 PST 2009
Hi Zhongxing,
Thanks for doing this! While the check looks correct, the reason the
old code in GRExprEngine was commented out was not because I disliked
it, but because AssumeInBound wasn't doing the right thing (at least
at the time). That happened when a bunch of changes were being made
to RegionStoreManager and the MemRegion hierarchy in general. Now
that those changes have settled, I think it is definitely time to
revisit this check. My main point here is that AssumeInBound still
might not be doing the right thing, and we need review it and test it.
Ted
On Nov 11, 2009, at 4:33 AM, Zhongxing Xu wrote:
> Author: zhongxingxu
> Date: Wed Nov 11 06:33:27 2009
> New Revision: 86834
>
> URL: http://llvm.org/viewvc/llvm-project?rev=86834&view=rev
> Log:
> Reimplement out-of-bound array access checker with the new checker
> interface.
> Now only one test case is XFAIL'ed.
>
> Added:
> cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp
> Modified:
> cfe/trunk/lib/Analysis/CMakeLists.txt
> cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
> cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h
> cfe/trunk/test/Analysis/no-outofbounds.c
> cfe/trunk/test/Analysis/outofbound.c
> cfe/trunk/test/Analysis/rdar-6541136-region.c
>
> Added: cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp?rev=86834&view=auto
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp (added)
> +++ cfe/trunk/lib/Analysis/ArrayBoundChecker.cpp Wed Nov 11 06:33:27
> 2009
> @@ -0,0 +1,87 @@
> +//== ArrayBoundChecker.cpp ------------------------------*- C++ -*--
> ==//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +//
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> +//
> +// This file defines ArrayBoundChecker, which is a path-sensitive
> check
> +// which looks for an out-of-bound array element access.
> +//
> +//
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> +
> +#include "GRExprEngineInternalChecks.h"
> +#include "clang/Analysis/PathSensitive/GRExprEngine.h"
> +#include "clang/Analysis/PathSensitive/BugReporter.h"
> +#include "clang/Analysis/PathSensitive/CheckerVisitor.h"
> +
> +using namespace clang;
> +
> +namespace {
> +class VISIBILITY_HIDDEN ArrayBoundChecker :
> + public CheckerVisitor<ArrayBoundChecker> {
> + BuiltinBug *BT;
> +public:
> + ArrayBoundChecker() : BT(0) {}
> + static void *getTag();
> + void VisitLocation(CheckerContext &C, const Stmt *S, SVal l);
> +};
> +}
> +
> +void clang::RegisterArrayBoundChecker(GRExprEngine &Eng) {
> + Eng.registerCheck(new ArrayBoundChecker());
> +}
> +
> +void *ArrayBoundChecker::getTag() {
> + static int x = 0; return &x;
> +}
> +
> +void ArrayBoundChecker::VisitLocation(CheckerContext &C, const Stmt
> *S, SVal l){
> + // Check for out of bound array element access.
> + const MemRegion *R = l.getAsRegion();
> + if (!R)
> + return;
> +
> + R = R->StripCasts();
> +
> + const ElementRegion *ER = dyn_cast<ElementRegion>(R);
> + if (!ER)
> + return;
> +
> + // Get the index of the accessed element.
> + DefinedOrUnknownSVal &Idx = cast<DefinedOrUnknownSVal>(ER-
> >getIndex());
> +
> + const GRState *state = C.getState();
> +
> + // Get the size of the array.
> + SVal NumVal = C.getStoreManager().getSizeInElements(state,
> + ER-
> >getSuperRegion());
> + DefinedOrUnknownSVal &NumElements = cast<DefinedOrUnknownSVal>
> (NumVal);
> +
> + const GRState *StInBound = state->AssumeInBound(Idx, NumElements,
> true);
> + const GRState *StOutBound = state->AssumeInBound(Idx,
> NumElements, false);
> + if (StOutBound && !StInBound) {
> + ExplodedNode *N = C.GenerateNode(S, StOutBound, true);
> +
> + if (!N)
> + return;
> +
> + if (!BT)
> + BT = new BuiltinBug("Out-of-bound array access",
> + "Access out-of-bound array element (buffer
> overflow)");
> +
> + // FIXME: It would be nice to eventually make this diagnostic
> more clear,
> + // e.g., by referencing the original declaration or by saying
> *why* this
> + // reference is outside the range.
> +
> + // Generate a report for this bug.
> + RangedBugReport *report =
> + new RangedBugReport(*BT, BT->getDescription().c_str(), N);
> +
> + report->addRange(S->getSourceRange());
> +
> + C.EmitReport(report);
> + }
> +}
>
> Modified: cfe/trunk/lib/Analysis/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CMakeLists.txt?rev=86834&r1=86833&r2=86834&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/Analysis/CMakeLists.txt (original)
> +++ cfe/trunk/lib/Analysis/CMakeLists.txt Wed Nov 11 06:33:27 2009
> @@ -2,6 +2,7 @@
>
> add_clang_library(clangAnalysis
> AnalysisContext.cpp
> + ArrayBoundChecker.cpp
> AttrNonNullChecker.cpp
> BadCallChecker.cpp
> BasicConstraintManager.cpp
>
> Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp?rev=86834&r1=86833&r2=86834&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp (original)
> +++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp Wed Nov 11
> 06:33:27 2009
> @@ -414,4 +414,5 @@
> RegisterReturnPointerRangeChecker(*this);
>
> RegisterCastToStructChecker(*this);
> + RegisterArrayBoundChecker(*this);
> }
>
> Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h?rev=86834&r1=86833&r2=86834&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h (original)
> +++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h Wed Nov 11
> 06:33:27 2009
> @@ -32,6 +32,7 @@
> void RegisterFixedAddressChecker(GRExprEngine &Eng);
> void RegisterCastToStructChecker(GRExprEngine &Eng);
> void RegisterUndefinedArgChecker(GRExprEngine &Eng);
> +void RegisterArrayBoundChecker(GRExprEngine &Eng);
>
> } // end clang namespace
> #endif
>
> Modified: cfe/trunk/test/Analysis/no-outofbounds.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/no-outofbounds.c?rev=86834&r1=86833&r2=86834&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/test/Analysis/no-outofbounds.c (original)
> +++ cfe/trunk/test/Analysis/no-outofbounds.c Wed Nov 11 06:33:27 2009
> @@ -1,5 +1,6 @@
> // RUN: clang-cc -checker-cfref -analyze -analyzer-store=basic -
> verify %s
> // RUN: clang-cc -checker-cfref -analyze -analyzer-store=region -
> verify %s
> +// XFAIL: *
>
> //
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> // This file tests cases where we should not flag out-of-bounds
> warnings.
>
> Modified: cfe/trunk/test/Analysis/outofbound.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/outofbound.c?rev=86834&r1=86833&r2=86834&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/test/Analysis/outofbound.c (original)
> +++ cfe/trunk/test/Analysis/outofbound.c Wed Nov 11 06:33:27 2009
> @@ -1,8 +1,7 @@
> // RUN: clang-cc -analyze -checker-cfref -analyzer-store=region -
> verify %s
> -// XFAIL: *
>
> char f1() {
> char* s = "abcd";
> char c = s[4]; // no-warning
> - return s[5] + c; // expected-warning{{Load or store into an out-
> of-bound memory position.}}
> + return s[5] + c; // expected-warning{{Access out-of-bound array
> element (buffer overflow)}}
> }
>
> Modified: cfe/trunk/test/Analysis/rdar-6541136-region.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/rdar-6541136-region.c?rev=86834&r1=86833&r2=86834&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/test/Analysis/rdar-6541136-region.c (original)
> +++ cfe/trunk/test/Analysis/rdar-6541136-region.c Wed Nov 11
> 06:33:27 2009
> @@ -13,10 +13,10 @@
> struct load_wine *cmd = (void*) &wonky[1];
> cmd = cmd;
> char *p = (void*) &wonky[1];
> - *p = 1; // no-warning
> + //*p = 1; // this is also an out-of-bound access.
> kernel_tea_cheese_t *q = &wonky[1];
> // This test case tests both the RegionStore logic (doesn't crash)
> and
> // the out-of-bounds checking. We don't expect the warning for
> now since
> // out-of-bound checking is temporarily disabled.
> - kernel_tea_cheese_t r = *q; // eventually-warning{{out-of-bound
> memory position}}
> + kernel_tea_cheese_t r = *q; // expected-warning{{Access out-of-
> bound array element (buffer overflow)}}
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list