<div dir="ltr">Hi All,<div>Yes Jordan as you pointed out in the current infrastructure doesn't allow analyzer to conclude if a condition is true along all paths. I will have a look into core if it is possible to achieve the same. Currently i have backed out delete null case. </div>
<div><br></div><div>Added patch for "Argument to delete is uninitialized" . Added test case for the same.</div><div><br></div><div><br></div><div><div>Index: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp</div>
<div>===================================================================</div><div>--- lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp<span class="" style="white-space:pre"> </span>(revision 187716)</div><div>+++ lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp<span class="" style="white-space:pre"> </span>(working copy)</div>
<div>@@ -28,13 +28,16 @@</div><div> </div><div> namespace {</div><div> class CallAndMessageChecker</div><div>- : public Checker< check::PreStmt<CallExpr>, check::PreObjCMessage,</div><div>+ : public Checker< check::PreStmt<CallExpr>,</div>
<div>+ check::PreStmt<CXXDeleteExpr>,</div><div>+ check::PreObjCMessage,</div><div> check::PreCall > {</div><div> mutable OwningPtr<BugType> BT_call_null;</div>
<div> mutable OwningPtr<BugType> BT_call_undef;</div><div> mutable OwningPtr<BugType> BT_cxx_call_null;</div><div> mutable OwningPtr<BugType> BT_cxx_call_undef;</div><div> mutable OwningPtr<BugType> BT_call_arg;</div>
<div>+ mutable OwningPtr<BugType> BT_cxx_arg_undef;</div><div> mutable OwningPtr<BugType> BT_msg_undef;</div><div> mutable OwningPtr<BugType> BT_objc_prop_undef;</div><div> mutable OwningPtr<BugType> BT_objc_subscript_undef;</div>
<div>@@ -44,6 +47,7 @@</div><div> public:</div><div> </div><div> void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;</div><div>+ void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;</div>
<div> void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;</div><div> void checkPreCall(const CallEvent &Call, CheckerContext &C) const;</div><div> </div><div>@@ -57,6 +61,9 @@</div>
<div> void emitNilReceiverBug(CheckerContext &C, const ObjCMethodCall &msg,</div><div> ExplodedNode *N) const;</div><div> </div><div>+ void ReportBadDelete(const CXXDeleteExpr *DE, CheckerContext &C,</div>
<div>+ BugType *BT, StringRef Desc, ExplodedNode *N) const;</div><div>+ </div><div> void HandleNilReceiver(CheckerContext &C,</div><div> ProgramStateRef state,</div><div>
const ObjCMethodCall &msg) const;</div><div>@@ -250,6 +257,32 @@</div><div> C.addTransition(StNonNull);</div><div> }</div><div> </div><div>+void CallAndMessageChecker::ReportBadDelete(const CXXDeleteExpr *DE, </div>
<div>+ CheckerContext &C,</div><div>+ BugType *BT,</div><div>+ StringRef Desc,</div><div>
+ ExplodedNode *N) const {</div><div>+</div><div>+ BugReport *R = new BugReport(*BT, Desc, N);</div><div>+ bugreporter::trackNullOrUndefValue(N, DE, *R);</div><div>+ C.emitReport(R);</div>
<div>+}</div><div>+</div><div>+void CallAndMessageChecker::checkPreStmt(const CXXDeleteExpr *DE, </div><div>+ CheckerContext &C) const {</div><div>+</div><div>+ SVal RHS = C.getSVal(DE->getArgument());</div>
<div>+ if (DE && RHS.isUndef()) {</div><div>+ ExplodedNode *N = C.generateSink();</div><div>+ if (!BT_cxx_arg_undef)</div><div>+ BT_cxx_arg_undef.reset(new BugType("Bad delete", "Memory Error"));</div>
<div>+ StringRef Desc = "Argument to delete is uninitialized";</div><div>+ ReportBadDelete(DE,C,BT_cxx_arg_undef.get(),Desc,N);</div><div>+ return;</div><div>+ }</div><div>+}</div><div>+</div><div>+</div>
<div> void CallAndMessageChecker::checkPreCall(const CallEvent &Call,</div><div> CheckerContext &C) const {</div><div> ProgramStateRef State = C.getState();</div><div>Index: test/Analysis/NewDelete-checker-test.cpp</div>
<div>===================================================================</div><div>--- test/Analysis/NewDelete-checker-test.cpp<span class="" style="white-space:pre"> </span>(revision 187717)</div><div>+++ test/Analysis/NewDelete-checker-test.cpp<span class="" style="white-space:pre"> </span>(working copy)</div>
<div>@@ -4,6 +4,7 @@</div><div> </div><div> typedef __typeof__(sizeof(int)) size_t;</div><div> extern "C" void *malloc(size_t);</div><div>+extern "C" void free (void* ptr);</div><div> int *global;</div>
<div> </div><div> //------------------</div><div>@@ -207,7 +208,26 @@</div><div> escapeVoidPtr(y);</div><div> } // no-warning</div><div> </div><div>+//============== Test Uninitialized delete delete[]========================</div>
<div>+void testUninitDelete() {</div><div>+ int *x;</div><div>+ int * y = new int;</div><div>+ delete x; // expected-warning{{Argument to delete is uninitialized}}</div><div>+ delete y; </div><div>+}</div><div> </div>
<div>+void testUninitDeleteArray() {</div><div>+ int *x;</div><div>+ int * y = new int[5];</div><div>+ delete[] x; // expected-warning{{Argument to delete is uninitialized}}</div><div>+ delete[] y; </div><div>+}</div>
<div>+</div><div>+void testUninitFree() {</div><div>+ int *x;</div><div>+ free(x); // expected-warning{{Function call argument is an uninitialized value}}</div><div>+}</div><div>+</div></div><div><br></div><div><br></div>
<div>Regards</div><div>Karthik</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Aug 6, 2013 at 6:03 AM, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div class="h5"><br><div><div>On Aug 5, 2013, at 10:38 , John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:</div>
<br><blockquote type="cite"><div style="word-wrap:break-word">On Aug 5, 2013, at 9:09 AM, Jordan Rose <<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>> wrote:<br><div><blockquote type="cite">
<div style="font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">delete nullptr / delete 0 is entirely legal in C++. Can you please take that back out?<br>
</div></blockquote><div><br></div>An analysis which points out that what’s being deleted is *always* null would</div><div>probably be useful, but I assume that would require different logic.</div></div></blockquote><br></div>
</div></div><div>Yes...unfortunately the analyzer is not great at proving something true along <i>all</i> paths that lead to a particular statement, instead of just <i>one</i> path.</div><span class="HOEnZb"><font color="#888888"><div>
<br></div><div>Jordan</div><br></font></span></div></blockquote></div><br></div>