[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 t

Zhongxing Xu xuzhongxing at gmail.com
Wed Nov 11 17:08:16 PST 2009


Yeah.

2009/11/12 Ted Kremenek <kremenek at apple.com>:
> 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