[cfe-commits] r52799 - in /cfe/trunk: lib/Analysis/BasicObjCFoundationChecks.cpp lib/Analysis/BasicObjCFoundationChecks.h lib/Analysis/GRSimpleVals.cpp test/Analysis/CFNumber.c
Ted Kremenek
kremenek at apple.com
Thu Jun 26 16:59:49 PDT 2008
Author: kremenek
Date: Thu Jun 26 18:59:48 2008
New Revision: 52799
URL: http://llvm.org/viewvc/llvm-project?rev=52799&view=rev
Log:
Added a simple static analysis check to look for improper uses of CFCreateNumber.
Added:
cfe/trunk/test/Analysis/CFNumber.c
Modified:
cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.cpp
cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.h
cfe/trunk/lib/Analysis/GRSimpleVals.cpp
Modified: cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.cpp?rev=52799&r1=52798&r2=52799&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.cpp (original)
+++ cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.cpp Thu Jun 26 18:59:48 2008
@@ -289,3 +289,260 @@
return false;
}
+
+//===----------------------------------------------------------------------===//
+// Error reporting.
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+class VISIBILITY_HIDDEN BadCFNumberCreate : public BugTypeCacheLocation {
+public:
+ typedef std::vector<BugReport*> AllErrorsTy;
+ AllErrorsTy AllErrors;
+
+ virtual const char* getName() const {
+ return "Bad use of CFNumberCreate";
+ }
+
+ virtual void EmitWarnings(BugReporter& BR) {
+ // FIXME: Refactor this method.
+ for (AllErrorsTy::iterator I=AllErrors.begin(), E=AllErrors.end(); I!=E;++I)
+ BR.EmitWarning(**I);
+ }
+};
+
+ // FIXME: This entire class should be refactored into the common
+ // BugReporter classes.
+class VISIBILITY_HIDDEN StrBugReport : public RangedBugReport {
+ std::string str;
+ const char* cstr;
+public:
+ StrBugReport(BugType& D, ExplodedNode<ValueState>* N, std::string s)
+ : RangedBugReport(D, N), str(s) {
+ cstr = str.c_str();
+ }
+
+ virtual const char* getDescription() const { return cstr; }
+};
+
+
+class VISIBILITY_HIDDEN AuditCFNumberCreate : public GRSimpleAPICheck {
+ // FIXME: Who should own this?
+ BadCFNumberCreate Desc;
+
+ // FIXME: Either this should be refactored into GRSimpleAPICheck, or
+ // it should always be passed with a call to Audit. The latter
+ // approach makes this class more stateless.
+ ASTContext& Ctx;
+ IdentifierInfo* II;
+ ValueStateManager* VMgr;
+
+ RVal GetRVal(ValueState* St, Expr* E) { return VMgr->GetRVal(St, E); }
+ RVal GetRVal(ValueState* St, LVal LV) { return VMgr->GetRVal(St, LV); }
+
+public:
+
+ AuditCFNumberCreate(ASTContext& ctx, ValueStateManager* vmgr)
+ : Ctx(ctx), II(&Ctx.Idents.get("CFNumberCreate")), VMgr(vmgr) {}
+
+ virtual ~AuditCFNumberCreate() {}
+
+ virtual bool Audit(ExplodedNode<ValueState>* N);
+
+ virtual void EmitWarnings(BugReporter& BR) {
+ Desc.EmitWarnings(BR);
+ }
+
+private:
+
+ void AddError(VarDecl* V, Expr* Ex, ExplodedNode<ValueState> *N,
+ uint64_t SourceSize, uint64_t TargetSize, uint64_t NumberKind);
+};
+} // end anonymous namespace
+
+enum CFNumberType {
+ kCFNumberSInt8Type = 1,
+ kCFNumberSInt16Type = 2,
+ kCFNumberSInt32Type = 3,
+ kCFNumberSInt64Type = 4,
+ kCFNumberFloat32Type = 5,
+ kCFNumberFloat64Type = 6,
+ kCFNumberCharType = 7,
+ kCFNumberShortType = 8,
+ kCFNumberIntType = 9,
+ kCFNumberLongType = 10,
+ kCFNumberLongLongType = 11,
+ kCFNumberFloatType = 12,
+ kCFNumberDoubleType = 13,
+ kCFNumberCFIndexType = 14,
+ kCFNumberNSIntegerType = 15,
+ kCFNumberCGFloatType = 16
+};
+
+namespace {
+ template<typename T>
+ class Optional {
+ bool IsKnown;
+ T Val;
+ public:
+ Optional() : IsKnown(false), Val(0) {}
+ Optional(const T& val) : IsKnown(true), Val(val) {}
+
+ bool isKnown() const { return IsKnown; }
+
+ const T& getValue() const {
+ assert (isKnown());
+ return Val;
+ }
+
+ operator const T&() const {
+ return getValue();
+ }
+ };
+}
+
+static Optional<uint64_t> GetCFNumberSize(ASTContext& Ctx, uint64_t i) {
+ static unsigned char FixedSize[] = { 8, 16, 32, 64, 32, 64 };
+
+ if (i < kCFNumberCharType)
+ return FixedSize[i-1];
+
+ QualType T;
+
+ switch (i) {
+ case kCFNumberCharType: T = Ctx.CharTy; break;
+ case kCFNumberShortType: T = Ctx.ShortTy; break;
+ case kCFNumberIntType: T = Ctx.IntTy; break;
+ case kCFNumberLongType: T = Ctx.LongTy; break;
+ case kCFNumberLongLongType: T = Ctx.LongLongTy; break;
+ case kCFNumberFloatType: T = Ctx.FloatTy; break;
+ case kCFNumberDoubleType: T = Ctx.DoubleTy; break;
+ case kCFNumberCFIndexType:
+ case kCFNumberNSIntegerType:
+ case kCFNumberCGFloatType:
+ // FIXME: We need a way to map from names to Type*.
+ default:
+ return Optional<uint64_t>();
+ }
+
+ return Ctx.getTypeSize(T);
+}
+
+#if 0
+static const char* GetCFNumberTypeStr(uint64_t i) {
+ static const char* Names[] = {
+ "kCFNumberSInt8Type",
+ "kCFNumberSInt16Type",
+ "kCFNumberSInt32Type",
+ "kCFNumberSInt64Type",
+ "kCFNumberFloat32Type",
+ "kCFNumberFloat64Type",
+ "kCFNumberCharType",
+ "kCFNumberShortType",
+ "kCFNumberIntType",
+ "kCFNumberLongType",
+ "kCFNumberLongLongType",
+ "kCFNumberFloatType",
+ "kCFNumberDoubleType",
+ "kCFNumberCFIndexType",
+ "kCFNumberNSIntegerType",
+ "kCFNumberCGFloatType"
+ };
+
+ return i <= kCFNumberCGFloatType ? Names[i-1] : "Invalid CFNumberType";
+}
+#endif
+
+bool AuditCFNumberCreate::Audit(ExplodedNode<ValueState>* N) {
+ CallExpr* CE = cast<CallExpr>(cast<PostStmt>(N->getLocation()).getStmt());
+ Expr* Callee = CE->getCallee();
+ RVal CallV = GetRVal(N->getState(), Callee);
+ lval::FuncVal* FuncV = dyn_cast<lval::FuncVal>(&CallV);
+
+ if (!FuncV || FuncV->getDecl()->getIdentifier() != II || CE->getNumArgs()!=3)
+ return false;
+
+ // Get the value of the "theType" argument.
+ RVal TheTypeVal = GetRVal(N->getState(), CE->getArg(1));
+
+ // FIXME: We really should allow ranges of valid theType values, and
+ // bifurcate the state appropriately.
+ nonlval::ConcreteInt* V = dyn_cast<nonlval::ConcreteInt>(&TheTypeVal);
+
+ if (!V)
+ return false;
+
+ uint64_t NumberKind = V->getValue().getLimitedValue();
+ Optional<uint64_t> TargetSize = GetCFNumberSize(Ctx, NumberKind);
+
+ // FIXME: In some cases we can emit an error.
+ if (!TargetSize.isKnown())
+ return false;
+
+ // Look at the value of the integer being passed by reference. Essentially
+ // we want to catch cases where the value passed in is not equal to the
+ // size of the type being created.
+ RVal TheValueExpr = GetRVal(N->getState(), CE->getArg(2));
+
+ // FIXME: Eventually we should handle arbitrary locations. We can do this
+ // by having an enhanced memory model that does low-level typing.
+ lval::DeclVal* LV = dyn_cast<lval::DeclVal>(&TheValueExpr);
+
+ if (!LV)
+ return false;
+
+ QualType T = LV->getDecl()->getType().getCanonicalType();
+
+ // FIXME: If the pointee isn't an integer type, should we flag a warning?
+ // People can do weird stuff with pointers.
+
+ if (!T->isIntegerType())
+ return false;
+
+ uint64_t SourceSize = Ctx.getTypeSize(T);
+
+ // CHECK: is SourceSize == TargetSize
+
+ if (SourceSize == TargetSize)
+ return false;
+
+ AddError(LV->getDecl(), CE->getArg(2), N, SourceSize, TargetSize, NumberKind);
+
+ // FIXME: We can actually create an abstract "CFNumber" object that has
+ // the bits initialized to the provided values.
+ return SourceSize < TargetSize;
+}
+
+void AuditCFNumberCreate::AddError(VarDecl* V, Expr* Ex,
+ ExplodedNode<ValueState> *N,
+ uint64_t SourceSize, uint64_t TargetSize,
+ uint64_t NumberKind) {
+
+ std::ostringstream os;
+
+ os << (SourceSize == 8 ? "An " : "A ")
+ << SourceSize << " bit integer is used to initialize a CFNumber "
+ "object that represents "
+ << (TargetSize == 8 ? "an " : "a ")
+ << TargetSize << " bit integer. ";
+
+ if (SourceSize < TargetSize)
+ os << (TargetSize - SourceSize)
+ << " bits of the CFNumber value will be garbage." ;
+ else
+ os << (SourceSize - TargetSize)
+ << " bits of the input integer will be lost.";
+
+ StrBugReport* B = new StrBugReport(Desc, N, os.str());
+ B->addRange(Ex->getSourceRange());
+ Desc.AllErrors.push_back(B);
+}
+
+GRSimpleAPICheck*
+clang::CreateAuditCFNumberCreate(ASTContext& Ctx,
+ ValueStateManager* VMgr) {
+
+ return new AuditCFNumberCreate(Ctx, VMgr);
+}
+
Modified: cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.h?rev=52799&r1=52798&r2=52799&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.h (original)
+++ cfe/trunk/lib/Analysis/BasicObjCFoundationChecks.h Thu Jun 26 18:59:48 2008
@@ -33,6 +33,10 @@
GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx,
ValueStateManager* VMgr);
+
+GRSimpleAPICheck* CreateAuditCFNumberCreate(ASTContext& Ctx,
+ ValueStateManager* VMgr);
+
} // end clang namespace
Modified: cfe/trunk/lib/Analysis/GRSimpleVals.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRSimpleVals.cpp?rev=52799&r1=52798&r2=52799&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRSimpleVals.cpp (original)
+++ cfe/trunk/lib/Analysis/GRSimpleVals.cpp Thu Jun 26 18:59:48 2008
@@ -354,11 +354,15 @@
Eng.Register(MakeDeadStoresChecker());
// Add extra checkers.
+ ASTContext& Ctx = Eng.getContext();
+ ValueStateManager* VMgr = &Eng.getStateManager();
- GRSimpleAPICheck* FoundationCheck =
- CreateBasicObjCFoundationChecks(Eng.getContext(), &Eng.getStateManager());
+ GRSimpleAPICheck* Check = CreateBasicObjCFoundationChecks(Ctx, VMgr);
+ Eng.AddObjCMessageExprCheck(Check);
+
+ Check = CreateAuditCFNumberCreate(Ctx, VMgr);
+ Eng.AddCallCheck(Check);
- Eng.AddObjCMessageExprCheck(FoundationCheck);
}
//===----------------------------------------------------------------------===//
Added: cfe/trunk/test/Analysis/CFNumber.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/CFNumber.c?rev=52799&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/CFNumber.c (added)
+++ cfe/trunk/test/Analysis/CFNumber.c Thu Jun 26 18:59:48 2008
@@ -0,0 +1,28 @@
+// RUN: clang -checker-cfref -verify %s
+
+typedef signed long CFIndex;
+typedef const struct __CFAllocator * CFAllocatorRef;
+enum { kCFNumberSInt8Type = 1, kCFNumberSInt16Type = 2,
+ kCFNumberSInt32Type = 3, kCFNumberSInt64Type = 4,
+ kCFNumberFloat32Type = 5, kCFNumberFloat64Type = 6,
+ kCFNumberCharType = 7, kCFNumberShortType = 8,
+ kCFNumberIntType = 9, kCFNumberLongType = 10,
+ kCFNumberLongLongType = 11, kCFNumberFloatType = 12,
+ kCFNumberDoubleType = 13, kCFNumberCFIndexType = 14,
+ kCFNumberNSIntegerType = 15, kCFNumberCGFloatType = 16,
+ kCFNumberMaxType = 16 };
+typedef CFIndex CFNumberType;
+typedef const struct __CFNumber * CFNumberRef;
+extern CFNumberRef CFNumberCreate(CFAllocatorRef allocator, CFNumberType theType, const void *valuePtr);
+
+#include <stdint.h>
+
+CFNumberRef f1() {
+ uint8_t x = 1;
+ return CFNumberCreate(0, kCFNumberSInt16Type, &x); // expected-warning{{An 8 bit integer is used to initialize a CFNumber object that represents a 16 bit integer. 8 bits of the CFNumber value will be garbage.}}
+}
+
+CFNumberRef f2() {
+ uint16_t x = 1;
+ return CFNumberCreate(0, kCFNumberSInt8Type, &x); // expected-warning{{A 16 bit integer is used to initialize a CFNumber object that represents an 8 bit integer. 8 bits of the input integer will be lost.}}
+}
More information about the cfe-commits
mailing list