<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<!--[if !mso]><style>v\:* {behavior:url(#default#VML);}
o\:* {behavior:url(#default#VML);}
w\:* {behavior:url(#default#VML);}
.shape {behavior:url(#default#VML);}
</style><![endif]--><style><!--
/* Font Definitions */
@font-face
{font-family:Helvetica;
panose-1:2 11 6 4 2 2 2 3 2 4;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:70.85pt 70.85pt 70.85pt 70.85pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="SV" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">New iteration on this checker I’d like to get reviewed. I have moved the checker to Sema and avoid SFINAE.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">/Anders<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Anna Zaks [mailto:ganna@apple.com]
<br>
<b>Sent:</b> den 18 november 2013 18:39<br>
<b>To:</b> Anders Rönnholm<br>
<b>Cc:</b> Richard Smith; Ted Kremenek; cfe-commits@cs.uiuc.edu; Jordan Rose<br>
<b>Subject:</b> Re: [PATCH] [StaticAnalyzer] New checker Sizeof on expression<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Nov 18, 2013, at 1:56 AM, Anders Rönnholm <<a href="mailto:Anders.Ronnholm@evidente.se">Anders.Ronnholm@evidente.se</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I have run the checker on llvm codebase and chrome and did not find any false positives. Not any true positives either. I also ran it on some SFINAE
examples with no false positives. But that was expected as I have not seen any SFINAE examples where they use a binary expression in the sizeof nor sizeof(sizeof()), is that really common?</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I saw that there is an isSFINAEContext function in semaexpr. What do you think about moving the check to sema and use that function to avoid triggering
on SFINAE?</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal">Adding the test case referenced by Richard, moving the check to Sema, and resubmitting the patch sound like a good plan to me.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Anna.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">/Anders</span><o:p></o:p></p>
</div>
<div style="margin-left:65.2pt">
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<div>
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span></span><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">Anna
Zaks [<a href="mailto:ganna@apple.com">mailto:ganna@apple.com</a>]<span class="apple-converted-space"> </span><br>
<b>Sent:</b><span class="apple-converted-space"> </span>den 23 oktober 2013 17:35<br>
<b>To:</b><span class="apple-converted-space"> </span>Daniel Marjamäki<br>
<b>Cc:</b><span class="apple-converted-space"> </span>Ted Kremenek; Anders Rönnholm; Richard Smith;
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>; Jordan Rose<br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: [PATCH] [StaticAnalyzer] New checker Sizeof on expression</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Daniel,<o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">I think Richard was asking about false positives/true positives ratio and I do not see any false positives in the list below. It looks like all the reports you found are valid. Did you run this check on large C++ codebases?<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">As far as I can tell there are two outstanding issues:<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> - Addressing these questions from Richard:<o:p></o:p></p>
</div>
</div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">sizeof(expression) is a common idiom in SFINAE contexts. Is that covered here?</span><o:p></o:p></p>
</div>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<div>
<p class="MsoNormal">Richard, can you provide examples?<o:p></o:p></p>
</div>
</div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">sizeof(sizeof(int)) is a "traditional" way to get sizeof(size_t). Why should we warn on that?</span><o:p></o:p></p>
</div>
</div>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</div>
<div>
<div>
<p class="MsoNormal">(I personally think that if this check goes into the analyzer, it's fine to warn in the second case.)<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> - Where this check should go (compiler or the analyzer)?<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">I think that if you can address the questions above, this check could go into the compiler.<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Cheers,<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Anna.<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal">On Oct 14, 2013, at 12:43 PM, Daniel Marjamäki <<a href="mailto:Daniel.Marjamaki@evidente.se"><span style="color:purple">Daniel.Marjamaki@evidente.se</span></a>> wrote:<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><br>
<br>
<br>
<o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""><br>
Hello!<br>
<br>
As I understood it.. you wanted to know what the hit ratio is. I have investigated the hit ratio on a number of packages in debian. Below is the negative and positives I've found. I provide the package URLs so you can look for yourself..<br>
<br>
I will continue investigating and let you know if I see any false positives...<br>
<br>
<br>
NEGATIVE (macro)<br>
package:<span class="apple-converted-space"> </span><a href="http://ftp.us.debian.org/debian/pool/main/a/agedu/agedu_9723.orig.tar.gz"><span style="color:purple">http://ftp.us.debian.org/debian/pool/main/a/agedu/agedu_9723.orig.tar.gz</span></a><br>
<br>
#define sresize(array, number, type) \<br>
( (void)sizeof((array)-(type *)0), \<br>
(type *) srealloc ((array), (number) * sizeof (type)) )<br>
<br>
We don't warn about this since it's in a macro. However it is interesting:<br>
sizeof((array)-(type *)0)<br>
It seems to be written this way by intention. To get sizeof(ptrdiff_t)? Maybe sizeof(ptr-ptr) should be ok.<br>
<br>
<br>
<br>
POSITIVE<br>
package:<span class="apple-converted-space"> </span><a href="http://ftp.us.debian.org/debian/pool/main/a/argus-client_3.0.0.orig.tar.gz"><span style="color:purple">http://ftp.us.debian.org/debian/pool/main/a/argus-client_3.0.0.orig.tar.gz</span></a><br>
<br>
file: argus-clients-3.0.0/radump/print-ldp.c<br>
line: 607<br>
print_unknown_data(tptr+sizeof(sizeof(struct ldp_msg_header)),"\n\t ",<br>
msg_len);<br>
<br>
file: argus-clients-3.0.0/radump/print-lmp.c<br>
line: 878<br>
print_unknown_data(tptr+sizeof(sizeof(struct lmp_object_header)),"\n\t ",<br>
lmp_obj_len-sizeof(struct lmp_object_header));<br>
<br>
Not sure if these are intended to be sizeof(size_t) or sizeof(struct lmp_object_header) but I guess the latter.<br>
<br>
<br>
<br>
POSITIVE<br>
package:<span class="apple-converted-space"> </span><a href="http://ftp.us.debian.org/debian/pool/main/b/babel/babel_1.4.0.dfsg.orig.tar.gz"><span style="color:purple">http://ftp.us.debian.org/debian/pool/main/b/babel/babel_1.4.0.dfsg.orig.tar.gz</span></a><br>
file: babel-1.4.0.dfsg/regression/output/libC/synch_RegOut_Impl.c<br>
<br>
332: if ((res < 0) || (res > sizeof(s_result_strings)/sizeof(sizeof(char *)))) {<br>
<br>
This "sizeof(sizeof(char *))" should be "sizeof(char*)" since s_result_strings is a array of char * pointers<br>
<br>
<br>
<br>
POSITIVE<br>
package:<span class="apple-converted-space"> </span><a href="http://ftp.us.debian.org/debian/pool/main/b/bird/bird_1.3.7.orig.tar.gz"><span style="color:purple">http://ftp.us.debian.org/debian/pool/main/b/bird/bird_1.3.7.orig.tar.gz</span></a><br>
file: bird-1.3.7/lib/mempool.c<br>
<br>
253:<br>
return ALLOC_OVERHEAD + sizeof(struct linpool) +<br>
cnt * (ALLOC_OVERHEAD + sizeof(sizeof(struct lp_chunk))) +<br>
m->total + m->total_large;<br>
<br>
Not sure if this is intended to be sizeof(size_t) or sizeof(struct lp_chunk) but I guess the latter.<br>
<br>
<br>
POSITIVE:<br>
package:<span class="apple-converted-space"> </span><a href="http://ftp.us.debian.org/debian/pool/main/b/blender/blender_2.49.2~dfsg.orig.tar.gz"><span style="color:purple">http://ftp.us.debian.org/debian/pool/main/b/blender/blender_2.49.2~dfsg.orig.tar.gz</span></a><br>
file: blender-2.49b.orig/source/blender/blenloader/intern/readfile.c<br>
6987: if(0==strncmp(ima->id.name+2, "Viewer Node", sizeof(ima->id.name+2))) {<br>
6991: if(0==strncmp(ima->id.name+2, "Render Result", sizeof(ima->id.name+2))) {<br>
<br>
These are bad, ima->id.name is an array, it should probably be changed to:<br>
if(0==strncmp(ima->id.name+2, "Viewer Node", sizeof(ima->id.name) - 2)) {<br>
<br>
<br>
Best regards,<br>
Daniel Marjamäki</span><o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span><o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:8.0pt;font-family:"Arial","sans-serif";color:gray">..................................................................................................................</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:8.0pt;font-family:"Arial","sans-serif"">Daniel Marjamäki<span class="apple-converted-space"> </span><span style="color:gray">Senior Engineer</span></span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:8.5pt;font-family:"Arial","sans-serif";color:gray">Evidente ES East</span><span class="apple-converted-space"><span lang="EN-US" style="font-size:8.0pt;font-family:"Arial","sans-serif";color:gray"> </span></span><span lang="EN-US" style="font-size:8.0pt;font-family:"Arial","sans-serif";color:gray">AB <span class="apple-converted-space"> </span>Warfvinges
väg 34 <span class="apple-converted-space"> </span>SE-112 51 Stockholm <span class="apple-converted-space"> </span>Sweden</span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:8.0pt;font-family:"Arial","sans-serif";color:gray">Mobile: <span class="apple-converted-space"> </span>+46 (0)709 12 42 62</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:8.0pt;font-family:"Arial","sans-serif";color:gray">E-mail: <span class="apple-converted-space"> </span> <span class="apple-converted-space"> </span><a href="mailto:Daniel.Marjamaki@evidente.se"><span style="color:purple">Daniel.Marjamaki</span></a><a href="mailto:Daniel.Marjamaki@evidente.se"><span style="color:purple">@evidente.se</span></a> <span class="apple-converted-space"> </span> </span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:8.0pt;font-family:"Arial","sans-serif""><a href="http://www.evidente.se/"><span style="color:purple">www.evidente.se</span></a></span><o:p></o:p></p>
</div>
</div>
</div>
</div>
<div>
<div class="MsoNormal" align="center" style="text-align:center">
<hr size="2" width="100%" align="center">
</div>
<div id="divRpF633884">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">Från:</span></b><span class="apple-converted-space"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span></span><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""><a href="mailto:cfe-commits-bounces@cs.uiuc.edu"><span style="color:purple">cfe-commits-bounces@cs.uiuc.edu</span></a><span class="apple-converted-space"> </span>[<a href="mailto:cfe-commits-bounces@cs.uiuc.edu"><span style="color:purple">cfe-commits-bounces@cs.uiuc.edu</span></a>]
för Anna Zaks [<a href="mailto:ganna@apple.com"><span style="color:purple">ganna@apple.com</span></a>]<br>
<b>Skickat:</b><span class="apple-converted-space"> </span>den 8 oktober 2013 18:18<br>
<b>Till:</b><span class="apple-converted-space"> </span>Ted Kremenek<br>
<b>Cc:</b><span class="apple-converted-space"> </span>Anders Rönnholm; Richard Smith;<span class="apple-converted-space"> </span><a href="mailto:cfe-commits@cs.uiuc.edu"><span style="color:purple">cfe-commits@cs.uiuc.edu</span></a><br>
<b>Ämne:</b><span class="apple-converted-space"> </span>Re: [PATCH] [StaticAnalyzer] New checker Sizeof on expression</span><o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal">On Oct 7, 2013, at 10:05 PM, Ted Kremenek <<a href="mailto:kremenek@apple.com" target="_blank"><span style="color:purple">kremenek@apple.com</span></a>> wrote:<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><br>
<br>
<br>
<o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">On Oct 7, 2013, at 6:05 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com" target="_blank"><span style="color:purple">jordan_rose@apple.com</span></a>> wrote:</span><o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""><br>
<br>
<br>
</span><o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">On Oct 7, 2013, at 13:58, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank"><span style="color:purple">richard@metafoo.co.uk</span></a>> wrote:</span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""><br>
<br>
<br>
</span><o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">On Mon, Oct 7, 2013 at 9:55 AM, Jordan Rose<span class="apple-converted-space"> </span><<a href="mailto:jordan_rose@apple.com" target="_blank"><span style="color:purple">jordan_rose@apple.com</span></a>><span class="apple-converted-space"> </span>wrote:</span><o:p></o:p></p>
</div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">I'm fine with this staying in the analyzer for now, unless David, Richard, or Eli feel it should be a warning right away.</span><o:p></o:p></p>
</div>
</blockquote>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">Do we have evidence that we want this? Does it catch bugs? If so, what do they look like? It seems like this would trigger on legitimate code; how does a user suppress the
warning in that case, and does that suppression make their code clearer?</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">What is the false/true positive ratio for bug finding here?</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">sizeof(expression) is a common idiom in SFINAE contexts. Is that covered here?</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">sizeof(sizeof(int)) is a "traditional" way to get sizeof(size_t). Why should we warn on that?</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">And more as a general question than something specific to this patch: Is there a region in the space of false positive ratios where we think a syntactic warning should go
into the static analyzer? If so, why? And what is that region? I would have thought that the static analyzer, like the clang warnings, would be aimed at (eventually) having a false positive ratio of near zero. If so, then should we ever put a warning in the
static analyzer if it doesn't require the static analyzer's technology (or have a high runtime cost)?</span><o:p></o:p></p>
</div>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">On this last (and bringing in Ted and Anna):</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">I think the main difference between compiler warnings and syntactic analyzer checks is that we try very hard to turn new compiler warnings on by default. A second-order
effect of this is that we generally avoid style warnings. The analyzer can be a bit looser about this, though: because people<span class="apple-converted-space"> </span><i>know</i> the analyzer is stricter and more in-depth, I think they might also accept
that a particular check doesn't fit their project.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">On the other hand, we still haven't gotten around to designing a proper bug tracking and/or manual suppression system, so that's one advantage of compiler warnings. And
as you say, checks without a high runtime cost don't really have a<span class="apple-converted-space"> </span><i>technical</i> reason to be in the analyzer.</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">Richard’s point is correct that we want the static analyzer to also have a high signal-to-noise ratio. Otherwise it is a useless tool. I’m also not a fan of having the
analyzer having a bunch of “junk” checkers that aren’t on by default, but if a checker, when it is enabled, is HIGHLY useful to some set of people (e.g., security experts who are more tolerant of false positive rates if they want to do an aggressive code audit)
I think the analyzer is a reasonable place to put them, given the current warning policy in Clang where we want the warnings there to be generally useful to everybody.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">To put it in more context, in the beginning the guiding principal of what goes in the static analyzer was:</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">(a) The warning is very expensive to compute.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> OR</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">(b) The warning is very domain-specific. For example, an API such as CFNumberCreate() on OS X has some interesting API invariants, but we generally should not be hacking
those API-specific warnings into the compiler. Exceptions exist of course, e.g., printf checking, but often the are grounded when such APIs are fairly standard (e.g., in the C standard itself) or the checking is based on some annotation like __attribute__((format))
where the compiler doesn’t know anything about a specific function itself, just the annotation.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">Style warnings can sometimes fall into (a) (in which case not putting them in the compiler makes obvious sense), but one could argue that they are more the flavor of (b)
then a traditional compiler warning. As I mentioned earlier, the static analyzer can be home to some highly specialized checkers that may not be generally useful for everybody but when enabled are very useful to certain people. Style warnings often fit in
this category of warnings.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">A related problem is that we don’t have an ontology for style warnings in clang (the compiler). Should they really be lumped into the same group of all other compiler warnings?
What about their behavior with -Werror? -Weverything? Style warnings really are about personal style, and users can be highly polarized about them. For example, it would be highly useful for Clang to have a -W80-columns or -Wspaces-instead-of-tabs (not
a serious proposal). Both would be cheap to compute, would obviously benefit LLVM developers, but they wouldn’t work for everybody, or even the majority of software developers. Where should they go? The compiler? The static analyzer? Both have discoverability
and usability concerns in either location. I’d argue that these two warnings would be better suited in the compiler because (when the user wants them) they’d get run all the time, but having these warnings counteracts the general guiding principal that compiler
warnings should generally be useful to most people, or clear categories of people (e.g, library authors), but not specific groups of people (e.g., LLVM developers).</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif""> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"">This is not a thought out proposal, but I would not be opposed to a new category of warnings, say -Wstyle, and have all style warnings under -Wstyle, perhaps named “-Wstyle-80-columns”
or “-Wstyle-spaces-instead-of-tabs”. None of these warnings would be on by default. If we want these kind of warnings in the compiler, I think these kind of warnings are worth calling out as being something “different” that people shouldn’t get hung up about
if they don’t think the warning applies to them. Also, having them in a special category of them own allows institutions to set broad warning polices such as “-Werror -Wno-error=style” where they want regular warnings to be treated as error, but not style
warnings, etc.</span><o:p></o:p></p>
</div>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal">I agree with Ted. Currently, there is no good way of adding style and highly specialized warnings to clang; given the behavior of Werror and Weverything. For example, we encourage users to learn about new generic warnings by turning on
Weverything, so the specialized warnings should not be included there. <o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>