aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/clang/Analysis/PathSensitive/Checker.h18
-rw-r--r--include/clang/Analysis/PathSensitive/ExplodedGraph.h14
-rw-r--r--include/clang/Analysis/PathSensitive/GRExprEngine.h2
-rw-r--r--lib/Analysis/CallAndMessageChecker.cpp216
-rw-r--r--lib/Analysis/GRExprEngine.cpp27
-rw-r--r--test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m8
-rw-r--r--test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m4
7 files changed, 162 insertions, 127 deletions
diff --git a/include/clang/Analysis/PathSensitive/Checker.h b/include/clang/Analysis/PathSensitive/Checker.h
index f6a5074fd8..91a4b6d1b1 100644
--- a/include/clang/Analysis/PathSensitive/Checker.h
+++ b/include/clang/Analysis/PathSensitive/Checker.h
@@ -42,6 +42,7 @@ class CheckerContext {
const GRState *state;
const Stmt *statement;
const unsigned size;
+ bool DoneEvaluating; // FIXME: This is not a permanent API change.
public:
CheckerContext(ExplodedNodeSet &dst, GRStmtNodeBuilder &builder,
GRExprEngine &eng, ExplodedNode *pred,
@@ -52,10 +53,22 @@ public:
OldTag(B.Tag, tag),
OldPointKind(B.PointKind, K),
OldHasGen(B.HasGeneratedNode),
- state(st), statement(stmt), size(Dst.size()) {}
+ state(st), statement(stmt), size(Dst.size()),
+ DoneEvaluating(false) {}
~CheckerContext();
+ // FIXME: This were added to support CallAndMessageChecker to indicating
+ // to GRExprEngine to "stop evaluating" a message expression under certain
+ // cases. This is *not* meant to be a permanent API change, and was added
+ // to aid in the transition of removing logic for checks from GRExprEngine.
+ void setDoneEvaluating() {
+ DoneEvaluating = true;
+ }
+ bool isDoneEvaluating() const {
+ return DoneEvaluating;
+ }
+
ConstraintManager &getConstraintManager() {
return Eng.getConstraintManager();
}
@@ -152,7 +165,7 @@ private:
friend class GRExprEngine;
// FIXME: Remove the 'tag' option.
- void GR_Visit(ExplodedNodeSet &Dst,
+ bool GR_Visit(ExplodedNodeSet &Dst,
GRStmtNodeBuilder &Builder,
GRExprEngine &Eng,
const Stmt *S,
@@ -164,6 +177,7 @@ private:
_PreVisit(C, S);
else
_PostVisit(C, S);
+ return C.isDoneEvaluating();
}
// FIXME: Remove the 'tag' option.
diff --git a/include/clang/Analysis/PathSensitive/ExplodedGraph.h b/include/clang/Analysis/PathSensitive/ExplodedGraph.h
index a7bbdf939f..76cab1ddc1 100644
--- a/include/clang/Analysis/PathSensitive/ExplodedGraph.h
+++ b/include/clang/Analysis/PathSensitive/ExplodedGraph.h
@@ -352,10 +352,16 @@ public:
typedef ImplTy::iterator iterator;
typedef ImplTy::const_iterator const_iterator;
- inline unsigned size() const { return Impl.size(); }
- inline bool empty() const { return Impl.empty(); }
-
- inline void clear() { Impl.clear(); }
+ unsigned size() const { return Impl.size(); }
+ bool empty() const { return Impl.empty(); }
+
+ void clear() { Impl.clear(); }
+ void insert(const ExplodedNodeSet &S) {
+ if (empty())
+ Impl = S.Impl;
+ else
+ Impl.insert(S.begin(), S.end());
+ }
inline iterator begin() { return Impl.begin(); }
inline iterator end() { return Impl.end(); }
diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index 1512b9099f..99ff57443b 100644
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -222,7 +222,7 @@ public:
protected:
/// CheckerVisit - Dispatcher for performing checker-specific logic
/// at specific statements.
- void CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src,
+ bool CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src,
bool isPrevisit);
void CheckerVisitBind(const Stmt *AssignE, const Stmt *StoreE,
diff --git a/lib/Analysis/CallAndMessageChecker.cpp b/lib/Analysis/CallAndMessageChecker.cpp
index ebc3a4fd56..920f21a22e 100644
--- a/lib/Analysis/CallAndMessageChecker.cpp
+++ b/lib/Analysis/CallAndMessageChecker.cpp
@@ -27,21 +27,27 @@ class VISIBILITY_HIDDEN CallAndMessageChecker
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
@@ -142,111 +148,107 @@ void CallAndMessageChecker::PreVisitObjCMessageExpr(CheckerContext &C,
}
}
- // 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);
}
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index a301f2549d..255a6639ac 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -108,12 +108,12 @@ public:
// 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;
@@ -129,8 +129,16 @@ void GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst,
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;
@@ -138,6 +146,7 @@ void GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst,
// Don't autotransition. The CheckerContext objects should do this
// automatically.
+ return false;
}
// FIXME: This is largely copy-paste from CheckerVisit(). Need to
@@ -1854,8 +1863,12 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME,
// 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();
diff --git a/test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m b/test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m
index 536c4a1b67..46164f8da4 100644
--- a/test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m
+++ b/test/Analysis/nil-receiver-undefined-larger-than-voidptr-ret.m
@@ -28,20 +28,20 @@ void createFoo() {
void createFoo2() {
MyClass *obj = 0;
- long double ld = [obj longDoubleM]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value}}
+ long double ld = [obj longDoubleM]; // expected-warning{{The receiver of message 'longDoubleM' is nil and returns a value of type 'long double' that will be garbage}}
}
void createFoo3() {
MyClass *obj;
obj = 0;
- long long ll = [obj longlongM]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value}}
+ long long ll = [obj longlongM]; // expected-warning{{The receiver of message 'longlongM' is nil and returns a value of type 'long long' that will be garbage}}
}
void createFoo4() {
MyClass *obj = 0;
- double d = [obj doubleM]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value}}
+ double d = [obj doubleM]; // expected-warning{{The receiver of message 'doubleM' is nil and returns a value of type 'double' that will be garbage}}
}
void createFoo5() {
@@ -59,7 +59,7 @@ void handleNilPruneLoop(MyClass *obj) {
long long j = [obj longlongM]; // no-warning
}
- long long j = [obj longlongM]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value}}
+ long long j = [obj longlongM]; // expected-warning{{The receiver of message 'longlongM' is nil and returns a value of type 'long long' that will be garbage}}
}
int handleVoidInComma() {
diff --git a/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m b/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m
index 7230f21c08..9d6fe5b27d 100644
--- a/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m
+++ b/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m
@@ -15,12 +15,12 @@ typedef struct Foo { int x; } Bar;
void createFoo() {
MyClass *obj = 0;
- Bar f = [obj foo]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value (of type 'Bar') to be garbage or otherwise undefined.}}
+ Bar f = [obj foo]; // expected-warning{{The receiver of message 'foo' is nil and returns a value of type 'Bar' that will be garbage}}
}
void createFoo2() {
MyClass *obj = 0;
[obj foo]; // no-warning
- Bar f = [obj foo]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value (of type 'Bar') to be garbage or otherwise undefined.}}
+ Bar f = [obj foo]; // expected-warning{{The receiver of message 'foo' is nil and returns a value of type 'Bar' that will be garbage}}
}
generated by cgit v1.2.3-18-g5258 (git 2.32.0) at 2025-07-06 11:59:55 +0000