diff options
Diffstat (limited to 'contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers')
31 files changed, 1060 insertions, 403 deletions
diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index fba14a0fc498..f66f8b75ed38 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -58,17 +58,20 @@ enum FoundationClass { FC_NSArray, FC_NSDictionary, FC_NSEnumerator, + FC_NSNull, FC_NSOrderedSet, FC_NSSet, FC_NSString }; -static FoundationClass findKnownClass(const ObjCInterfaceDecl *ID) { +static FoundationClass findKnownClass(const ObjCInterfaceDecl *ID, + bool IncludeSuperclasses = true) { static llvm::StringMap<FoundationClass> Classes; if (Classes.empty()) { Classes["NSArray"] = FC_NSArray; Classes["NSDictionary"] = FC_NSDictionary; Classes["NSEnumerator"] = FC_NSEnumerator; + Classes["NSNull"] = FC_NSNull; Classes["NSOrderedSet"] = FC_NSOrderedSet; Classes["NSSet"] = FC_NSSet; Classes["NSString"] = FC_NSString; @@ -76,7 +79,7 @@ static FoundationClass findKnownClass(const ObjCInterfaceDecl *ID) { // FIXME: Should we cache this at all? FoundationClass result = Classes.lookup(ID->getIdentifier()->getName()); - if (result == FC_None) + if (result == FC_None && IncludeSuperclasses) if (const ObjCInterfaceDecl *Super = ID->getSuperClass()) return findKnownClass(Super); @@ -88,20 +91,49 @@ static FoundationClass findKnownClass(const ObjCInterfaceDecl *ID) { //===----------------------------------------------------------------------===// namespace { - class NilArgChecker : public Checker<check::PreObjCMessage> { + class NilArgChecker : public Checker<check::PreObjCMessage, + check::PostStmt<ObjCDictionaryLiteral>, + check::PostStmt<ObjCArrayLiteral> > { mutable OwningPtr<APIMisuse> BT; - void WarnIfNilArg(CheckerContext &C, - const ObjCMethodCall &msg, unsigned Arg, - FoundationClass Class, - bool CanBeSubscript = false) const; + void warnIfNilExpr(const Expr *E, + const char *Msg, + CheckerContext &C) const; + + void warnIfNilArg(CheckerContext &C, + const ObjCMethodCall &msg, unsigned Arg, + FoundationClass Class, + bool CanBeSubscript = false) const; + + void generateBugReport(ExplodedNode *N, + StringRef Msg, + SourceRange Range, + const Expr *Expr, + CheckerContext &C) const; public: void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; + void checkPostStmt(const ObjCDictionaryLiteral *DL, + CheckerContext &C) const; + void checkPostStmt(const ObjCArrayLiteral *AL, + CheckerContext &C) const; }; } -void NilArgChecker::WarnIfNilArg(CheckerContext &C, +void NilArgChecker::warnIfNilExpr(const Expr *E, + const char *Msg, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + if (State->isNull(C.getSVal(E)).isConstrainedTrue()) { + + if (ExplodedNode *N = C.generateSink()) { + generateBugReport(N, Msg, E->getSourceRange(), E, C); + } + + } +} + +void NilArgChecker::warnIfNilArg(CheckerContext &C, const ObjCMethodCall &msg, unsigned int Arg, FoundationClass Class, @@ -111,9 +143,6 @@ void NilArgChecker::WarnIfNilArg(CheckerContext &C, if (!State->isNull(msg.getArgSVal(Arg)).isConstrainedTrue()) return; - if (!BT) - BT.reset(new APIMisuse("nil argument")); - if (ExplodedNode *N = C.generateSink()) { SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); @@ -147,14 +176,26 @@ void NilArgChecker::WarnIfNilArg(CheckerContext &C, << msg.getSelector().getAsString() << "' cannot be nil"; } } - - BugReport *R = new BugReport(*BT, os.str(), N); - R->addRange(msg.getArgSourceRange(Arg)); - bugreporter::trackNullOrUndefValue(N, msg.getArgExpr(Arg), *R); - C.emitReport(R); + + generateBugReport(N, os.str(), msg.getArgSourceRange(Arg), + msg.getArgExpr(Arg), C); } } +void NilArgChecker::generateBugReport(ExplodedNode *N, + StringRef Msg, + SourceRange Range, + const Expr *E, + CheckerContext &C) const { + if (!BT) + BT.reset(new APIMisuse("nil argument")); + + BugReport *R = new BugReport(*BT, Msg, N); + R->addRange(Range); + bugreporter::trackNullOrUndefValue(N, E, *R); + C.emitReport(R); +} + void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { const ObjCInterfaceDecl *ID = msg.getReceiverInterface(); @@ -223,26 +264,43 @@ void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg, if (S.getNameForSlot(0).equals("dictionaryWithObject") && S.getNameForSlot(1).equals("forKey")) { Arg = 0; - WarnIfNilArg(C, msg, /* Arg */1, Class); + warnIfNilArg(C, msg, /* Arg */1, Class); } else if (S.getNameForSlot(0).equals("setObject") && S.getNameForSlot(1).equals("forKey")) { Arg = 0; - WarnIfNilArg(C, msg, /* Arg */1, Class); + warnIfNilArg(C, msg, /* Arg */1, Class); } else if (S.getNameForSlot(0).equals("setObject") && S.getNameForSlot(1).equals("forKeyedSubscript")) { CanBeSubscript = true; Arg = 0; - WarnIfNilArg(C, msg, /* Arg */1, Class, CanBeSubscript); + warnIfNilArg(C, msg, /* Arg */1, Class, CanBeSubscript); } else if (S.getNameForSlot(0).equals("removeObjectForKey")) { Arg = 0; } } - // If argument is '0', report a warning. if ((Arg != InvalidArgIndex)) - WarnIfNilArg(C, msg, Arg, Class, CanBeSubscript); + warnIfNilArg(C, msg, Arg, Class, CanBeSubscript); + +} +void NilArgChecker::checkPostStmt(const ObjCArrayLiteral *AL, + CheckerContext &C) const { + unsigned NumOfElements = AL->getNumElements(); + for (unsigned i = 0; i < NumOfElements; ++i) { + warnIfNilExpr(AL->getElement(i), "Array element cannot be nil", C); + } +} + +void NilArgChecker::checkPostStmt(const ObjCDictionaryLiteral *DL, + CheckerContext &C) const { + unsigned NumOfElements = DL->getNumElements(); + for (unsigned i = 0; i < NumOfElements; ++i) { + ObjCDictionaryElement Element = DL->getKeyValueElement(i); + warnIfNilExpr(Element.Key, "Dictionary key cannot be nil", C); + warnIfNilExpr(Element.Value, "Dictionary value cannot be nil", C); + } } //===----------------------------------------------------------------------===// @@ -729,12 +787,31 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg, // Improves the modeling of loops over Cocoa collections. //===----------------------------------------------------------------------===// +// The map from container symbol to the container count symbol. +// We currently will remember the last countainer count symbol encountered. +REGISTER_MAP_WITH_PROGRAMSTATE(ContainerCountMap, SymbolRef, SymbolRef) +REGISTER_MAP_WITH_PROGRAMSTATE(ContainerNonEmptyMap, SymbolRef, bool) + namespace { class ObjCLoopChecker - : public Checker<check::PostStmt<ObjCForCollectionStmt> > { - + : public Checker<check::PostStmt<ObjCForCollectionStmt>, + check::PostObjCMessage, + check::DeadSymbols, + check::PointerEscape > { + mutable IdentifierInfo *CountSelectorII; + + bool isCollectionCountMethod(const ObjCMethodCall &M, + CheckerContext &C) const; + public: + ObjCLoopChecker() : CountSelectorII(0) {} void checkPostStmt(const ObjCForCollectionStmt *FCS, CheckerContext &C) const; + void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; + void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; + ProgramStateRef checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const; }; } @@ -819,23 +896,240 @@ static ProgramStateRef checkElementNonNil(CheckerContext &C, return State->assume(Val.castAs<DefinedOrUnknownSVal>(), true); } +/// Returns NULL state if the collection is known to contain elements +/// (or is known not to contain elements if the Assumption parameter is false.) +static ProgramStateRef +assumeCollectionNonEmpty(CheckerContext &C, ProgramStateRef State, + SymbolRef CollectionS, bool Assumption) { + if (!State || !CollectionS) + return State; + + const SymbolRef *CountS = State->get<ContainerCountMap>(CollectionS); + if (!CountS) { + const bool *KnownNonEmpty = State->get<ContainerNonEmptyMap>(CollectionS); + if (!KnownNonEmpty) + return State->set<ContainerNonEmptyMap>(CollectionS, Assumption); + return (Assumption == *KnownNonEmpty) ? State : NULL; + } + + SValBuilder &SvalBuilder = C.getSValBuilder(); + SVal CountGreaterThanZeroVal = + SvalBuilder.evalBinOp(State, BO_GT, + nonloc::SymbolVal(*CountS), + SvalBuilder.makeIntVal(0, (*CountS)->getType()), + SvalBuilder.getConditionType()); + Optional<DefinedSVal> CountGreaterThanZero = + CountGreaterThanZeroVal.getAs<DefinedSVal>(); + if (!CountGreaterThanZero) { + // The SValBuilder cannot construct a valid SVal for this condition. + // This means we cannot properly reason about it. + return State; + } + + return State->assume(*CountGreaterThanZero, Assumption); +} + +static ProgramStateRef +assumeCollectionNonEmpty(CheckerContext &C, ProgramStateRef State, + const ObjCForCollectionStmt *FCS, + bool Assumption) { + if (!State) + return NULL; + + SymbolRef CollectionS = + State->getSVal(FCS->getCollection(), C.getLocationContext()).getAsSymbol(); + return assumeCollectionNonEmpty(C, State, CollectionS, Assumption); +} + + +/// If the fist block edge is a back edge, we are reentering the loop. +static bool alreadyExecutedAtLeastOneLoopIteration(const ExplodedNode *N, + const ObjCForCollectionStmt *FCS) { + if (!N) + return false; + + ProgramPoint P = N->getLocation(); + if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) { + if (BE->getSrc()->getLoopTarget() == FCS) + return true; + return false; + } + + // Keep looking for a block edge. + for (ExplodedNode::const_pred_iterator I = N->pred_begin(), + E = N->pred_end(); I != E; ++I) { + if (alreadyExecutedAtLeastOneLoopIteration(*I, FCS)) + return true; + } + + return false; +} + void ObjCLoopChecker::checkPostStmt(const ObjCForCollectionStmt *FCS, CheckerContext &C) const { + ProgramStateRef State = C.getState(); + // Check if this is the branch for the end of the loop. SVal CollectionSentinel = C.getSVal(FCS); - if (CollectionSentinel.isZeroConstant()) - return; - - ProgramStateRef State = C.getState(); - State = checkCollectionNonNil(C, State, FCS); - State = checkElementNonNil(C, State, FCS); + if (CollectionSentinel.isZeroConstant()) { + if (!alreadyExecutedAtLeastOneLoopIteration(C.getPredecessor(), FCS)) + State = assumeCollectionNonEmpty(C, State, FCS, /*Assumption*/false); + // Otherwise, this is a branch that goes through the loop body. + } else { + State = checkCollectionNonNil(C, State, FCS); + State = checkElementNonNil(C, State, FCS); + State = assumeCollectionNonEmpty(C, State, FCS, /*Assumption*/true); + } + if (!State) C.generateSink(); else if (State != C.getState()) C.addTransition(State); } +bool ObjCLoopChecker::isCollectionCountMethod(const ObjCMethodCall &M, + CheckerContext &C) const { + Selector S = M.getSelector(); + // Initialize the identifiers on first use. + if (!CountSelectorII) + CountSelectorII = &C.getASTContext().Idents.get("count"); + + // If the method returns collection count, record the value. + if (S.isUnarySelector() && + (S.getIdentifierInfoForSlot(0) == CountSelectorII)) + return true; + + return false; +} + +void ObjCLoopChecker::checkPostObjCMessage(const ObjCMethodCall &M, + CheckerContext &C) const { + if (!M.isInstanceMessage()) + return; + + const ObjCInterfaceDecl *ClassID = M.getReceiverInterface(); + if (!ClassID) + return; + + FoundationClass Class = findKnownClass(ClassID); + if (Class != FC_NSDictionary && + Class != FC_NSArray && + Class != FC_NSSet && + Class != FC_NSOrderedSet) + return; + + SymbolRef ContainerS = M.getReceiverSVal().getAsSymbol(); + if (!ContainerS) + return; + + // If we are processing a call to "count", get the symbolic value returned by + // a call to "count" and add it to the map. + if (!isCollectionCountMethod(M, C)) + return; + + const Expr *MsgExpr = M.getOriginExpr(); + SymbolRef CountS = C.getSVal(MsgExpr).getAsSymbol(); + if (CountS) { + ProgramStateRef State = C.getState(); + + C.getSymbolManager().addSymbolDependency(ContainerS, CountS); + State = State->set<ContainerCountMap>(ContainerS, CountS); + + if (const bool *NonEmpty = State->get<ContainerNonEmptyMap>(ContainerS)) { + State = State->remove<ContainerNonEmptyMap>(ContainerS); + State = assumeCollectionNonEmpty(C, State, ContainerS, *NonEmpty); + } + + C.addTransition(State); + } + return; +} + +static SymbolRef getMethodReceiverIfKnownImmutable(const CallEvent *Call) { + const ObjCMethodCall *Message = dyn_cast_or_null<ObjCMethodCall>(Call); + if (!Message) + return 0; + + const ObjCMethodDecl *MD = Message->getDecl(); + if (!MD) + return 0; + + const ObjCInterfaceDecl *StaticClass; + if (isa<ObjCProtocolDecl>(MD->getDeclContext())) { + // We can't find out where the method was declared without doing more work. + // Instead, see if the receiver is statically typed as a known immutable + // collection. + StaticClass = Message->getOriginExpr()->getReceiverInterface(); + } else { + StaticClass = MD->getClassInterface(); + } + + if (!StaticClass) + return 0; + + switch (findKnownClass(StaticClass, /*IncludeSuper=*/false)) { + case FC_None: + return 0; + case FC_NSArray: + case FC_NSDictionary: + case FC_NSEnumerator: + case FC_NSNull: + case FC_NSOrderedSet: + case FC_NSSet: + case FC_NSString: + break; + } + + return Message->getReceiverSVal().getAsSymbol(); +} + +ProgramStateRef +ObjCLoopChecker::checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const { + SymbolRef ImmutableReceiver = getMethodReceiverIfKnownImmutable(Call); + + // Remove the invalidated symbols form the collection count map. + for (InvalidatedSymbols::const_iterator I = Escaped.begin(), + E = Escaped.end(); + I != E; ++I) { + SymbolRef Sym = *I; + + // Don't invalidate this symbol's count if we know the method being called + // is declared on an immutable class. This isn't completely correct if the + // receiver is also passed as an argument, but in most uses of NSArray, + // NSDictionary, etc. this isn't likely to happen in a dangerous way. + if (Sym == ImmutableReceiver) + continue; + + // The symbol escaped. Pessimistically, assume that the count could have + // changed. + State = State->remove<ContainerCountMap>(Sym); + State = State->remove<ContainerNonEmptyMap>(Sym); + } + return State; +} + +void ObjCLoopChecker::checkDeadSymbols(SymbolReaper &SymReaper, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + // Remove the dead symbols from the collection count map. + ContainerCountMapTy Tracked = State->get<ContainerCountMap>(); + for (ContainerCountMapTy::iterator I = Tracked.begin(), + E = Tracked.end(); I != E; ++I) { + SymbolRef Sym = I->first; + if (SymReaper.isDead(Sym)) { + State = State->remove<ContainerCountMap>(Sym); + State = State->remove<ContainerNonEmptyMap>(Sym); + } + } + + C.addTransition(State); +} + namespace { /// \class ObjCNonNilReturnValueChecker /// \brief The checker restricts the return values of APIs known to @@ -845,6 +1139,7 @@ class ObjCNonNilReturnValueChecker mutable bool Initialized; mutable Selector ObjectAtIndex; mutable Selector ObjectAtIndexedSubscript; + mutable Selector NullSelector; public: ObjCNonNilReturnValueChecker() : Initialized(false) {} @@ -870,6 +1165,7 @@ void ObjCNonNilReturnValueChecker::checkPostObjCMessage(const ObjCMethodCall &M, ASTContext &Ctx = C.getASTContext(); ObjectAtIndex = GetUnarySelector("objectAtIndex", Ctx); ObjectAtIndexedSubscript = GetUnarySelector("objectAtIndexedSubscript", Ctx); + NullSelector = GetNullarySelector("null", Ctx); } // Check the receiver type. @@ -889,10 +1185,11 @@ void ObjCNonNilReturnValueChecker::checkPostObjCMessage(const ObjCMethodCall &M, State = assumeExprIsNonNull(M.getOriginExpr(), State, C); } + FoundationClass Cl = findKnownClass(Interface); + // Objects returned from // [NSArray|NSOrderedSet]::[ObjectAtIndex|ObjectAtIndexedSubscript] // are never 'nil'. - FoundationClass Cl = findKnownClass(Interface); if (Cl == FC_NSArray || Cl == FC_NSOrderedSet) { Selector Sel = M.getSelector(); if (Sel == ObjectAtIndex || Sel == ObjectAtIndexedSubscript) { @@ -900,6 +1197,14 @@ void ObjCNonNilReturnValueChecker::checkPostObjCMessage(const ObjCMethodCall &M, State = assumeExprIsNonNull(M.getOriginExpr(), State, C); } } + + // Objects returned from [NSNull null] are not nil. + if (Cl == FC_NSNull) { + if (M.getSelector() == NullSelector) { + // Go ahead and assume the value is non-nil. + State = assumeExprIsNonNull(M.getOriginExpr(), State, C); + } + } } C.addTransition(State); } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index a3327d8b3194..a87104984883 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -37,14 +37,15 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE, if (!FD) return false; - unsigned id = FD->getBuiltinID(); - - if (!id) + switch (FD->getBuiltinID()) { + default: return false; - switch (id) { - case Builtin::BI__builtin_expect: { + case Builtin::BI__builtin_expect: + case Builtin::BI__builtin_addressof: { // For __builtin_expect, just return the value of the subexpression. + // __builtin_addressof is going from a reference to a pointer, but those + // are represented the same way in the analyzer. assert (CE->arg_begin() != CE->arg_end()); SVal X = state->getSVal(*(CE->arg_begin()), LCtx); C.addTransition(state->BindExpr(CE, LCtx, X)); @@ -73,9 +74,24 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE, C.addTransition(state->BindExpr(CE, LCtx, loc::MemRegionVal(R))); return true; } - } - return false; + case Builtin::BI__builtin_object_size: { + // This must be resolvable at compile time, so we defer to the constant + // evaluator for a value. + SVal V = UnknownVal(); + llvm::APSInt Result; + if (CE->EvaluateAsInt(Result, C.getASTContext(), Expr::SE_NoSideEffects)) { + // Make sure the result has the correct type. + SValBuilder &SVB = C.getSValBuilder(); + BasicValueFactory &BVF = SVB.getBasicValueFactory(); + BVF.getAPSIntType(CE->getType()).apply(Result); + V = SVB.makeIntVal(Result); + } + + C.addTransition(state->BindExpr(CE, LCtx, V)); + return true; + } + } } void ento::registerBuiltinFunctionChecker(CheckerManager &mgr) { diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index aa1ca6f2f809..c3736d7e5d71 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -141,8 +141,9 @@ public: SVal val) const; static ProgramStateRef InvalidateBuffer(CheckerContext &C, - ProgramStateRef state, - const Expr *Ex, SVal V); + ProgramStateRef state, + const Expr *Ex, SVal V, + bool IsSourceBuffer); static bool SummarizeRegion(raw_ostream &os, ASTContext &Ctx, const MemRegion *MR); @@ -231,7 +232,7 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, return NULL; if (!BT_Null) - BT_Null.reset(new BuiltinBug("Unix API", + BT_Null.reset(new BuiltinBug(categories::UnixAPI, "Null pointer argument in call to byte string function")); SmallString<80> buf; @@ -525,7 +526,7 @@ void CStringChecker::emitOverlapBug(CheckerContext &C, ProgramStateRef state, return; if (!BT_Overlap) - BT_Overlap.reset(new BugType("Unix API", "Improper arguments")); + BT_Overlap.reset(new BugType(categories::UnixAPI, "Improper arguments")); // Generate a report for this bug. BugReport *report = @@ -661,7 +662,7 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C, if (Recorded) return *Recorded; } - + // Otherwise, get a new symbol and update the state. SValBuilder &svalBuilder = C.getSValBuilder(); QualType sizeTy = svalBuilder.getContext().getSizeType(); @@ -669,8 +670,21 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C, MR, Ex, sizeTy, C.blockCount()); - if (!hypothetical) + if (!hypothetical) { + if (Optional<NonLoc> strLn = strLength.getAs<NonLoc>()) { + // In case of unbounded calls strlen etc bound the range to SIZE_MAX/4 + BasicValueFactory &BVF = svalBuilder.getBasicValueFactory(); + const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy); + llvm::APSInt fourInt = APSIntType(maxValInt).getValue(4); + const llvm::APSInt *maxLengthInt = BVF.evalAPSInt(BO_Div, maxValInt, + fourInt); + NonLoc maxLength = svalBuilder.makeIntVal(*maxLengthInt); + SVal evalLength = svalBuilder.evalBinOpNN(state, BO_LE, *strLn, + maxLength, sizeTy); + state = state->assume(evalLength.castAs<DefinedOrUnknownSVal>(), true); + } state = state->set<CStringLength>(MR, strLength); + } return strLength; } @@ -689,7 +703,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, if (ExplodedNode *N = C.addTransition(state)) { if (!BT_NotCString) - BT_NotCString.reset(new BuiltinBug("Unix API", + BT_NotCString.reset(new BuiltinBug(categories::UnixAPI, "Argument is not a null-terminated string.")); SmallString<120> buf; @@ -749,7 +763,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, if (ExplodedNode *N = C.addTransition(state)) { if (!BT_NotCString) - BT_NotCString.reset(new BuiltinBug("Unix API", + BT_NotCString.reset(new BuiltinBug(categories::UnixAPI, "Argument is not a null-terminated string.")); SmallString<120> buf; @@ -796,8 +810,9 @@ const StringLiteral *CStringChecker::getCStringLiteral(CheckerContext &C, } ProgramStateRef CStringChecker::InvalidateBuffer(CheckerContext &C, - ProgramStateRef state, - const Expr *E, SVal V) { + ProgramStateRef state, + const Expr *E, SVal V, + bool IsSourceBuffer) { Optional<Loc> L = V.getAs<Loc>(); if (!L) return state; @@ -817,8 +832,20 @@ ProgramStateRef CStringChecker::InvalidateBuffer(CheckerContext &C, // Invalidate this region. const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); - return state->invalidateRegions(R, E, C.blockCount(), LCtx, - /*CausesPointerEscape*/ false); + + bool CausesPointerEscape = false; + RegionAndSymbolInvalidationTraits ITraits; + // Invalidate and escape only indirect regions accessible through the source + // buffer. + if (IsSourceBuffer) { + ITraits.setTrait(R, + RegionAndSymbolInvalidationTraits::TK_PreserveContents); + ITraits.setTrait(R, RegionAndSymbolInvalidationTraits::TK_SuppressEscape); + CausesPointerEscape = true; + } + + return state->invalidateRegions(R, E, C.blockCount(), LCtx, + CausesPointerEscape, 0, 0, &ITraits); } // If we have a non-region value by chance, just remove the binding. @@ -955,13 +982,20 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, state = state->BindExpr(CE, LCtx, destVal); } - // Invalidate the destination. + // Invalidate the destination (regular invalidation without pointer-escaping + // the address of the top-level region). // FIXME: Even if we can't perfectly model the copy, we should see if we // can use LazyCompoundVals to copy the source values into the destination. // This would probably remove any existing bindings past the end of the // copied region, but that's still an improvement over blank invalidation. - state = InvalidateBuffer(C, state, Dest, - state->getSVal(Dest, C.getLocationContext())); + state = InvalidateBuffer(C, state, Dest, C.getSVal(Dest), + /*IsSourceBuffer*/false); + + // Invalidate the source (const-invalidation without const-pointer-escaping + // the address of the top-level region). + state = InvalidateBuffer(C, state, Source, C.getSVal(Source), + /*IsSourceBuffer*/true); + C.addTransition(state); } } @@ -1564,13 +1598,19 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, Result = lastElement; } - // Invalidate the destination. This must happen before we set the C string - // length because invalidation will clear the length. + // Invalidate the destination (regular invalidation without pointer-escaping + // the address of the top-level region). This must happen before we set the + // C string length because invalidation will clear the length. // FIXME: Even if we can't perfectly model the copy, we should see if we // can use LazyCompoundVals to copy the source values into the destination. // This would probably remove any existing bindings past the end of the // string, but that's still an improvement over blank invalidation. - state = InvalidateBuffer(C, state, Dst, *dstRegVal); + state = InvalidateBuffer(C, state, Dst, *dstRegVal, + /*IsSourceBuffer*/false); + + // Invalidate the source (const-invalidation without const-pointer-escaping + // the address of the top-level region). + state = InvalidateBuffer(C, state, srcExpr, srcVal, /*IsSourceBuffer*/true); // Set the C string length of the destination, if we know it. if (isBounded && !isAppending) { @@ -1792,7 +1832,8 @@ void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { // Invalidate the search string, representing the change of one delimiter // character to NUL. - State = InvalidateBuffer(C, State, SearchStrPtr, Result); + State = InvalidateBuffer(C, State, SearchStrPtr, Result, + /*IsSourceBuffer*/false); // Overwrite the search string pointer. The new value is either an address // further along in the same string, or NULL if there are no more tokens. @@ -2018,10 +2059,7 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR, #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) {\ - static CStringChecker *TheChecker = 0; \ - if (TheChecker == 0) \ - TheChecker = mgr.registerChecker<CStringChecker>(); \ - TheChecker->Filter.Check##name = true; \ + mgr.registerChecker<CStringChecker>()->Filter.Check##name = true; \ } REGISTER_CHECKER(CStringNullArg) diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp index 92c0eef3e880..d29a12a90eeb 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp @@ -141,7 +141,6 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { if (containsBadStrncatPattern(CE)) { const Expr *DstArg = CE->getArg(0); const Expr *LenArg = CE->getArg(2); - SourceRange R = LenArg->getSourceRange(); PathDiagnosticLocation Loc = PathDiagnosticLocation::createBegin(LenArg, BR.getSourceManager(), AC); @@ -159,7 +158,7 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { os << "se a safer 'strlcat' API"; BR.EmitBasicReport(FD, "Anti-pattern in the argument", "C String API", - os.str(), Loc, &R, 1); + os.str(), Loc, LenArg->getSourceRange()); } } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index 4965d2299616..fefcbe7b09cb 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -28,21 +28,26 @@ using namespace ento; namespace { class CallAndMessageChecker - : public Checker< check::PreStmt<CallExpr>, check::PreObjCMessage, + : public Checker< check::PreStmt<CallExpr>, + check::PreStmt<CXXDeleteExpr>, + check::PreObjCMessage, check::PreCall > { mutable OwningPtr<BugType> BT_call_null; mutable OwningPtr<BugType> BT_call_undef; mutable OwningPtr<BugType> BT_cxx_call_null; mutable OwningPtr<BugType> BT_cxx_call_undef; mutable OwningPtr<BugType> BT_call_arg; + mutable OwningPtr<BugType> BT_cxx_delete_undef; mutable OwningPtr<BugType> BT_msg_undef; mutable OwningPtr<BugType> BT_objc_prop_undef; mutable OwningPtr<BugType> BT_objc_subscript_undef; mutable OwningPtr<BugType> BT_msg_arg; mutable OwningPtr<BugType> BT_msg_ret; + mutable OwningPtr<BugType> BT_call_few_args; public: void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; + void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const; void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; @@ -244,11 +249,36 @@ void CallAndMessageChecker::checkPreStmt(const CallExpr *CE, BT_call_null.reset( new BuiltinBug("Called function pointer is null (null dereference)")); emitBadCall(BT_call_null.get(), C, Callee); + return; } C.addTransition(StNonNull); } +void CallAndMessageChecker::checkPreStmt(const CXXDeleteExpr *DE, + CheckerContext &C) const { + + SVal Arg = C.getSVal(DE->getArgument()); + if (Arg.isUndef()) { + StringRef Desc; + ExplodedNode *N = C.generateSink(); + if (!N) + return; + if (!BT_cxx_delete_undef) + BT_cxx_delete_undef.reset(new BuiltinBug("Uninitialized argument value")); + if (DE->isArrayFormAsWritten()) + Desc = "Argument to 'delete[]' is uninitialized"; + else + Desc = "Argument to 'delete' is uninitialized"; + BugType *BT = BT_cxx_delete_undef.get(); + BugReport *R = new BugReport(*BT, Desc, N); + bugreporter::trackNullOrUndefValue(N, DE, *R); + C.emitReport(R); + return; + } +} + + void CallAndMessageChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); @@ -280,11 +310,33 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call, State = StNonNull; } + const Decl *D = Call.getDecl(); + if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) { + // If we have a declaration, we can make sure we pass enough parameters to + // the function. + unsigned Params = FD->getNumParams(); + if (Call.getNumArgs() < Params) { + ExplodedNode *N = C.generateSink(); + if (!N) + return; + + LazyInit_BT("Function call with too few arguments", BT_call_few_args); + + SmallString<512> Str; + llvm::raw_svector_ostream os(Str); + os << "Function taking " << Params << " argument" + << (Params == 1 ? "" : "s") << " is called with less (" + << Call.getNumArgs() << ")"; + + BugReport *R = new BugReport(*BT_call_few_args, os.str(), N); + C.emitReport(R); + } + } + // Don't check for uninitialized field values in arguments if the // caller has a body that is available and we have the chance to inline it. // This is a hack, but is a reasonable compromise betweens sometimes warning // and sometimes not depending on if we decide to inline a function. - const Decl *D = Call.getDecl(); const bool checkUninitFields = !(C.getAnalysisManager().shouldInlineCall() && (D && D->getBody())); @@ -395,8 +447,7 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C, static bool supportsNilWithFloatRet(const llvm::Triple &triple) { return (triple.getVendor() == llvm::Triple::Apple && - (triple.getOS() == llvm::Triple::IOS || - !triple.isMacOSXVersionLT(10,5))); + (triple.isiOS() || !triple.isMacOSXVersionLT(10,5))); } void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 63080ea230f8..415d3ecc39b5 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -283,7 +283,7 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) { PathDiagnosticLocation::createBegin(FS, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), bugType, "Security", os.str(), - FSLoc, ranges.data(), ranges.size()); + FSLoc, ranges); } //===----------------------------------------------------------------------===// @@ -297,8 +297,7 @@ void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) { if (!filter.check_gets) return; - const FunctionProtoType *FPT - = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens()); + const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>(); if (!FPT) return; @@ -307,7 +306,7 @@ void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) { return; // Is the argument a 'char*'? - const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(0)); + const PointerType *PT = FPT->getArgType(0)->getAs<PointerType>(); if (!PT) return; @@ -315,7 +314,6 @@ void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) { return; // Issue a warning. - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), @@ -323,7 +321,7 @@ void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) { "Security", "Call to function 'gets' is extremely insecure as it can " "always result in a buffer overflow", - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -335,8 +333,7 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) { if (!filter.check_getpw) return; - const FunctionProtoType *FPT - = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens()); + const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>(); if (!FPT) return; @@ -349,7 +346,7 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) { return; // Verify the second argument type is char*. - const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(1)); + const PointerType *PT = FPT->getArgType(1)->getAs<PointerType>(); if (!PT) return; @@ -357,7 +354,6 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) { return; // Issue a warning. - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), @@ -365,7 +361,7 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) { "Security", "The getpw() function is dangerous as it may overflow the " "provided buffer. It is obsoleted by getpwuid().", - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -381,8 +377,7 @@ void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) { return; } - const FunctionProtoType *FPT - = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens()); + const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>(); if(!FPT) return; @@ -391,7 +386,7 @@ void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) { return; // Verify that the argument is Pointer Type. - const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(0)); + const PointerType *PT = FPT->getArgType(0)->getAs<PointerType>(); if (!PT) return; @@ -400,7 +395,6 @@ void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) { return; // Issue a waring. - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), @@ -409,7 +403,7 @@ void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) { "Call to function 'mktemp' is insecure as it always " "creates or uses insecure temporary file. Use 'mkstemp' " "instead", - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } @@ -473,7 +467,6 @@ void WalkAST::checkCall_mkstemp(const CallExpr *CE, const FunctionDecl *FD) { return; // Issue a warning. - SourceRange R = strArg->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); SmallString<512> buf; @@ -492,7 +485,7 @@ void WalkAST::checkCall_mkstemp(const CallExpr *CE, const FunctionDecl *FD) { out << ')'; BR.EmitBasicReport(AC->getDecl(), "Insecure temporary file creation", "Security", - out.str(), CELoc, &R, 1); + out.str(), CELoc, strArg->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -509,7 +502,6 @@ void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) { return; // Issue a warning. - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), @@ -520,7 +512,7 @@ void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) { "provide bounding of the memory buffer. Replace " "unbounded copy functions with analogous functions that " "support length arguments such as 'strlcpy'. CWE-119.", - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -537,7 +529,6 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) { return; // Issue a warning. - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), @@ -548,15 +539,14 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) { "provide bounding of the memory buffer. Replace " "unbounded copy functions with analogous functions that " "support length arguments such as 'strlcat'. CWE-119.", - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// // Common check for str* functions with no bounds parameters. //===----------------------------------------------------------------------===// bool WalkAST::checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD) { - const FunctionProtoType *FPT - = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens()); + const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>(); if (!FPT) return false; @@ -568,7 +558,7 @@ bool WalkAST::checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD) { // Verify the type for both arguments. for (int i = 0; i < 2; i++) { // Verify that the arguments are pointers. - const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(i)); + const PointerType *PT = FPT->getArgType(i)->getAs<PointerType>(); if (!PT) return false; @@ -590,15 +580,14 @@ void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) { if (!filter.check_rand || !CheckRand) return; - const FunctionProtoType *FTP - = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens()); + const FunctionProtoType *FTP = FD->getType()->getAs<FunctionProtoType>(); if (!FTP) return; if (FTP->getNumArgs() == 1) { // Is the argument an 'unsigned short *'? // (Actually any integer type is allowed.) - const PointerType *PT = dyn_cast<PointerType>(FTP->getArgType(0)); + const PointerType *PT = FTP->getArgType(0)->getAs<PointerType>(); if (!PT) return; @@ -619,11 +608,10 @@ void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) { << "' is obsolete because it implements a poor random number generator." << " Use 'arc4random' instead"; - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), os1.str(), "Security", os2.str(), - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -635,8 +623,7 @@ void WalkAST::checkCall_random(const CallExpr *CE, const FunctionDecl *FD) { if (!CheckRand || !filter.check_rand) return; - const FunctionProtoType *FTP - = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens()); + const FunctionProtoType *FTP = FD->getType()->getAs<FunctionProtoType>(); if (!FTP) return; @@ -645,7 +632,6 @@ void WalkAST::checkCall_random(const CallExpr *CE, const FunctionDecl *FD) { return; // Issue a warning. - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), @@ -653,7 +639,7 @@ void WalkAST::checkCall_random(const CallExpr *CE, const FunctionDecl *FD) { "Security", "The 'random' function produces a sequence of values that " "an adversary may be able to predict. Use 'arc4random' " - "instead", CELoc, &R, 1); + "instead", CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -666,7 +652,6 @@ void WalkAST::checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD) { return; // All calls to vfork() are insecure, issue a warning. - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), @@ -677,7 +662,7 @@ void WalkAST::checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD) { "denial of service situations in the parent process. " "Replace calls to vfork with calls to the safer " "'posix_spawn' function", - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -713,8 +698,7 @@ void WalkAST::checkUncheckedReturnValue(CallExpr *CE) { if (identifierid >= num_setids) return; - const FunctionProtoType *FTP - = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens()); + const FunctionProtoType *FTP = FD->getType()->getAs<FunctionProtoType>(); if (!FTP) return; @@ -739,11 +723,10 @@ void WalkAST::checkUncheckedReturnValue(CallExpr *CE) { << "' is not checked. If an error occurs in '" << *FD << "', the following code may execute with unexpected privileges"; - SourceRange R = CE->getCallee()->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), os1.str(), "Security", os2.str(), - CELoc, &R, 1); + CELoc, CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp index f2c50501a65c..1207b67c97fa 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp @@ -60,15 +60,14 @@ void WalkAST::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) { if (!isa<DeclRefExpr>(ArgEx->IgnoreParens())) return; - SourceRange R = ArgEx->getSourceRange(); PathDiagnosticLocation ELoc = PathDiagnosticLocation::createBegin(E, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), "Potential unintended use of sizeof() on pointer type", - "Logic", + categories::LogicError, "The code calls sizeof() on a pointer type. " "This can produce an unexpected result.", - ELoc, &R, 1); + ELoc, ArgEx->getSourceRange()); } } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp index a9dd19a395c5..956dca7d9258 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp @@ -190,7 +190,7 @@ public: /// /// \returns true if the call has been successfully evaluated /// and false otherwise. Note, that only one checker can evaluate a call. If - /// more then one checker claim that they can evaluate the same call the + /// more than one checker claims that they can evaluate the same call the /// first one wins. /// /// eval::Call diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td index fc35b223ee7f..862212d532fd 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td @@ -100,6 +100,10 @@ def CastToStructChecker : Checker<"CastToStruct">, HelpText<"Check for cast from non-struct pointer to struct pointer">, DescFile<"CastToStructChecker.cpp">; +def IdenticalExprChecker : Checker<"IdenticalExpr">, + HelpText<"Warn about unintended use of identical expressions in operators">, + DescFile<"IdenticalExprChecker.cpp">; + def FixedAddressChecker : Checker<"FixedAddr">, HelpText<"Check for assignment of a fixed address to a pointer">, DescFile<"FixedAddressChecker.cpp">; @@ -538,4 +542,8 @@ def ExprInspectionChecker : Checker<"ExprInspection">, HelpText<"Check the analyzer's understanding of expressions">, DescFile<"ExprInspectionChecker.cpp">; +def ExplodedGraphViewer : Checker<"ViewExplodedGraph">, + HelpText<"View Exploded Graphs using GraphViz">, + DescFile<"DebugCheckers.cpp">; + } // end "debug" diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ClangSACheckers.h b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ClangSACheckers.h index bea908dfa687..de2ebce52c02 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ClangSACheckers.h +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ClangSACheckers.h @@ -15,7 +15,7 @@ #ifndef LLVM_CLANG_SA_LIB_CHECKERS_CLANGSACHECKERS_H #define LLVM_CLANG_SA_LIB_CHECKERS_CLANGSACHECKERS_H -#include "clang/StaticAnalyzer/Checkers/CommonBugCategories.h" +#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" namespace clang { diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CommonBugCategories.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CommonBugCategories.cpp deleted file mode 100644 index e2a8ea616611..000000000000 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CommonBugCategories.cpp +++ /dev/null @@ -1,18 +0,0 @@ -//=--- CommonBugCategories.cpp - Provides common issue categories -*- C++ -*-=// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -// Common strings used for the "category" of many static analyzer issues. -namespace clang { namespace ento { namespace categories { - -const char *CoreFoundationObjectiveC = "Core Foundation/Objective-C"; -const char *MemoryCoreFoundationObjectiveC = - "Memory (Core Foundation/Objective-C)"; -const char *UnixAPI = "Unix API"; -}}} - diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index f336a6e68051..9d855ce649ac 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -18,7 +18,6 @@ #include "clang/AST/ParentMap.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Analysis/Analyses/LiveVariables.h" -#include "clang/Analysis/Visitors/CFGRecStmtDeclVisitor.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" @@ -86,10 +85,9 @@ void ReachableCode::computeReachableBlocks() { SmallVector<const CFGBlock*, 10> worklist; worklist.push_back(&cfg.getEntry()); - + while (!worklist.empty()) { - const CFGBlock *block = worklist.back(); - worklist.pop_back(); + const CFGBlock *block = worklist.pop_back_val(); llvm::BitVector::reference isReachable = reachable[block->getBlockID()]; if (isReachable) continue; @@ -391,26 +389,24 @@ public: //===----------------------------------------------------------------------===// namespace { -class FindEscaped : public CFGRecStmtDeclVisitor<FindEscaped>{ - CFG *cfg; +class FindEscaped { public: - FindEscaped(CFG *c) : cfg(c) {} - - CFG& getCFG() { return *cfg; } - llvm::SmallPtrSet<const VarDecl*, 20> Escaped; - void VisitUnaryOperator(UnaryOperator* U) { - // Check for '&'. Any VarDecl whose value has its address-taken we - // treat as escaped. - Expr *E = U->getSubExpr()->IgnoreParenCasts(); - if (U->getOpcode() == UO_AddrOf) - if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) - if (VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) { - Escaped.insert(VD); - return; - } - Visit(E); + void operator()(const Stmt *S) { + // Check for '&'. Any VarDecl whose address has been taken we treat as + // escaped. + // FIXME: What about references? + const UnaryOperator *U = dyn_cast<UnaryOperator>(S); + if (!U) + return; + if (U->getOpcode() != UO_AddrOf) + return; + + const Expr *E = U->getSubExpr()->IgnoreParenCasts(); + if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) + if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) + Escaped.insert(VD); } }; } // end anonymous namespace @@ -438,8 +434,8 @@ public: CFG &cfg = *mgr.getCFG(D); AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D); ParentMap &pmap = mgr.getParentMap(D); - FindEscaped FS(&cfg); - FS.getCFG().VisitBlockStmts(FS); + FindEscaped FS; + cfg.VisitBlockStmts(FS); DeadStoreObs A(cfg, BR.getContext(), BR, AC, pmap, FS.Escaped); L->runOnAllBlocks(A); } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp index fe12866e51c3..a2c8d1fd8f39 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp @@ -17,6 +17,8 @@ #include "clang/Analysis/CallGraph.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "llvm/Support/Process.h" using namespace clang; @@ -152,25 +154,29 @@ void ento::registerCallGraphDumper(CheckerManager &mgr) { namespace { class ConfigDumper : public Checker< check::EndOfTranslationUnit > { + typedef AnalyzerOptions::ConfigTable Table; + + static int compareEntry(const Table::MapEntryTy *const *LHS, + const Table::MapEntryTy *const *RHS) { + return (*LHS)->getKey().compare((*RHS)->getKey()); + } + public: void checkEndOfTranslationUnit(const TranslationUnitDecl *TU, AnalysisManager& mgr, BugReporter &BR) const { + const Table &Config = mgr.options.Config; - const AnalyzerOptions::ConfigTable &Config = mgr.options.Config; - AnalyzerOptions::ConfigTable::const_iterator I = - Config.begin(), E = Config.end(); + SmallVector<const Table::MapEntryTy *, 32> Keys; + for (Table::const_iterator I = Config.begin(), E = Config.end(); I != E; + ++I) + Keys.push_back(&*I); + llvm::array_pod_sort(Keys.begin(), Keys.end(), compareEntry); - std::vector<StringRef> Keys; - for (; I != E ; ++I) { Keys.push_back(I->getKey()); } - sort(Keys.begin(), Keys.end()); - llvm::errs() << "[config]\n"; - for (unsigned i = 0, n = Keys.size(); i < n ; ++i) { - StringRef Key = Keys[i]; - I = Config.find(Key); - llvm::errs() << Key << " = " << I->second << '\n'; - } + for (unsigned I = 0, E = Keys.size(); I != E; ++I) + llvm::errs() << Keys[I]->getKey() << " = " << Keys[I]->second << '\n'; + llvm::errs() << "[stats]\n" << "num-entries = " << Keys.size() << '\n'; } }; @@ -179,3 +185,22 @@ public: void ento::registerConfigDumper(CheckerManager &mgr) { mgr.registerChecker<ConfigDumper>(); } + +//===----------------------------------------------------------------------===// +// ExplodedGraph Viewer +//===----------------------------------------------------------------------===// + +namespace { +class ExplodedGraphViewer : public Checker< check::EndAnalysis > { +public: + ExplodedGraphViewer() {} + void checkEndAnalysis(ExplodedGraph &G, BugReporter &B,ExprEngine &Eng) const { + Eng.ViewGraph(0); + } +}; + +} + +void ento::registerExplodedGraphViewer(CheckerManager &mgr) { + mgr.registerChecker<ExplodedGraphViewer>(); +} diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp index 6d3dd1e42f02..b43dc18c2179 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp @@ -40,21 +40,15 @@ namespace { /// /// Checks for the init, dealloc, and any other functions that might be allowed /// to perform direct instance variable assignment based on their name. -struct MethodFilter { - virtual ~MethodFilter() {} - virtual bool operator()(ObjCMethodDecl *M) { - if (M->getMethodFamily() == OMF_init || - M->getMethodFamily() == OMF_dealloc || - M->getMethodFamily() == OMF_copy || - M->getMethodFamily() == OMF_mutableCopy || - M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || - M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos) - return true; - return false; - } -}; - -static MethodFilter DefaultMethodFilter; +static bool DefaultMethodFilter(const ObjCMethodDecl *M) { + if (M->getMethodFamily() == OMF_init || M->getMethodFamily() == OMF_dealloc || + M->getMethodFamily() == OMF_copy || + M->getMethodFamily() == OMF_mutableCopy || + M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || + M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos) + return true; + return false; +} class DirectIvarAssignment : public Checker<check::ASTDecl<ObjCImplementationDecl> > { @@ -89,7 +83,7 @@ class DirectIvarAssignment : }; public: - MethodFilter *ShouldSkipMethod; + bool (*ShouldSkipMethod)(const ObjCMethodDecl *); DirectIvarAssignment() : ShouldSkipMethod(&DefaultMethodFilter) {} @@ -230,22 +224,16 @@ void ento::registerDirectIvarAssignment(CheckerManager &mgr) { // Register the checker that checks for direct accesses in functions annotated // with __attribute__((annotate("objc_no_direct_instance_variable_assignment"))). -namespace { -struct InvalidatorMethodFilter : MethodFilter { - virtual ~InvalidatorMethodFilter() {} - virtual bool operator()(ObjCMethodDecl *M) { - for (specific_attr_iterator<AnnotateAttr> - AI = M->specific_attr_begin<AnnotateAttr>(), - AE = M->specific_attr_end<AnnotateAttr>(); AI != AE; ++AI) { - const AnnotateAttr *Ann = *AI; - if (Ann->getAnnotation() == "objc_no_direct_instance_variable_assignment") - return false; - } - return true; +static bool AttrFilter(const ObjCMethodDecl *M) { + for (specific_attr_iterator<AnnotateAttr> + AI = M->specific_attr_begin<AnnotateAttr>(), + AE = M->specific_attr_end<AnnotateAttr>(); + AI != AE; ++AI) { + const AnnotateAttr *Ann = *AI; + if (Ann->getAnnotation() == "objc_no_direct_instance_variable_assignment") + return false; } -}; - -InvalidatorMethodFilter AttrFilter; + return true; } void ento::registerDirectIvarAssignmentForAnnotatedFunctions( diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index 759aa6605ee7..7116e4dcd885 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -1,4 +1,4 @@ -//== DynamicTypePropagation.cpp ----------------------------------- -*- C++ -*--=// +//== DynamicTypePropagation.cpp -------------------------------- -*- C++ -*--=// // // The LLVM Compiler Infrastructure // @@ -62,7 +62,7 @@ void DynamicTypePropagation::checkPreCall(const CallEvent &Call, if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) { // C++11 [class.cdtor]p4: When a virtual function is called directly or // indirectly from a constructor or from a destructor, including during - // the construction or destruction of the class’s non-static data members, + // the construction or destruction of the class's non-static data members, // and the object to which the call applies is the object under // construction or destruction, the function called is the final overrider // in the constructor's or destructor's class and not one overriding it in diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index 810473f1a6e0..3ed2435b92ed 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -22,6 +22,8 @@ class ExprInspectionChecker : public Checker< eval::Call > { void analyzerEval(const CallExpr *CE, CheckerContext &C) const; void analyzerCheckInlined(const CallExpr *CE, CheckerContext &C) const; + void analyzerWarnIfReached(const CallExpr *CE, CheckerContext &C) const; + void analyzerCrash(const CallExpr *CE, CheckerContext &C) const; typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *, CheckerContext &C) const; @@ -39,6 +41,8 @@ bool ExprInspectionChecker::evalCall(const CallExpr *CE, .Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval) .Case("clang_analyzer_checkInlined", &ExprInspectionChecker::analyzerCheckInlined) + .Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash) + .Case("clang_analyzer_warnIfReached", &ExprInspectionChecker::analyzerWarnIfReached) .Default(0); if (!Handler) @@ -97,6 +101,17 @@ void ExprInspectionChecker::analyzerEval(const CallExpr *CE, C.emitReport(R); } +void ExprInspectionChecker::analyzerWarnIfReached(const CallExpr *CE, + CheckerContext &C) const { + ExplodedNode *N = C.getPredecessor(); + + if (!BT) + BT.reset(new BugType("Checking analyzer assumptions", "debug")); + + BugReport *R = new BugReport(*BT, "REACHABLE", N); + C.emitReport(R); +} + void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, CheckerContext &C) const { ExplodedNode *N = C.getPredecessor(); @@ -117,6 +132,11 @@ void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, C.emitReport(R); } +void ExprInspectionChecker::analyzerCrash(const CallExpr *CE, + CheckerContext &C) const { + LLVM_BUILTIN_TRAP; +} + void ento::registerExprInspectionChecker(CheckerManager &Mgr) { Mgr.registerChecker<ExprInspectionChecker>(); } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index c67c597feced..1dc60c6dbddc 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -619,7 +619,8 @@ static bool getPrintfFormatArgumentNum(const CallExpr *CE, const FormatAttr *Format = *i; ArgNum = Format->getFormatIdx() - 1; - if ((Format->getType() == "printf") && CE->getNumArgs() > ArgNum) + if ((Format->getType()->getName() == "printf") && + CE->getNumArgs() > ArgNum) return true; } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp index 271ba4702c57..4997f8d04ae8 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp @@ -25,8 +25,8 @@ // ^, ^= | 0 | | | x | x | | // <<, <<= | | | | x | 0 | | // >>, >>= | | | | x | 0 | | -// || | 1 | 1 | 1 | x | x | 1 | 1 -// && | 1 | x | x | 0 | 0 | x | x +// || | x | 1 | 1 | x | x | 1 | 1 +// && | x | x | x | 0 | 0 | x | x // = | x | | | | | | // == | 1 | | | | | | // >= | 1 | | | | | | @@ -678,19 +678,8 @@ bool IdempotentOperationChecker::CanVary(const Expr *Ex, return CanVary(B->getRHS(), AC) || CanVary(B->getLHS(), AC); } - case Stmt::UnaryOperatorClass: { - const UnaryOperator *U = cast<const UnaryOperator>(Ex); - // Handle trivial case first - switch (U->getOpcode()) { - case UO_Extension: - return false; - default: - return CanVary(U->getSubExpr(), AC); - } - } - case Stmt::ChooseExprClass: - return CanVary(cast<const ChooseExpr>(Ex)->getChosenSubExpr( - AC->getASTContext()), AC); + case Stmt::UnaryOperatorClass: + return CanVary(cast<UnaryOperator>(Ex)->getSubExpr(), AC); case Stmt::ConditionalOperatorClass: case Stmt::BinaryConditionalOperatorClass: return CanVary(cast<AbstractConditionalOperator>(Ex)->getCond(), AC); diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp new file mode 100644 index 000000000000..e696e38597bc --- /dev/null +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp @@ -0,0 +1,226 @@ +//== IdenticalExprChecker.cpp - Identical expression checker----------------==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// \brief This defines IdenticalExprChecker, a check that warns about +/// unintended use of identical expressions. +/// +/// It checks for use of identical expressions with comparison operators. +/// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/AST/RecursiveASTVisitor.h" + +using namespace clang; +using namespace ento; + +static bool isIdenticalExpr(const ASTContext &Ctx, const Expr *Expr1, + const Expr *Expr2); +//===----------------------------------------------------------------------===// +// FindIdenticalExprVisitor - Identify nodes using identical expressions. +//===----------------------------------------------------------------------===// + +namespace { +class FindIdenticalExprVisitor + : public RecursiveASTVisitor<FindIdenticalExprVisitor> { +public: + explicit FindIdenticalExprVisitor(BugReporter &B, AnalysisDeclContext *A) + : BR(B), AC(A) {} + // FindIdenticalExprVisitor only visits nodes + // that are binary operators. + bool VisitBinaryOperator(const BinaryOperator *B); + +private: + BugReporter &BR; + AnalysisDeclContext *AC; +}; +} // end anonymous namespace + +bool FindIdenticalExprVisitor::VisitBinaryOperator(const BinaryOperator *B) { + BinaryOperator::Opcode Op = B->getOpcode(); + if (!BinaryOperator::isComparisonOp(Op)) + return true; + // + // Special case for floating-point representation. + // + // If expressions on both sides of comparison operator are of type float, + // then for some comparison operators no warning shall be + // reported even if the expressions are identical from a symbolic point of + // view. Comparison between expressions, declared variables and literals + // are treated differently. + // + // != and == between float literals that have the same value should NOT warn. + // < > between float literals that have the same value SHOULD warn. + // + // != and == between the same float declaration should NOT warn. + // < > between the same float declaration SHOULD warn. + // + // != and == between eq. expressions that evaluates into float + // should NOT warn. + // < > between eq. expressions that evaluates into float + // should NOT warn. + // + const Expr *LHS = B->getLHS()->IgnoreParenImpCasts(); + const Expr *RHS = B->getRHS()->IgnoreParenImpCasts(); + + const DeclRefExpr *DeclRef1 = dyn_cast<DeclRefExpr>(LHS); + const DeclRefExpr *DeclRef2 = dyn_cast<DeclRefExpr>(RHS); + const FloatingLiteral *FloatLit1 = dyn_cast<FloatingLiteral>(LHS); + const FloatingLiteral *FloatLit2 = dyn_cast<FloatingLiteral>(RHS); + if ((DeclRef1) && (DeclRef2)) { + if ((DeclRef1->getType()->hasFloatingRepresentation()) && + (DeclRef2->getType()->hasFloatingRepresentation())) { + if (DeclRef1->getDecl() == DeclRef2->getDecl()) { + if ((Op == BO_EQ) || (Op == BO_NE)) { + return true; + } + } + } + } else if ((FloatLit1) && (FloatLit2)) { + if (FloatLit1->getValue().bitwiseIsEqual(FloatLit2->getValue())) { + if ((Op == BO_EQ) || (Op == BO_NE)) { + return true; + } + } + } else if (LHS->getType()->hasFloatingRepresentation()) { + // If any side of comparison operator still has floating-point + // representation, then it's an expression. Don't warn. + // Here only LHS is checked since RHS will be implicit casted to float. + return true; + } else { + // No special case with floating-point representation, report as usual. + } + + if (isIdenticalExpr(AC->getASTContext(), B->getLHS(), B->getRHS())) { + PathDiagnosticLocation ELoc = + PathDiagnosticLocation::createOperatorLoc(B, BR.getSourceManager()); + StringRef Message; + if (((Op == BO_EQ) || (Op == BO_LE) || (Op == BO_GE))) + Message = "comparison of identical expressions always evaluates to true"; + else + Message = "comparison of identical expressions always evaluates to false"; + BR.EmitBasicReport(AC->getDecl(), "Compare of identical expressions", + categories::LogicError, Message, ELoc); + } + // We want to visit ALL nodes (subexpressions of binary comparison + // expressions too) that contains comparison operators. + // True is always returned to traverse ALL nodes. + return true; +} +/// \brief Determines whether two expression trees are identical regarding +/// operators and symbols. +/// +/// Exceptions: expressions containing macros or functions with possible side +/// effects are never considered identical. +/// Limitations: (t + u) and (u + t) are not considered identical. +/// t*(u + t) and t*u + t*t are not considered identical. +/// +static bool isIdenticalExpr(const ASTContext &Ctx, const Expr *Expr1, + const Expr *Expr2) { + // If Expr1 & Expr2 are of different class then they are not + // identical expression. + if (Expr1->getStmtClass() != Expr2->getStmtClass()) + return false; + // If Expr1 has side effects then don't warn even if expressions + // are identical. + if (Expr1->HasSideEffects(Ctx)) + return false; + // Is expression is based on macro then don't warn even if + // the expressions are identical. + if ((Expr1->getExprLoc().isMacroID()) || (Expr2->getExprLoc().isMacroID())) + return false; + // If all children of two expressions are identical, return true. + Expr::const_child_iterator I1 = Expr1->child_begin(); + Expr::const_child_iterator I2 = Expr2->child_begin(); + while (I1 != Expr1->child_end() && I2 != Expr2->child_end()) { + const Expr *Child1 = dyn_cast<Expr>(*I1); + const Expr *Child2 = dyn_cast<Expr>(*I2); + if (!Child1 || !Child2 || !isIdenticalExpr(Ctx, Child1, Child2)) + return false; + ++I1; + ++I2; + } + // If there are different number of children in the expressions, return false. + // (TODO: check if this is a redundant condition.) + if (I1 != Expr1->child_end()) + return false; + if (I2 != Expr2->child_end()) + return false; + + switch (Expr1->getStmtClass()) { + default: + return false; + case Stmt::ArraySubscriptExprClass: + case Stmt::CStyleCastExprClass: + case Stmt::ImplicitCastExprClass: + case Stmt::ParenExprClass: + return true; + case Stmt::BinaryOperatorClass: { + const BinaryOperator *BinOp1 = dyn_cast<BinaryOperator>(Expr1); + const BinaryOperator *BinOp2 = dyn_cast<BinaryOperator>(Expr2); + return BinOp1->getOpcode() == BinOp2->getOpcode(); + } + case Stmt::CharacterLiteralClass: { + const CharacterLiteral *CharLit1 = dyn_cast<CharacterLiteral>(Expr1); + const CharacterLiteral *CharLit2 = dyn_cast<CharacterLiteral>(Expr2); + return CharLit1->getValue() == CharLit2->getValue(); + } + case Stmt::DeclRefExprClass: { + const DeclRefExpr *DeclRef1 = dyn_cast<DeclRefExpr>(Expr1); + const DeclRefExpr *DeclRef2 = dyn_cast<DeclRefExpr>(Expr2); + return DeclRef1->getDecl() == DeclRef2->getDecl(); + } + case Stmt::IntegerLiteralClass: { + const IntegerLiteral *IntLit1 = dyn_cast<IntegerLiteral>(Expr1); + const IntegerLiteral *IntLit2 = dyn_cast<IntegerLiteral>(Expr2); + return IntLit1->getValue() == IntLit2->getValue(); + } + case Stmt::FloatingLiteralClass: { + const FloatingLiteral *FloatLit1 = dyn_cast<FloatingLiteral>(Expr1); + const FloatingLiteral *FloatLit2 = dyn_cast<FloatingLiteral>(Expr2); + return FloatLit1->getValue().bitwiseIsEqual(FloatLit2->getValue()); + } + case Stmt::MemberExprClass: { + const MemberExpr *MemberExpr1 = dyn_cast<MemberExpr>(Expr1); + const MemberExpr *MemberExpr2 = dyn_cast<MemberExpr>(Expr2); + return MemberExpr1->getMemberDecl() == MemberExpr2->getMemberDecl(); + } + case Stmt::UnaryOperatorClass: { + const UnaryOperator *UnaryOp1 = dyn_cast<UnaryOperator>(Expr1); + const UnaryOperator *UnaryOp2 = dyn_cast<UnaryOperator>(Expr2); + if (UnaryOp1->getOpcode() != UnaryOp2->getOpcode()) + return false; + return !UnaryOp1->isIncrementDecrementOp(); + } + } +} + +//===----------------------------------------------------------------------===// +// FindIdenticalExprChecker +//===----------------------------------------------------------------------===// + +namespace { +class FindIdenticalExprChecker : public Checker<check::ASTCodeBody> { +public: + void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr, + BugReporter &BR) const { + FindIdenticalExprVisitor Visitor(BR, Mgr.getAnalysisDeclContext(D)); + Visitor.TraverseDecl(const_cast<Decl *>(D)); + } +}; +} // end anonymous namespace + +void ento::registerIdenticalExprChecker(CheckerManager &Mgr) { + Mgr.registerChecker<FindIdenticalExprChecker>(); +} diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 5d3eb65148dc..c7aa0fb150cb 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -100,7 +100,7 @@ public: } void dump(raw_ostream &OS) const { - static const char *Table[] = { + static const char *const Table[] = { "Allocated", "Released", "Relinquished" @@ -279,13 +279,19 @@ private: bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const; - /// Check if the function is known not to free memory, or if it is + /// Check if the function is known free memory, or if it is /// "interesting" and should be modeled explicitly. /// + /// \param [out] EscapingSymbol A function might not free memory in general, + /// but could be known to free a particular symbol. In this case, false is + /// returned and the single escaping symbol is returned through the out + /// parameter. + /// /// We assume that pointers do not escape through calls to system functions /// not handled by this checker. - bool doesNotFreeMemOrInteresting(const CallEvent *Call, - ProgramStateRef State) const; + bool mayFreeAnyEscapedMemoryOrIsModeledExplicitly(const CallEvent *Call, + ProgramStateRef State, + SymbolRef &EscapingSymbol) const; // Implementation of the checkPointerEscape callabcks. ProgramStateRef checkPointerEscapeAux(ProgramStateRef State, @@ -307,7 +313,7 @@ private: const Expr *DeallocExpr) const; void ReportMismatchedDealloc(CheckerContext &C, SourceRange Range, const Expr *DeallocExpr, const RefState *RS, - SymbolRef Sym) const; + SymbolRef Sym, bool OwnershipTransferred) const; void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range, const Expr *DeallocExpr, const Expr *AllocExpr = 0) const; @@ -1036,7 +1042,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, RsBase->getAllocationFamily() == getAllocationFamily(C, ParentExpr); if (!DeallocMatchesAlloc) { ReportMismatchedDealloc(C, ArgExpr->getSourceRange(), - ParentExpr, RsBase, SymBase); + ParentExpr, RsBase, SymBase, Hold); return 0; } @@ -1054,7 +1060,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, } } - ReleasedAllocated = (RsBase != 0); + ReleasedAllocated = (RsBase != 0) && RsBase->isAllocated(); // Clean out the info on previous call to free return info. State = State->remove<FreeReturnValue>(SymBase); @@ -1254,7 +1260,8 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C, SourceRange Range, const Expr *DeallocExpr, const RefState *RS, - SymbolRef Sym) const { + SymbolRef Sym, + bool OwnershipTransferred) const { if (!Filter.CMismatchedDeallocatorChecker) return; @@ -1273,15 +1280,27 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C, SmallString<20> DeallocBuf; llvm::raw_svector_ostream DeallocOs(DeallocBuf); - os << "Memory"; - if (printAllocDeallocName(AllocOs, C, AllocExpr)) - os << " allocated by " << AllocOs.str(); + if (OwnershipTransferred) { + if (printAllocDeallocName(DeallocOs, C, DeallocExpr)) + os << DeallocOs.str() << " cannot"; + else + os << "Cannot"; + + os << " take ownership of memory"; + + if (printAllocDeallocName(AllocOs, C, AllocExpr)) + os << " allocated by " << AllocOs.str(); + } else { + os << "Memory"; + if (printAllocDeallocName(AllocOs, C, AllocExpr)) + os << " allocated by " << AllocOs.str(); - os << " should be deallocated by "; - printExpectedDeallocName(os, RS->getAllocationFamily()); + os << " should be deallocated by "; + printExpectedDeallocName(os, RS->getAllocationFamily()); - if (printAllocDeallocName(DeallocOs, C, DeallocExpr)) - os << ", not " << DeallocOs.str(); + if (printAllocDeallocName(DeallocOs, C, DeallocExpr)) + os << ", not " << DeallocOs.str(); + } BugReport *R = new BugReport(*BT_MismatchedDealloc, os.str(), N); R->markInteresting(Sym); @@ -1664,8 +1683,8 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, if (!Errors.empty()) { static SimpleProgramPointTag Tag("MallocChecker : DeadSymbolsLeak"); N = C.addTransition(C.getState(), C.getPredecessor(), &Tag); - for (SmallVector<SymbolRef, 2>::iterator - I = Errors.begin(), E = Errors.end(); I != E; ++I) { + for (SmallVectorImpl<SymbolRef>::iterator + I = Errors.begin(), E = Errors.end(); I != E; ++I) { reportLeak(*I, N, C); } } @@ -1784,7 +1803,8 @@ bool MallocChecker::isReleased(SymbolRef Sym, CheckerContext &C) const { bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const { - if (isReleased(Sym, C)) { + // FIXME: Handle destructor called from delete more precisely. + if (isReleased(Sym, C) && S) { ReportUseAfterFree(C, S->getSourceRange(), Sym); return true; } @@ -1842,35 +1862,38 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, return state; } -bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, - ProgramStateRef State) const { +bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( + const CallEvent *Call, + ProgramStateRef State, + SymbolRef &EscapingSymbol) const { assert(Call); - + EscapingSymbol = 0; + // For now, assume that any C++ call can free memory. // TODO: If we want to be more optimistic here, we'll need to make sure that // regions escape to C++ containers. They seem to do that even now, but for // mysterious reasons. if (!(isa<FunctionCall>(Call) || isa<ObjCMethodCall>(Call))) - return false; + return true; // Check Objective-C messages by selector name. if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) { // If it's not a framework call, or if it takes a callback, assume it // can free memory. if (!Call->isInSystemHeader() || Call->hasNonZeroCallbackArg()) - return false; + return true; // If it's a method we know about, handle it explicitly post-call. // This should happen before the "freeWhenDone" check below. if (isKnownDeallocObjCMethodName(*Msg)) - return true; + return false; // If there's a "freeWhenDone" parameter, but the method isn't one we know // about, we can't be sure that the object will use free() to deallocate the // memory, so we can't model it explicitly. The best we can do is use it to // decide whether the pointer escapes. if (Optional<bool> FreeWhenDone = getFreeWhenDoneArg(*Msg)) - return !*FreeWhenDone; + return *FreeWhenDone; // If the first selector piece ends with "NoCopy", and there is no // "freeWhenDone" parameter set to zero, we know ownership is being @@ -1878,7 +1901,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, // free() to deallocate the memory, so we can't model it explicitly. StringRef FirstSlot = Msg->getSelector().getNameForSlot(0); if (FirstSlot.endswith("NoCopy")) - return false; + return true; // If the first selector starts with addPointer, insertPointer, // or replacePointer, assume we are dealing with NSPointerArray or similar. @@ -1887,34 +1910,42 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, if (FirstSlot.startswith("addPointer") || FirstSlot.startswith("insertPointer") || FirstSlot.startswith("replacePointer")) { - return false; + return true; + } + + // We should escape receiver on call to 'init'. This is especially relevant + // to the receiver, as the corresponding symbol is usually not referenced + // after the call. + if (Msg->getMethodFamily() == OMF_init) { + EscapingSymbol = Msg->getReceiverSVal().getAsSymbol(); + return true; } // Otherwise, assume that the method does not free memory. // Most framework methods do not free memory. - return true; + return false; } // At this point the only thing left to handle is straight function calls. const FunctionDecl *FD = cast<FunctionCall>(Call)->getDecl(); if (!FD) - return false; + return true; ASTContext &ASTC = State->getStateManager().getContext(); // If it's one of the allocation functions we can reason about, we model // its behavior explicitly. if (isMemFunction(FD, ASTC)) - return true; + return false; // If it's not a system call, assume it frees memory. if (!Call->isInSystemHeader()) - return false; + return true; // White list the system functions whose arguments escape. const IdentifierInfo *II = FD->getIdentifier(); if (!II) - return false; + return true; StringRef FName = II->getName(); // White list the 'XXXNoCopy' CoreFoundation functions. @@ -1928,10 +1959,10 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, if (const DeclRefExpr *DE = dyn_cast<DeclRefExpr>(ArgE)) { StringRef DeallocatorName = DE->getFoundDecl()->getName(); if (DeallocatorName == "kCFAllocatorNull") - return true; + return false; } } - return false; + return true; } // Associating streams with malloced buffers. The pointer can escape if @@ -1940,7 +1971,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, // Currently, we do not inspect the 'closefn' function (PR12101). if (FName == "funopen") if (Call->getNumArgs() >= 4 && Call->getArgSVal(4).isConstant(0)) - return true; + return false; // Do not warn on pointers passed to 'setbuf' when used with std streams, // these leaks might be intentional when setting the buffer for stdio. @@ -1952,7 +1983,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, if (const DeclRefExpr *ArgDRE = dyn_cast<DeclRefExpr>(ArgE)) if (const VarDecl *D = dyn_cast<VarDecl>(ArgDRE->getDecl())) if (D->getCanonicalDecl()->getName().find("std") != StringRef::npos) - return false; + return true; } } @@ -1966,7 +1997,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, FName == "CVPixelBufferCreateWithBytes" || FName == "CVPixelBufferCreateWithPlanarBytes" || FName == "OSAtomicEnqueue") { - return false; + return true; } // Handle cases where we know a buffer's /address/ can escape. @@ -1974,11 +2005,11 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, // even though the address escapes, it's still our responsibility to free the // buffer. if (Call->argumentsMayEscape()) - return false; + return true; // Otherwise, assume that the function does not free memory. // Most system calls do not free the memory. - return true; + return false; } static bool retTrue(const RefState *RS) { @@ -2012,9 +2043,11 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, bool(*CheckRefState)(const RefState*)) const { // If we know that the call does not free memory, or we want to process the // call later, keep tracking the top level arguments. - if ((Kind == PSK_DirectEscapeOnCall || - Kind == PSK_IndirectEscapeOnCall) && - doesNotFreeMemOrInteresting(Call, State)) { + SymbolRef EscapingSymbol = 0; + if (Kind == PSK_DirectEscapeOnCall && + !mayFreeAnyEscapedMemoryOrIsModeledExplicitly(Call, State, + EscapingSymbol) && + !EscapingSymbol) { return State; } @@ -2023,6 +2056,9 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, I != E; ++I) { SymbolRef sym = *I; + if (EscapingSymbol && EscapingSymbol != sym) + continue; + if (const RefState *RS = State->get<RegionState>(sym)) { if (RS->isAllocated() && CheckRefState(RS)) { State = State->remove<RegionState>(sym); diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp index 34425e314062..0cdf911bb4b1 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp @@ -213,11 +213,11 @@ void MallocOverflowSecurityChecker::OutputPossibleOverflows( e = PossibleMallocOverflows.end(); i != e; ++i) { - SourceRange R = i->mulop->getSourceRange(); BR.EmitBasicReport(D, "malloc() size overflow", categories::UnixAPI, "the computation of the size of the memory allocation may overflow", PathDiagnosticLocation::createOperatorLoc(i->mulop, - BR.getSourceManager()), &R, 1); + BR.getSourceManager()), + i->mulop->getSourceRange()); } } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp index d29f34fb03e2..6c776eb9ebb5 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp @@ -239,7 +239,7 @@ public: BR.EmitBasicReport(D, "Allocator sizeof operand mismatch", categories::UnixAPI, OS.str(), - L, Ranges.data(), Ranges.size()); + L, Ranges); } } } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp index 0009e1b7cf49..0e1064ef53a6 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp @@ -26,31 +26,29 @@ using namespace ento; namespace { -class NoReturnFunctionChecker : public Checker< check::PostStmt<CallExpr>, +class NoReturnFunctionChecker : public Checker< check::PostCall, check::PostObjCMessage > { public: - void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; + void checkPostCall(const CallEvent &CE, CheckerContext &C) const; void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; }; } -void NoReturnFunctionChecker::checkPostStmt(const CallExpr *CE, +void NoReturnFunctionChecker::checkPostCall(const CallEvent &CE, CheckerContext &C) const { ProgramStateRef state = C.getState(); - const Expr *Callee = CE->getCallee(); + bool BuildSinks = false; - bool BuildSinks = getFunctionExtInfo(Callee->getType()).getNoReturn(); + if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CE.getDecl())) + BuildSinks = FD->getAttr<AnalyzerNoReturnAttr>() || FD->isNoReturn(); - if (!BuildSinks) { - SVal L = state->getSVal(Callee, C.getLocationContext()); - const FunctionDecl *FD = L.getAsFunctionDecl(); - if (!FD) - return; + const Expr *Callee = CE.getOriginExpr(); + if (!BuildSinks && Callee) + BuildSinks = getFunctionExtInfo(Callee->getType()).getNoReturn(); - if (FD->getAttr<AnalyzerNoReturnAttr>() || FD->isNoReturn()) - BuildSinks = true; - else if (const IdentifierInfo *II = FD->getIdentifier()) { + if (!BuildSinks && CE.isGlobalCFunction()) { + if (const IdentifierInfo *II = CE.getCalleeIdentifier()) { // HACK: Some functions are not marked noreturn, and don't return. // Here are a few hardwired ones. If this takes too long, we can // potentially cache these results. @@ -66,6 +64,9 @@ void NoReturnFunctionChecker::checkPostStmt(const CallExpr *CE, .Case("assfail", true) .Case("db_error", true) .Case("__assert", true) + // For the purpose of static analysis, we do not care that + // this MSVC function will return if the user decides to continue. + .Case("_wassert", true) .Case("__assert_rtn", true) .Case("__assert_fail", true) .Case("dtrace_assfail", true) diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp index 4a0309de044e..503b1b501a71 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp @@ -140,12 +140,11 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { << Name << "' must be a C array of pointer-sized values, not '" << Arg->getType().getAsString() << "'"; - SourceRange R = Arg->getSourceRange(); PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); BR.EmitBasicReport(AC->getDecl(), OsName.str(), categories::CoreFoundationObjectiveC, - Os.str(), CELoc, &R, 1); + Os.str(), CELoc, Arg->getSourceRange()); } // Recurse and check children. diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 0f456ea8d785..c474e78310fa 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -28,6 +28,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" +#include "clang/StaticAnalyzer/Checkers/ObjCRetainCount.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/ImmutableList.h" @@ -41,116 +42,36 @@ using namespace clang; using namespace ento; +using namespace objc_retain; using llvm::StrInStrNoCase; //===----------------------------------------------------------------------===// -// Primitives used for constructing summaries for function/method calls. +// Adapters for FoldingSet. //===----------------------------------------------------------------------===// -/// ArgEffect is used to summarize a function/method call's effect on a -/// particular argument. -enum ArgEffect { DoNothing, Autorelease, Dealloc, DecRef, DecRefMsg, - DecRefBridgedTransfered, - IncRefMsg, IncRef, MakeCollectable, MayEscape, - - // Stop tracking the argument - the effect of the call is - // unknown. - StopTracking, - - // In some cases, we obtain a better summary for this checker - // by looking at the call site than by inlining the function. - // Signifies that we should stop tracking the symbol even if - // the function is inlined. - StopTrackingHard, - - // The function decrements the reference count and the checker - // should stop tracking the argument. - DecRefAndStopTrackingHard, DecRefMsgAndStopTrackingHard - }; - namespace llvm { template <> struct FoldingSetTrait<ArgEffect> { -static inline void Profile(const ArgEffect X, FoldingSetNodeID& ID) { +static inline void Profile(const ArgEffect X, FoldingSetNodeID &ID) { ID.AddInteger((unsigned) X); } }; +template <> struct FoldingSetTrait<RetEffect> { + static inline void Profile(const RetEffect &X, FoldingSetNodeID &ID) { + ID.AddInteger((unsigned) X.getKind()); + ID.AddInteger((unsigned) X.getObjKind()); +} +}; } // end llvm namespace +//===----------------------------------------------------------------------===// +// Reference-counting logic (typestate + counts). +//===----------------------------------------------------------------------===// + /// ArgEffects summarizes the effects of a function/method call on all of /// its arguments. typedef llvm::ImmutableMap<unsigned,ArgEffect> ArgEffects; namespace { - -/// RetEffect is used to summarize a function/method call's behavior with -/// respect to its return value. -class RetEffect { -public: - enum Kind { NoRet, OwnedSymbol, OwnedAllocatedSymbol, - NotOwnedSymbol, GCNotOwnedSymbol, ARCNotOwnedSymbol, - OwnedWhenTrackedReceiver, - // Treat this function as returning a non-tracked symbol even if - // the function has been inlined. This is used where the call - // site summary is more presise than the summary indirectly produced - // by inlining the function - NoRetHard - }; - - enum ObjKind { CF, ObjC, AnyObj }; - -private: - Kind K; - ObjKind O; - - RetEffect(Kind k, ObjKind o = AnyObj) : K(k), O(o) {} - -public: - Kind getKind() const { return K; } - - ObjKind getObjKind() const { return O; } - - bool isOwned() const { - return K == OwnedSymbol || K == OwnedAllocatedSymbol || - K == OwnedWhenTrackedReceiver; - } - - bool operator==(const RetEffect &Other) const { - return K == Other.K && O == Other.O; - } - - static RetEffect MakeOwnedWhenTrackedReceiver() { - return RetEffect(OwnedWhenTrackedReceiver, ObjC); - } - - static RetEffect MakeOwned(ObjKind o, bool isAllocated = false) { - return RetEffect(isAllocated ? OwnedAllocatedSymbol : OwnedSymbol, o); - } - static RetEffect MakeNotOwned(ObjKind o) { - return RetEffect(NotOwnedSymbol, o); - } - static RetEffect MakeGCNotOwned() { - return RetEffect(GCNotOwnedSymbol, ObjC); - } - static RetEffect MakeARCNotOwned() { - return RetEffect(ARCNotOwnedSymbol, ObjC); - } - static RetEffect MakeNoRet() { - return RetEffect(NoRet); - } - static RetEffect MakeNoRetHard() { - return RetEffect(NoRetHard); - } - - void Profile(llvm::FoldingSetNodeID& ID) const { - ID.AddInteger((unsigned) K); - ID.AddInteger((unsigned) O); - } -}; - -//===----------------------------------------------------------------------===// -// Reference-counting logic (typestate + counts). -//===----------------------------------------------------------------------===// - class RefVal { public: enum Kind { @@ -396,7 +317,7 @@ public: return DefaultArgEffect; } - + void addArg(ArgEffects::Factory &af, unsigned idx, ArgEffect e) { Args = af.add(Args, idx, e); } @@ -496,8 +417,6 @@ template <> struct DenseMapInfo<ObjCSummaryKey> { } }; -template <> -struct isPodLike<ObjCSummaryKey> { static const bool value = true; }; } // end llvm namespace namespace { @@ -631,7 +550,7 @@ class RetainSummaryManager { /// data in ScratchArgs. ArgEffects getArgEffects(); - enum UnaryFuncKind { cfretain, cfrelease, cfmakecollectable }; + enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable }; const RetainSummary *getUnarySummary(const FunctionType* FT, UnaryFuncKind func); @@ -885,6 +804,10 @@ static bool isRelease(const FunctionDecl *FD, StringRef FName) { return FName.endswith("Release"); } +static bool isAutorelease(const FunctionDecl *FD, StringRef FName) { + return FName.endswith("Autorelease"); +} + static bool isMakeCollectable(const FunctionDecl *FD, StringRef FName) { // FIXME: Remove FunctionDecl parameter. // FIXME: Is it really okay if MakeCollectable isn't a suffix? @@ -895,7 +818,7 @@ static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) { switch (E) { case DoNothing: case Autorelease: - case DecRefBridgedTransfered: + case DecRefBridgedTransferred: case IncRef: case IncRefMsg: case MakeCollectable: @@ -1144,12 +1067,19 @@ RetainSummaryManager::getFunctionSummary(const FunctionDecl *FD) { if (RetTy->isPointerType()) { // For CoreFoundation ('CF') types. if (cocoa::isRefType(RetTy, "CF", FName)) { - if (isRetain(FD, FName)) + if (isRetain(FD, FName)) { S = getUnarySummary(FT, cfretain); - else if (isMakeCollectable(FD, FName)) + } else if (isAutorelease(FD, FName)) { + S = getUnarySummary(FT, cfautorelease); + // The headers use cf_consumed, but we can fully model CFAutorelease + // ourselves. + AllowAnnotations = false; + } else if (isMakeCollectable(FD, FName)) { S = getUnarySummary(FT, cfmakecollectable); - else + AllowAnnotations = false; + } else { S = getCFCreateGetRuleSummary(FD); + } break; } @@ -1252,9 +1182,10 @@ RetainSummaryManager::getUnarySummary(const FunctionType* FT, ArgEffect Effect; switch (func) { - case cfretain: Effect = IncRef; break; - case cfrelease: Effect = DecRef; break; - case cfmakecollectable: Effect = MakeCollectable; break; + case cfretain: Effect = IncRef; break; + case cfrelease: Effect = DecRef; break; + case cfautorelease: Effect = Autorelease; break; + case cfmakecollectable: Effect = MakeCollectable; break; } ScratchArgs = AF.add(ScratchArgs, 0, Effect); @@ -1823,16 +1754,6 @@ void CFRefReport::addGCModeDescription(const LangOptions &LOpts, addExtraText(GCModeDescription); } -// FIXME: This should be a method on SmallVector. -static inline bool contains(const SmallVectorImpl<ArgEffect>& V, - ArgEffect X) { - for (SmallVectorImpl<ArgEffect>::const_iterator I=V.begin(), E=V.end(); - I!=E; ++I) - if (*I == X) return true; - - return false; -} - static bool isNumericLiteralExpression(const Expr *E) { // FIXME: This set of cases was copied from SemaExprObjC. return isa<IntegerLiteral>(E) || @@ -1994,7 +1915,8 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, RefVal PrevV = *PrevT; // Specially handle -dealloc. - if (!GCEnabled && contains(AEffects, Dealloc)) { + if (!GCEnabled && std::find(AEffects.begin(), AEffects.end(), Dealloc) != + AEffects.end()) { // Determine if the object's reference count was pushed to zero. assert(!(PrevV == CurrV) && "The typestate *must* have changed."); // We may not have transitioned to 'release' if we hit an error. @@ -2007,7 +1929,8 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, } // Specially handle CFMakeCollectable and friends. - if (contains(AEffects, MakeCollectable)) { + if (std::find(AEffects.begin(), AEffects.end(), MakeCollectable) != + AEffects.end()) { // Get the name of the function. const Stmt *S = N->getLocation().castAs<StmtPoint>().getStmt(); SVal X = @@ -2686,7 +2609,7 @@ void RetainCountChecker::checkPostStmt(const CastExpr *CE, AE = IncRef; break; case clang::OBC_BridgeTransfer: - AE = DecRefBridgedTransfered; + AE = DecRefBridgedTransferred; break; } @@ -3074,7 +2997,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, break; case DecRef: - case DecRefBridgedTransfered: + case DecRefBridgedTransferred: case DecRefAndStopTrackingHard: switch (V.getKind()) { default: @@ -3084,8 +3007,8 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case RefVal::Owned: assert(V.getCount() > 0); if (V.getCount() == 1) - V = V ^ (E == DecRefBridgedTransfered ? - RefVal::NotOwned : RefVal::Released); + V = V ^ (E == DecRefBridgedTransferred ? RefVal::NotOwned + : RefVal::Released); else if (E == DecRefAndStopTrackingHard) return removeRefBinding(state, sym); @@ -3193,11 +3116,13 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { canEval = II->isStr("NSMakeCollectable"); } else if (ResultTy->isPointerType()) { // Handle: (CF|CG)Retain + // CFAutorelease // CFMakeCollectable // It's okay to be a little sloppy here (CGMakeCollectable doesn't exist). if (cocoa::isRefType(ResultTy, "CF", FName) || cocoa::isRefType(ResultTy, "CG", FName)) { - canEval = isRetain(FD, FName) || isMakeCollectable(FD, FName); + canEval = isRetain(FD, FName) || isAutorelease(FD, FName) || + isMakeCollectable(FD, FName); } } @@ -3445,6 +3370,16 @@ void RetainCountChecker::checkBind(SVal loc, SVal val, const Stmt *S, } } + // If we are storing the value into an auto function scope variable annotated + // with (__attribute__((cleanup))), stop tracking the value to avoid leak + // false positives. + if (const VarRegion *LVR = dyn_cast_or_null<VarRegion>(loc.getAsRegion())) { + const VarDecl *VD = LVR->getDecl(); + if (VD->getAttr<CleanupAttr>()) { + escapes = true; + } + } + // If our store can represent the binding and we aren't storing to something // that doesn't have local storage then just return and have the simulation // state continue as is. @@ -3635,6 +3570,13 @@ void RetainCountChecker::checkEndFunction(CheckerContext &Ctx) const { RefBindingsTy B = state->get<RefBindings>(); ExplodedNode *Pred = Ctx.getPredecessor(); + // Don't process anything within synthesized bodies. + const LocationContext *LCtx = Pred->getLocationContext(); + if (LCtx->getAnalysisDeclContext()->isBodyAutosynthesized()) { + assert(LCtx->getParent()); + return; + } + for (RefBindingsTy::iterator I = B.begin(), E = B.end(); I != E; ++I) { state = handleAutoreleaseCounts(state, Pred, /*Tag=*/0, Ctx, I->first, I->second); @@ -3646,7 +3588,7 @@ void RetainCountChecker::checkEndFunction(CheckerContext &Ctx) const { // We will do that later. // FIXME: we should instead check for imbalances of the retain/releases, // and suggest annotations. - if (Ctx.getLocationContext()->getParent()) + if (LCtx->getParent()) return; B = state->get<RefBindings>(); @@ -3747,3 +3689,37 @@ void ento::registerRetainCountChecker(CheckerManager &Mgr) { Mgr.registerChecker<RetainCountChecker>(Mgr.getAnalyzerOptions()); } +//===----------------------------------------------------------------------===// +// Implementation of the CallEffects API. +//===----------------------------------------------------------------------===// + +namespace clang { namespace ento { namespace objc_retain { + +// This is a bit gross, but it allows us to populate CallEffects without +// creating a bunch of accessors. This kind is very localized, so the +// damage of this macro is limited. +#define createCallEffect(D, KIND)\ + ASTContext &Ctx = D->getASTContext();\ + LangOptions L = Ctx.getLangOpts();\ + RetainSummaryManager M(Ctx, L.GCOnly, L.ObjCAutoRefCount);\ + const RetainSummary *S = M.get ## KIND ## Summary(D);\ + CallEffects CE(S->getRetEffect());\ + CE.Receiver = S->getReceiverEffect();\ + unsigned N = D->param_size();\ + for (unsigned i = 0; i < N; ++i) {\ + CE.Args.push_back(S->getArg(i));\ + } + +CallEffects CallEffects::getEffect(const ObjCMethodDecl *MD) { + createCallEffect(MD, Method); + return CE; +} + +CallEffects CallEffects::getEffect(const FunctionDecl *FD) { + createCallEffect(FD, Function); + return CE; +} + +#undef createCallEffect + +}}} diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp index 1ccf339bacc1..9ca0ab5d7fb7 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -227,8 +227,8 @@ void SimpleStreamChecker::reportLeaks(SymbolVector LeakedStreams, ExplodedNode *ErrNode) const { // Attach bug reports to the leak node. // TODO: Identify the leaked file descriptor. - for (SmallVector<SymbolRef, 2>::iterator - I = LeakedStreams.begin(), E = LeakedStreams.end(); I != E; ++I) { + for (SmallVectorImpl<SymbolRef>::iterator + I = LeakedStreams.begin(), E = LeakedStreams.end(); I != E; ++I) { BugReport *R = new BugReport(*LeakBugType, "Opened file is never closed; potential resource leak", ErrNode); R->markInteresting(*I); @@ -259,9 +259,7 @@ SimpleStreamChecker::checkPointerEscape(ProgramStateRef State, const CallEvent *Call, PointerEscapeKind Kind) const { // If we know that the call cannot close a file, there is nothing to do. - if ((Kind == PSK_DirectEscapeOnCall || - Kind == PSK_IndirectEscapeOnCall) && - guaranteedNotToCloseFile(*Call)) { + if (Kind == PSK_DirectEscapeOnCall && guaranteedNotToCloseFile(*Call)) { return State; } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index 673356319833..3f6549de56b0 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -40,6 +40,15 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, ProgramStateRef state = C.getState(); const LocationContext *LCtx = C.getLocationContext(); if (state->getSVal(B, LCtx).isUndef()) { + + // Do not report assignments of uninitialized values inside swap functions. + // This should allow to swap partially uninitialized structs + // (radar://14129997) + if (const FunctionDecl *EnclosingFunctionDecl = + dyn_cast<FunctionDecl>(C.getStackFrame()->getDecl())) + if (C.getCalleeName(EnclosingFunctionDecl) == "swap") + return; + // Generate an error node. ExplodedNode *N = C.generateSink(); if (!N) diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp index 176ee480826c..5df8846766e1 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "clang/AST/DeclCXX.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" @@ -42,7 +43,7 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A, // Don't warn if we're in an implicitly-generated constructor. const Decl *D = C.getLocationContext()->getDecl(); if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) - if (Ctor->isImplicitlyDefined()) + if (Ctor->isDefaulted()) return; ExplodedNode *N = C.generateSink(); diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index e04f49c3746d..016e3c804592 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -38,6 +38,14 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, if (!val.isUndef()) return; + // Do not report assignments of uninitialized values inside swap functions. + // This should allow to swap partially uninitialized structs + // (radar://14129997) + if (const FunctionDecl *EnclosingFunctionDecl = + dyn_cast<FunctionDecl>(C.getStackFrame()->getDecl())) + if (C.getCalleeName(EnclosingFunctionDecl) == "swap") + return; + ExplodedNode *N = C.generateSink(); if (!N) diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp index 91c2ffb5aabf..a40b5a3e8378 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp @@ -67,9 +67,12 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph &G, I != E; ++I) { const ProgramPoint &P = I->getLocation(); LC = P.getLocationContext(); + if (!LC->inTopFrame()) + continue; if (!D) D = LC->getAnalysisDeclContext()->getDecl(); + // Save the CFG if we don't have it already if (!C) C = LC->getAnalysisDeclContext()->getUnoptimizedCFG(); diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp index 06f01ad75422..7b6adbfad87c 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -191,7 +191,7 @@ void WalkAST::ReportVirtualCall(const CallExpr *CE, bool isPure) { "Call pure virtual function during construction or " "Destruction", "Cplusplus", - os.str(), CELoc, &R, 1); + os.str(), CELoc, R); return; } else { @@ -201,7 +201,7 @@ void WalkAST::ReportVirtualCall(const CallExpr *CE, bool isPure) { "Call virtual function during construction or " "Destruction", "Cplusplus", - os.str(), CELoc, &R, 1); + os.str(), CELoc, R); return; } } |