[cfe-commits] r86523 - in /cfe/trunk: include/clang/Analysis/PathSensitive/SVals.h lib/Analysis/FixedAddressChecker.cpp lib/Analysis/GRExprEngineInternalChecks.cpp lib/Analysis/GRExprEngineInternalChecks.h lib/Analysis/SVals.cpp test/Analysis/ptr

Zhongxing Xu xuzhongxing at gmail.com
Mon Nov 9 16:33:09 PST 2009


2009/11/10 Ted Kremenek <kremenek at apple.com>:
> Hi Zhongxing,
>
> Very nice  I vaguely recall we use to do this check, albeit not as directly.

Perhaps in EvalLocation().

>  How do we want to handle cases like PR 5272
> (http://llvm.org/bugs/show_bug.cgi?id=5272)?

Leave it as a false positive.

> Using hardcoded addresses is
> fine in certain circumstances.

Definitely. So I see this kind of check as some kind of guide to the
programmer, not as some absolute judger of the code. False positives
are allowed. There should not be too much.

>
> On Nov 8, 2009, at 10:52 PM, Zhongxing Xu wrote:
>
>> Author: zhongxingxu
>> Date: Mon Nov  9 00:52:44 2009
>> New Revision: 86523
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=86523&view=rev
>> Log:
>> Add checker for CWE-587: Assignment of a Fixed Address to a Pointer.
>>
>> Added:
>>   cfe/trunk/lib/Analysis/FixedAddressChecker.cpp
>> Modified:
>>   cfe/trunk/include/clang/Analysis/PathSensitive/SVals.h
>>   cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
>>   cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h
>>   cfe/trunk/lib/Analysis/SVals.cpp
>>   cfe/trunk/test/Analysis/ptr-arith.c
>>
>> Modified: cfe/trunk/include/clang/Analysis/PathSensitive/SVals.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/SVals.h?rev=86523&r1=86522&r2=86523&view=diff
>>
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Analysis/PathSensitive/SVals.h (original)
>> +++ cfe/trunk/include/clang/Analysis/PathSensitive/SVals.h Mon Nov  9
>> 00:52:44 2009
>> @@ -96,6 +96,8 @@
>>    return getRawKind() > UnknownKind;
>>  }
>>
>> +  bool isConstant() const;
>> +
>>  bool isZeroConstant() const;
>>
>>  /// hasConjuredSymbol - If this SVal wraps a conjured symbol, return
>> true;
>>
>> Added: cfe/trunk/lib/Analysis/FixedAddressChecker.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FixedAddressChecker.cpp?rev=86523&view=auto
>>
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/FixedAddressChecker.cpp (added)
>> +++ cfe/trunk/lib/Analysis/FixedAddressChecker.cpp Mon Nov  9 00:52:44
>> 2009
>> @@ -0,0 +1,69 @@
>> +//=== FixedAddressChecker.cpp - Fixed address usage 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 FixedAddressChecker, a builtin checker that checks
>> for
>> +// assignment of a fixed address to a pointer.
>> +// This check corresponds to CWE-587.
>> +//
>>
>> +//===----------------------------------------------------------------------===//
>> +
>> +#include "clang/Analysis/PathSensitive/CheckerVisitor.h"
>> +#include "GRExprEngineInternalChecks.h"
>> +
>> +using namespace clang;
>> +
>> +namespace {
>> +class VISIBILITY_HIDDEN FixedAddressChecker
>> +  : public CheckerVisitor<FixedAddressChecker> {
>> +  BuiltinBug *BT;
>> +public:
>> +  FixedAddressChecker() : BT(0) {}
>> +  static void *getTag();
>> +  void PreVisitBinaryOperator(CheckerContext &C, const BinaryOperator
>> *B);
>> +};
>> +}
>> +
>> +void *FixedAddressChecker::getTag() {
>> +  static int x;
>> +  return &x;
>> +}
>> +
>> +void FixedAddressChecker::PreVisitBinaryOperator(CheckerContext &C,
>> +                                                 const BinaryOperator *B)
>> {
>> +  // Using a fixed address is not portable because that address will
>> probably
>> +  // not be valid in all environments or platforms.
>> +
>> +  if (B->getOpcode() != BinaryOperator::Assign)
>> +    return;
>> +
>> +  QualType T = B->getType();
>> +  if (!T->isPointerType())
>> +    return;
>> +
>> +  const GRState *state = C.getState();
>> +
>> +  SVal RV = state->getSVal(B->getRHS());
>> +
>> +  if (!RV.isConstant() || RV.isZeroConstant())
>> +    return;
>> +
>> +  if (ExplodedNode *N = C.GenerateNode(B)) {
>> +    if (!BT)
>> +      BT = new BuiltinBug("Use fixed address",
>> +                          "Using a fixed address is not portable because
>> that address will probably not be valid in all environments or platforms.");
>> +    RangedBugReport *R = new RangedBugReport(*BT,
>> BT->getDescription().c_str(),
>> +                                             N);
>> +    R->addRange(B->getRHS()->getSourceRange());
>> +    C.EmitReport(R);
>> +  }
>> +}
>> +
>> +void clang::RegisterFixedAddressChecker(GRExprEngine &Eng) {
>> +  Eng.registerCheck(new FixedAddressChecker());
>> +}
>>
>> Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp?rev=86523&r1=86522&r2=86523&view=diff
>>
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp (original)
>> +++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp Mon Nov  9
>> 00:52:44 2009
>> @@ -413,7 +413,7 @@
>>  RegisterReturnStackAddressChecker(*this);
>>  RegisterReturnUndefChecker(*this);
>>  RegisterPointerSubChecker(*this);
>> -
>> +  RegisterFixedAddressChecker(*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=86523&r1=86522&r2=86523&view=diff
>>
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h (original)
>> +++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.h Mon Nov  9
>> 00:52:44 2009
>> @@ -24,6 +24,7 @@
>> void RegisterReturnStackAddressChecker(GRExprEngine &Eng);
>> void RegisterReturnUndefChecker(GRExprEngine &Eng);
>> void RegisterVLASizeChecker(GRExprEngine &Eng);
>> -void RegisterPointerSubChecker(GRExprEngine &Eng);
>> +void RegisterPointerSubChecker(GRExprEngine &Eng);
>> +void RegisterFixedAddressChecker(GRExprEngine &Eng);
>> } // end clang namespace
>> #endif
>>
>> Modified: cfe/trunk/lib/Analysis/SVals.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/SVals.cpp?rev=86523&r1=86522&r2=86523&view=diff
>>
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/SVals.cpp (original)
>> +++ cfe/trunk/lib/Analysis/SVals.cpp Mon Nov  9 00:52:44 2009
>> @@ -173,6 +173,10 @@
>> // Useful predicates.
>>
>> //===----------------------------------------------------------------------===//
>>
>> +bool SVal::isConstant() const {
>> +  return isa<nonloc::ConcreteInt>(this) || isa<loc::ConcreteInt>(this);
>> +}
>> +
>> bool SVal::isZeroConstant() const {
>>  if (isa<loc::ConcreteInt>(*this))
>>    return cast<loc::ConcreteInt>(*this).getValue() == 0;
>>
>> Modified: cfe/trunk/test/Analysis/ptr-arith.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ptr-arith.c?rev=86523&r1=86522&r2=86523&view=diff
>>
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/ptr-arith.c (original)
>> +++ cfe/trunk/test/Analysis/ptr-arith.c Mon Nov  9 00:52:44 2009
>> @@ -36,3 +36,8 @@
>>  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.}}
>> }
>> +
>> +void f4() {
>> +  int *p;
>> +  p = (int*) 0x10000; // expected-warning{{Using a fixed address is not
>> portable because that address will probably not be valid in all environments
>> or platforms.}}
>> +}
>>
>>
>> _______________________________________________
>> 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