From 04bc87683acacce119967dfa5f7c35b4ecef012a Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Thu, 26 Jun 2008 23:59:48 +0000 Subject: [PATCH] Added a simple static analysis check to look for improper uses of CFCreateNumber. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@52799 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/BasicObjCFoundationChecks.cpp | 257 +++++++++++++++++++++ lib/Analysis/BasicObjCFoundationChecks.h | 4 + lib/Analysis/GRSimpleVals.cpp | 10 +- test/Analysis/CFNumber.c | 28 +++ 4 files changed, 296 insertions(+), 3 deletions(-) create mode 100644 test/Analysis/CFNumber.c diff --git a/lib/Analysis/BasicObjCFoundationChecks.cpp b/lib/Analysis/BasicObjCFoundationChecks.cpp index cf26d1e30b..1667a21494 100644 --- a/lib/Analysis/BasicObjCFoundationChecks.cpp +++ b/lib/Analysis/BasicObjCFoundationChecks.cpp @@ -289,3 +289,260 @@ bool BasicObjCFoundationChecks::AuditNSString(NodeTy* N, return false; } + +//===----------------------------------------------------------------------===// +// Error reporting. +//===----------------------------------------------------------------------===// + +namespace { + +class VISIBILITY_HIDDEN BadCFNumberCreate : public BugTypeCacheLocation { +public: + typedef std::vector 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* 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* N); + + virtual void EmitWarnings(BugReporter& BR) { + Desc.EmitWarnings(BR); + } + +private: + + void AddError(VarDecl* V, Expr* Ex, ExplodedNode *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 + 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 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(); + } + + 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* N) { + CallExpr* CE = cast(cast(N->getLocation()).getStmt()); + Expr* Callee = CE->getCallee(); + RVal CallV = GetRVal(N->getState(), Callee); + lval::FuncVal* FuncV = dyn_cast(&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(&TheTypeVal); + + if (!V) + return false; + + uint64_t NumberKind = V->getValue().getLimitedValue(); + Optional 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(&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 *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); +} + diff --git a/lib/Analysis/BasicObjCFoundationChecks.h b/lib/Analysis/BasicObjCFoundationChecks.h index df2992b054..fafb025b54 100644 --- a/lib/Analysis/BasicObjCFoundationChecks.h +++ b/lib/Analysis/BasicObjCFoundationChecks.h @@ -33,6 +33,10 @@ class ValueStateManager; GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx, ValueStateManager* VMgr); + +GRSimpleAPICheck* CreateAuditCFNumberCreate(ASTContext& Ctx, + ValueStateManager* VMgr); + } // end clang namespace diff --git a/lib/Analysis/GRSimpleVals.cpp b/lib/Analysis/GRSimpleVals.cpp index 0933265493..c208f0c7b5 100644 --- a/lib/Analysis/GRSimpleVals.cpp +++ b/lib/Analysis/GRSimpleVals.cpp @@ -354,11 +354,15 @@ void GRSimpleVals::RegisterChecks(GRExprEngine& Eng) { 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); } //===----------------------------------------------------------------------===// diff --git a/test/Analysis/CFNumber.c b/test/Analysis/CFNumber.c new file mode 100644 index 0000000000..e31355b846 --- /dev/null +++ b/test/Analysis/CFNumber.c @@ -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 + +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.}} +} -- 2.40.0