[cfe-commits] r86517 - in /cfe/trunk: lib/Analysis/GRExprEngineInternalChecks.cpp lib/Analysis/GRExprEngineInternalChecks.h lib/Analysis/PointerSubChecker.cpp test/Analysis/ptr-arith.c

[Ted Kremenek]

Hi Zhongxing,

It seems to me that we should be comparing the "base" regions, not the  
regions themselves.  For example, we can have two different  
ElementRegions that refer to different locations of the same buffer,  
two FieldRegions that refer to different fields within a struct, etc.,  
and that would be completely valid.  I think it's also fine to involve  
SymbolicRegions, since they just represent the base of some chunk of  
memory, and typically they will wrapped with an ElementRegion anyway.

What do you think?

Ted

On Nov 8, 2009, at 9:34 PM, Zhongxing Xu wrote:

> Author: zhongxingxu
> Date: Sun Nov  8 23:34:10 2009
> New Revision: 86517
>
> URL: http://llvm.org/viewvc/llvm-project?rev=86517&view=rev
> Log:
> Add checker for CWE-469: Use of Pointer Subtraction to Determine  
> Size. This
> checker does not build sink nodes. Because svaluator computes an  
> unknown value
> for the subtraction now.
>
> Added:
>    cfe/trunk/lib/Analysis/PointerSubChecker.cpp
> Modified:
>    cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
>    cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h
>    cfe/trunk/test/Analysis/ptr-arith.c
>
> Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp?rev=86517&r1=86516&r2=86517&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp (original)
> +++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp Sun Nov  8  
> 23:34:10 2009
> @@ -412,6 +412,7 @@
>   RegisterDivZeroChecker(*this);
>   RegisterReturnStackAddressChecker(*this);
>   RegisterReturnUndefChecker(*this);
> +  RegisterPointerSubChecker(*this);
>
>   // Note that this must be registered after  
> ReturnStackAddressChecker.
>   RegisterReturnPointerRangeChecker(*this);
>
> Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h?rev=86517&r1=86516&r2=86517&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h (original)
> +++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h Sun Nov  8  
> 23:34:10 2009
> @@ -24,6 +24,6 @@
> void RegisterReturnStackAddressChecker(GRExprEngine &Eng);
> void RegisterReturnUndefChecker(GRExprEngine &Eng);
> void RegisterVLASizeChecker(GRExprEngine &Eng);
> -
> +void RegisterPointerSubChecker(GRExprEngine &Eng);
> } // end clang namespace
> #endif
>
> Added: cfe/trunk/lib/Analysis/PointerSubChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PointerSubChecker.cpp?rev=86517&view=auto
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/PointerSubChecker.cpp (added)
> +++ cfe/trunk/lib/Analysis/PointerSubChecker.cpp Sun Nov  8 23:34:10  
> 2009
> @@ -0,0 +1,71 @@
> +//=== PointerSubChecker.cpp - Pointer subtraction checker ------*- C 
> ++ -*--===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open  
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +//
> +// This files defines PointerSubChecker, a builtin checker that  
> checks for
> +// pointer subtractions on two pointers pointing to different  
> memory chunks.
> +// This check corresponds to CWE-469.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +
> +#include "clang/Analysis/PathSensitive/CheckerVisitor.h"
> +#include "GRExprEngineInternalChecks.h"
> +
> +using namespace clang;
> +
> +namespace {
> +class VISIBILITY_HIDDEN PointerSubChecker
> +  : public CheckerVisitor<PointerSubChecker> {
> +  BuiltinBug *BT;
> +public:
> +  PointerSubChecker() : BT(0) {}
> +  static void *getTag();
> +  void PreVisitBinaryOperator(CheckerContext &C, const  
> BinaryOperator *B);
> +};
> +}
> +
> +void *PointerSubChecker::getTag() {
> +  static int x;
> +  return &x;
> +}
> +
> +void PointerSubChecker::PreVisitBinaryOperator(CheckerContext &C,
> +                                               const BinaryOperator  
> *B) {
> +  // When doing pointer subtraction, if the two pointers do not  
> point to the
> +  // same memory chunk, emit a warning.
> +  if (B->getOpcode() != BinaryOperator::Sub)
> +    return;
> +
> +  const GRState *state = C.getState();
> +  SVal LV = state->getSVal(B->getLHS());
> +  SVal RV = state->getSVal(B->getRHS());
> +
> +  const MemRegion *LR = LV.getAsRegion();
> +  const MemRegion *RR = RV.getAsRegion();
> +
> +  if (!(LR && RR) || (LR == RR))
> +    return;
> +
> +  // We don't reason about SymbolicRegions for now.
> +  if (isa<SymbolicRegion>(LR) || isa<SymbolicRegion>(RR))
> +    return;
> +
> +  if (ExplodedNode *N = C.GenerateNode(B)) {
> +    if (!BT)
> +      BT = new BuiltinBug("Pointer subtraction",
> +                          "Subtraction of two pointers that do not  
> point to the same memory chunk may cause incorrect result.");
> +    RangedBugReport *R = new RangedBugReport(*BT, BT->getDescription 
> ().c_str(),
> +                                             N);
> +    R->addRange(B->getSourceRange());
> +    C.EmitReport(R);
> +  }
> +}
> +
> +void clang::RegisterPointerSubChecker(GRExprEngine &Eng) {
> +  Eng.registerCheck(new PointerSubChecker());
> +}
>
> Modified: cfe/trunk/test/Analysis/ptr-arith.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ptr-arith.c?rev=86517&r1=86516&r2=86517&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/test/Analysis/ptr-arith.c (original)
> +++ cfe/trunk/test/Analysis/ptr-arith.c Sun Nov  8 23:34:10 2009
> @@ -31,3 +31,8 @@
>     port = 10 * port + (*p - '0');
>   return port;
> }
> +
> +void f3() {
> +  int x, y;
> +  int d = &y - &x; // expected-warning{{Subtraction of two pointers  
> that do not point to the same memory chunk may cause incorrect  
> result.}}
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits