<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Hi, Per. Thanks again for working on this. I don't think we can rename the existing core.CallAndMessage, as much as we might want to. Names that aren't in "alpha" are API-ish, and we try not to break them without a strong reason.</div><div><br></div><div>We've also been leaving "Checker" off of the fully-qualified names (e.g. CallAndMessageUnInitRefArg instead of CallAndMessageUnInitRefArgChecker).</div><div><br></div><div><div>+ ChecksFilter filter;</div><div>+ CallAndMessageChecker()</div><div>+ : filter() {}</div></div><div><br></div><div>Since ChecksFilter has a non-trivial constructor, you can just leave out the explicit constructor for CallAndMessageChecker and the right thing will happen.</div><div><br></div><div><div>+ OwningPtr<BugType> &BT, const ParmVarDecl* ParamDecl) const;</div></div><div><br></div><div>Two LLVM style nitpicks: please attach the * to the variable name, and please wrap to the next line to stay within 80 columns.</div><div><br></div><div><div>+ if (filter.check_CallAndMessageUnInitRefArgChecker) {</div></div><div><br></div><div>Suggestion: factor this whole section out into a helper function.</div><div><br></div><div><br></div><div><div>+ //ParamIsPointer</div><div>+ //ParamIsReference</div><div>+ if (ParamDecl->getType().getTypePtr()->isPointerType()) {</div><div>+ ParamIsPointerToConst = ParamDecl->getType().getTypePtr()</div><div>+ ->getPointeeType().isConstQualified();</div><div>+ Message = "Function call argument is a pointer to uninitialized value";</div><div>+ } else if (ParamDecl->getType().getTypePtr()->isReferenceType()) {</div><div>+ ParamIsReferenceToConst = ParamDecl->getType().getTypePtr()</div><div>+ ->getPointeeType().isConstQualified();</div><div>+ Message = "Function call argument is an uninitialized value";</div><div>+ }</div></div><div><br></div><div>A few things here:</div><div>- No need for these comments.</div><div>- QualType has an operator->, so you can drop the .getTypePtr()s without any change.</div><div>- It's probably very slightly nicer to factor out the common code (checking for const qualification on the pointee). I would do something like this:</div><div><br></div><div>if (ParamDecl->getType()->isPointerType())</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>Message = "..."</div><div>// same for references</div><div><br></div><div>if (!Message)</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>return;</div><div>if (!ParamDecl->getType()->getPointeeType().isConstQualified())</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>return;</div><div><br></div><div><br></div><div><div>+ LazyInit_BT("Uninitialized argument value", BT);</div></div><div><br></div><div>This is duplicated below. It's a little unfortunate to have to keep the strings in sync. Maybe we can factor that out somehow? (Or maybe it's better to use different BugTypes. What do you think?)</div><div><br></div><div><br></div><div><div>+ if (argEx)</div><div>+ bugreporter::trackNullOrUndefValue(N, argEx, *R);</div></div><div><br></div><div>We definitely need tests for this. Are we actually going to track back to when this pointer first start pointing to an undefined region? If not, what path notes <i>are</i> we showing here? Does this work for both references and pointers? (You can see how to test path notes in test/Analysis/inlining/path-notes.c.)</div><div><br></div><div>If things aren't working perfectly here, it probably doesn't block the patch, but we should still know where we are today and what still needs to be done. And we don't want to print something that's straight-up incorrect.</div><div><br></div><div>For the test cases, how about some pointers to memory returned by malloc() as well? Also, please add "core" to the set of checkers run in each case...it's a more realistic testing environment to have the core checkers enabled.</div><div><br></div><div>Thank you!</div><div>Jordan</div><div><br></div><br><div><div>On Feb 26, 2014, at 5:29 , Per Viberg <<a href="mailto:Per.Viberg@evidente.se">Per.Viberg@evidente.se</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style id="owaParaStyle" type="text/css">P {margin-top:0;margin-bottom:0;}</style>
<div ocsi="0" fpstyle="1">
<div style="direction: ltr; font-family: Tahoma; font-size: 10pt;"><br>
Hi,<br>
<br>
I've extended the check that warns for uninitialized arguments to:<br>
- warn for pointer arguments that point to uninitialized local (stack) variables.
<br>
- warn for function calls with uninitialized arguments, when the corresponding function parameter is a const reference.
<br>
<br>
<b><font face="Courier New">void doStuff(const int *p);<br>
void doStuffRef(const int& c);<br>
<br>
void f(void) {<br>
int x;<br>
doStuff(&x); // warning<br>
}<br>
</font></b><b><font face="Courier New">void g(void) {<br>
int x;<br>
doStuffRef(x); //</font></b><b><font face="Courier New"><b><font face="Courier New"><b><font face="Courier New"> warning</font></b><br>
</font></b> </font></b><br>
<br>
I've also parameterize the CallAndMessageChecker in a similar way as CheckSecuritySyntaxOnly.<br>
<br>
that is:<br>
// only check if arguments are uninitialized (the old functionality only)<br>
clang -cc1 -analyze -analyzer-checker=core.callandmessage uninit-const.cpp<br>
<br>
// with extended functionality, i.e. "check if const pointer arguments are uninitialized" (also performs the old check).<br>
clang -cc1 -analyze -analyzer-checker=alpha.core.CallAndMessageUnInitRefArgChecker uninit-const.cpp<br>
<b><font face="Courier New"><br>
</font></b>
<div><br>
/Per<br>
<br>
<div style="font-size:13px; font-family:Tahoma"><p class="MsoNormal"><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US">.......................................................................................................................</span><span style="font-size: 8pt; font-family: Arial, sans-serif;" lang="EN-US"><br>
Per Viberg </span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US">Senior Engineer</span><span style="font-size:8.5pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US"><br>
Evidente ES East</span><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-US"> AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
</span><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" lang="EN-US"></span></p><p class="MsoNormal"><span style="font-size:8pt; font-family:'Arial','sans-serif'; color:gray" lang="EN-GB">Phone: +46 (0)8 402 79 00<br>
Mobile: +46 (0)70 912 42 52<br>
E-mail: <a href="mailto:Per.Viberg@evidente.se" target="_blank"><font color="#0000ff">Per.Viberg@evidente.se</font></a>
</span><span style="font-size: 8pt; font-family: Arial, sans-serif;" lang="EN-GB"><br>
<br>
<a href="http://www.evidente.se/" target="_blank"><font color="#0000ff">www.evidente.se</font></a></span></p><p class="MsoNormal"><span style="font-size:6pt; font-family:'Arial','sans-serif'" lang="EN-GB">This e-mail, which might contain confidential information, is addressed to the above stated person/company. If you are not the correct addressee, employee or in
any other way the person concerned, please notify the sender immediately. At the same time, please delete this e-mail and destroy any prints. Thank You.</span></p>
</div>
</div>
</div>
</div>
<span><refToUninitConst_rev1.diff></span>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></div><br></body></html>