BugType *BT_call_arg;
BugType *BT_msg_undef;
BugType *BT_msg_arg;
- BugType *BT_struct_ret;
- BugType *BT_void_ptr;
+ BugType *BT_msg_ret;
public:
CallAndMessageChecker() :
BT_call_null(0), BT_call_undef(0), BT_call_arg(0),
- BT_msg_undef(0), BT_msg_arg(0), BT_struct_ret(0), BT_void_ptr(0) {}
+ BT_msg_undef(0), BT_msg_arg(0), BT_msg_ret(0) {}
static void *getTag() {
static int x = 0;
return &x;
}
+
void PreVisitCallExpr(CheckerContext &C, const CallExpr *CE);
void PreVisitObjCMessageExpr(CheckerContext &C, const ObjCMessageExpr *ME);
+
private:
void EmitBadCall(BugType *BT, CheckerContext &C, const CallExpr *CE);
+ void EmitNilReceiverBug(CheckerContext &C, const ObjCMessageExpr *ME,
+ ExplodedNode *N);
+
+ void HandleNilReceiver(CheckerContext &C, const GRState *state,
+ const ObjCMessageExpr *ME);
};
} // end anonymous namespace
}
}
- // Check if the receiver was nil and then return value a struct.
+ // Check if the receiver was nil and then returns a value that may
+ // be garbage.
if (const Expr *Receiver = ME->getReceiver()) {
- SVal L_untested = state->getSVal(Receiver);
- // Assume that the receiver is not NULL.
- DefinedOrUnknownSVal L = cast<DefinedOrUnknownSVal>(L_untested);
- const GRState *StNotNull = state->Assume(L, true);
-
- // Assume that the receiver is NULL.
- const GRState *StNull = state->Assume(L, false);
-
- if (StNull) {
- QualType RetTy = ME->getType();
- if (RetTy->isRecordType()) {
- if (C.getPredecessor()->getParentMap().isConsumedExpr(ME)) {
- // The [0 ...] expressions will return garbage. Flag either an
- // explicit or implicit error. Because of the structure of this
- // function we currently do not bifurfacte the state graph at
- // this point.
- // FIXME: We should bifurcate and fill the returned struct with
- // garbage.
- if (ExplodedNode* N = C.GenerateSink(StNull)) {
- if (!StNotNull) {
- if (!BT_struct_ret) {
- std::string sbuf;
- llvm::raw_string_ostream os(sbuf);
- os << "The receiver in the message expression is 'nil' and "
- "results in the returned value (of type '"
- << ME->getType().getAsString()
- << "') to be garbage or otherwise undefined";
- BT_struct_ret = new BuiltinBug(os.str().c_str());
- }
-
- EnhancedBugReport *R = new EnhancedBugReport(*BT_struct_ret,
- BT_struct_ret->getName(), N);
- R->addRange(Receiver->getSourceRange());
- R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue,
- Receiver);
- C.EmitReport(R);
- return;
- }
- else
- // Do not report implicit bug.
- return;
- }
- }
- } else {
- ASTContext &Ctx = C.getASTContext();
- if (RetTy != Ctx.VoidTy) {
- if (C.getPredecessor()->getParentMap().isConsumedExpr(ME)) {
- // sizeof(void *)
- const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
- // sizeof(return type)
- const uint64_t returnTypeSize = Ctx.getTypeSize(ME->getType());
-
- if (voidPtrSize < returnTypeSize) {
- if (ExplodedNode* N = C.GenerateSink(StNull)) {
- if (!StNotNull) {
- if (!BT_struct_ret) {
- std::string sbuf;
- llvm::raw_string_ostream os(sbuf);
- os << "The receiver in the message expression is 'nil' and "
- "results in the returned value (of type '"
- << ME->getType().getAsString()
- << "' and of size "
- << returnTypeSize / 8
- << " bytes) to be garbage or otherwise undefined";
- BT_void_ptr = new BuiltinBug(os.str().c_str());
- }
-
- EnhancedBugReport *R = new EnhancedBugReport(*BT_void_ptr,
- BT_void_ptr->getName(), N);
- R->addRange(Receiver->getSourceRange());
- R->addVisitorCreator(
- bugreporter::registerTrackNullOrUndefValue, Receiver);
- C.EmitReport(R);
- return;
- } else
- // Do not report implicit bug.
- return;
- }
- }
- else if (!StNotNull) {
- // Handle the safe cases where the return value is 0 if the
- // receiver is nil.
- //
- // FIXME: For now take the conservative approach that we only
- // return null values if we *know* that the receiver is nil.
- // This is because we can have surprises like:
- //
- // ... = [[NSScreens screens] objectAtIndex:0];
- //
- // What can happen is that [... screens] could return nil, but
- // it most likely isn't nil. We should assume the semantics
- // of this case unless we have *a lot* more knowledge.
- //
- SVal V = C.getValueManager().makeZeroVal(ME->getType());
- C.GenerateNode(StNull->BindExpr(ME, V));
- return;
- }
- }
- }
- }
+ DefinedOrUnknownSVal receiverVal =
+ cast<DefinedOrUnknownSVal>(state->getSVal(Receiver));
+
+ const GRState *notNullState, *nullState;
+ llvm::tie(notNullState, nullState) = state->Assume(receiverVal);
+
+ if (nullState && !notNullState) {
+ HandleNilReceiver(C, nullState, ME);
+ C.setDoneEvaluating(); // FIXME: eventually remove.
+ return;
}
- // Do not propagate null state.
- if (StNotNull)
- C.GenerateNode(StNotNull);
+
+ assert(notNullState);
+ state = notNullState;
}
+
+ // Add a state transition if the state has changed.
+ C.addTransition(state);
+}
+
+void CallAndMessageChecker::EmitNilReceiverBug(CheckerContext &C,
+ const ObjCMessageExpr *ME,
+ ExplodedNode *N) {
+
+ if (!BT_msg_ret)
+ BT_msg_ret =
+ new BuiltinBug("Receiver in message expression is "
+ "'nil' and returns a garbage value");
+
+ llvm::SmallString<200> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << "The receiver of message '" << ME->getSelector().getAsString()
+ << "' is nil and returns a value of type '"
+ << ME->getType().getAsString() << "' that will be garbage";
+
+ EnhancedBugReport *report = new EnhancedBugReport(*BT_msg_ret, os.str(), N);
+ const Expr *receiver = ME->getReceiver();
+ report->addRange(receiver->getSourceRange());
+ report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue,
+ receiver);
+ C.EmitReport(report);
+}
+
+void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C,
+ const GRState *state,
+ const ObjCMessageExpr *ME) {
+
+ // Check the return type of the message expression. A message to nil will
+ // return different values depending on the return type and the architecture.
+ QualType RetTy = ME->getType();
+
+ if (RetTy->isStructureType()) {
+ // FIXME: At some point we shouldn't rely on isConsumedExpr(), but instead
+ // have the "use of undefined value" be smarter about where the
+ // undefined value came from.
+ if (C.getPredecessor()->getParentMap().isConsumedExpr(ME)) {
+ if (ExplodedNode* N = C.GenerateSink(state))
+ EmitNilReceiverBug(C, ME, N);
+ return;
+ }
+
+ // The result is not consumed by a surrounding expression. Just propagate
+ // the current state.
+ C.addTransition(state);
+ return;
+ }
+
+ // Other cases: check if the return type is smaller than void*.
+ ASTContext &Ctx = C.getASTContext();
+ if (RetTy != Ctx.VoidTy &&
+ C.getPredecessor()->getParentMap().isConsumedExpr(ME)) {
+ // Compute: sizeof(void *) and sizeof(return type)
+ const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
+ const uint64_t returnTypeSize = Ctx.getTypeSize(ME->getType());
+
+ if (voidPtrSize < returnTypeSize) {
+ if (ExplodedNode* N = C.GenerateSink(state))
+ EmitNilReceiverBug(C, ME, N);
+ return;
+ }
+
+ // Handle the safe cases where the return value is 0 if the
+ // receiver is nil.
+ //
+ // FIXME: For now take the conservative approach that we only
+ // return null values if we *know* that the receiver is nil.
+ // This is because we can have surprises like:
+ //
+ // ... = [[NSScreens screens] objectAtIndex:0];
+ //
+ // What can happen is that [... screens] could return nil, but
+ // it most likely isn't nil. We should assume the semantics
+ // of this case unless we have *a lot* more knowledge.
+ //
+ SVal V = C.getValueManager().makeZeroVal(ME->getType());
+ C.GenerateNode(state->BindExpr(ME, V));
+ return;
+ }
+
+ C.addTransition(state);
}
// Checker worklist routines.
//===----------------------------------------------------------------------===//
-void GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst,
+bool GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst,
ExplodedNodeSet &Src, bool isPrevisit) {
if (Checkers.empty()) {
- Dst = Src;
- return;
+ Dst.insert(Src);
+ return false;
}
ExplodedNodeSet Tmp;
Checker *checker = I->second;
for (ExplodedNodeSet::iterator NI = PrevSet->begin(), NE = PrevSet->end();
- NI != NE; ++NI)
- checker->GR_Visit(*CurrSet, *Builder, *this, S, *NI, tag, isPrevisit);
+ NI != NE; ++NI) {
+ // FIXME: Halting evaluation of the checkers is something we may
+ // not support later. The design is still evolving.
+ if (checker->GR_Visit(*CurrSet, *Builder, *this, S, *NI,
+ tag, isPrevisit)) {
+ if (CurrSet != &Dst)
+ Dst.insert(*CurrSet);
+ return true;
+ }
+ }
// Update which NodeSet is the current one.
PrevSet = CurrSet;
// Don't autotransition. The CheckerContext objects should do this
// automatically.
+ return false;
}
// FIXME: This is largely copy-paste from CheckerVisit(). Need to
// Handle previsits checks.
ExplodedNodeSet Src, DstTmp;
- Src.Add(Pred);
- CheckerVisit(ME, DstTmp, Src, true);
+ Src.Add(Pred);
+
+ if (CheckerVisit(ME, DstTmp, Src, true)) {
+ Dst.insert(DstTmp);
+ return;
+ }
unsigned size = Dst.size();
projects / clang / commitdiff