<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=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@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;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
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.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle21
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
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 bgcolor="white" lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="color:windowtext">Hi,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">Yes, maybe this extra beta category would create another checker grouping confusion.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">I think even alpha checkers should not crash (at least not on well known open source projects. Llvm clang, postgresql, openssl etc.).<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">So probably alpha checkers should not be accepted until they can be driven to crash easily. And more or less this is the current practice too.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">What would be nice to have is the listing possibility of the checkers in this grouping:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">- be able to list stable checkers only<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">- alpha checkers only (which may be giving some FPs or returning unreasonable explanations)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">- be able to list checkers that are meant for developers only: debug checkers, modeling checkers<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">Some users would like to use immature checkers to find some more true positives at the cost of receiving more FPs or hard to understand reports.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">What we could do is to add Artem’s notes on the specific checkers to the checker documentation as limitation.
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">Then users would know what they can and what they should not expect from an alpha. And it would be a hint for developers too what can be improved.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">I will start a patch for this…<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><br>
Regards,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">Daniel<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="color:windowtext">From:</span></b><span style="color:windowtext"> Ádám Balogh
<br>
<b>Sent:</b> Thursday, May 23, 2019 10:23 AM<br>
<b>To:</b> Artem Dergachev <noqnoqneo@gmail.com>; Kristóf Umann <dkszelethus@gmail.com><br>
<b>Cc:</b> György Orbán <gyorgy.orban@ericsson.com>; cfe-dev <cfe-dev@lists.llvm.org>; Dániel Krupp <daniel.krupp@ericsson.com><br>
<b>Subject:</b> RE: [analyzer] Alpha checker statuses.<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="color:windowtext">Hello,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">I think that a typical beta checker could be the iterator checkers. They are stable, but some of them produces tons of false positives which are very difficult to fix. Enabling an alpha can crash your whole
analysis on some of your files but there is no such danger for the beta. They are only very noisy but you can filter your results after running them. So you do not loose other findings because of them, but on some projects their results are of no real use.
These checkers may or may not be enabled by default, but the users are not discouraged to using them.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext">Regards,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="HU" style="color:windowtext">Ádám<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="color:windowtext">From:</span></b><span style="color:windowtext"> Artem Dergachev <<a href="mailto:noqnoqneo@gmail.com">noqnoqneo@gmail.com</a>>
<br>
<b>Sent:</b> 2019. május 22., szerda 23:04<br>
<b>To:</b> Kristóf Umann <<a href="mailto:dkszelethus@gmail.com">dkszelethus@gmail.com</a>><br>
<b>Cc:</b> György Orbán <<a href="mailto:gyorgy.orban@ericsson.com">gyorgy.orban@ericsson.com</a>>; cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>>; Dániel Krupp <<a href="mailto:daniel.krupp@ericsson.com">daniel.krupp@ericsson.com</a>>;
Ádám Balogh <<a href="mailto:adam.balogh@ericsson.com">adam.balogh@ericsson.com</a>><br>
<b>Subject:</b> Re: [analyzer] Alpha checker statuses.<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal" style="margin-bottom:12.0pt">Mmm, i still don't understand what's the *purpose* of the beta package.<br>
<br>
I can understand the idea of "here's a new checker, we addressed all issues we know about but there may be other issues that we might have missed, so please try it out if you're into this sort of stuff". For now such checkers are usually outright turned on
by default.<br>
<br>
But you're proposing to put checkers into beta when they have known classes of false positives that are relatively easy to fix. So it's not for gathering feedback (we already have enough) and there's no plan on addressing feedback rapidly either (because if
you have the capability to address feedback rapidly, there's no reason to start with feedback not addressed). It sounds as if we'll still bounce back all the bug reports against beta checkers with "hey, this is beta, we know it's broken, right?". If that's
the case, then they're not really different from alpha checkers and i wouldn't be comfortable providing them to the users.<o:p></o:p></p>
<div>
<p class="MsoNormal">On 5/22/19 10:57 AM, Kristóf Umann wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal">I implemented what we discussed here in the following manner: Each checker or checker option must either be in:<br>
<br>
* Alpha: Unfinished, users are strongly discouraged from using it. <o:p></o:p></p>
<div>
<p class="MsoNormal">* Beta: Stable but might produce more false positives and the emitted reports might lack proper explanation<br>
* Released: Production ready, very few false positives with easy to understand bug reports.<br>
<br>
Any checker or checker option in addition can be marked as hidden, meaning that it's a developer feature.<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Thu, 16 May 2019 at 01:24, Artem Dergachev <<a href="mailto:noqnoqneo@gmail.com">noqnoqneo@gmail.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Just wanted to also point out that help is very welcome with implementing the missing functionality in the checkers that we've been discussing here.<br>
<br>
(more replies inline)<o:p></o:p></p>
<div>
<p class="MsoNormal">On 5/15/19 5:26 AM, Kristóf Umann wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal">How about this:<br>
<br>
<span style="font-family:"Courier New"">-analyzer-checker-help</span>: Displays production ready (non-modeling) checkers, and beta checkers with a disclaimer for each description. Don't forget to patch clang-tidy!<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">-analyzer-checker-help-developer</span>: Displays only developer (modeling, debug) checkers (renamed from *-hidden).<o:p></o:p></p>
</div>
<p class="MsoNormal"><span style="font-family:"Courier New"">-analyzer-checker-help-alpha</span>: Displays only alpha (non-modeling) checkers with very scary disclaimers both around the list and for each checker description.
<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Note how each of these is mutually exclusive. I think we shouldn't make a flag that displays all checkers, but should allow the following invocation to do so:
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><br>
<span style="font-family:"Courier New"">clang -cc1 -analyzer-checker-help -analyzer-checker-help-alpha -analyzer-checker-help-hidden</span><o:p></o:p></p>
</div>
</div>
</div>
</div>
</blockquote>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
This makes sense to me and i think this is how it should have always been done.<o:p></o:p></p>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Note that hidden checkers may only show up in -analyzer-checker-help-developer. Options for implicit checkers are NOT implicitly hidden, but options for alpha checkers are implicitly marked alpha. <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">On Wed, 15 May 2019 at 12:39, Kristóf Umann <<a href="mailto:dkszelethus@gmail.com" target="_blank">dkszelethus@gmail.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal">+ György Orbán<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Wed, 15 May 2019 at 00:41, Artem Dergachev <<a href="mailto:noqnoqneo@gmail.com" target="_blank">noqnoqneo@gmail.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-bottom:12.0pt">I wanted to give more visibility to the discussion on the status of
<br>
different Static Analyzer "alpha" (unfinished) checkers that's going on <br>
on Phabricator (<a href="https://reviews.llvm.org/D57858" target="_blank">https://reviews.llvm.org/D57858</a>). Story so far: We're
<br>
trying to figure out how many of them can be finished with relatively <br>
little effort (and therefore we should probably ask around to gather <br>
feedback) and how many are clearly not ready for any sort of use and <br>
should be hidden until the most glaring bugs are fixed. For now we <br>
officially treat all alpha checkers as the latter - so don't use them!<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal">Just to add to the summary: despite our "disapproval" of using alpha checkers, we always made them very visible, since if you wanted to list the available checkers, you inevitable came across them (and they are also lexicographically greater
than the rest of them).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Due to the lack of branches in our SVN repository, we used the alpha package to make development incremental, which inevitable resulted in some checkers in there being unfinished and unstable, while others merely need some finishing touches,
and could be used despite being rough around the patches.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">rough around the edges* <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">The discussion came up in a patch that plans to expose checker options, that always existed but were never listable, unless you read the source code. However, just like alpha checkers, many of these also hide features under development,
while other would genuinely be useful to fine tune the analyzer for a specific project.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-bottom:12.0pt">This discussion is important because different people's codebases are
<br>
ridiculously different, so it's almost impossible to estimate the <br>
quality and usefulness of static analysis unless as many varied <br>
codebases as possible are involved.<br>
<br>
>>! In D57858#1500668, @Szelethus wrote:<br>
> `IteratorChecker` is a prime example that still suffers from not <br>
ideal FP/TP ratio, but users at Ericsson value it plenty enough not to <br>
be bothered by it. Many of the changes here and in private directly <br>
address user bug reports (That's just one, but I do remember having <br>
others around too!).<br>
<br>
Once it has visitor path notes about where did it get its iterators <br>
from, some of the iterator checks should definitely be considered for <br>
being turned on by default. Especially the mismatched iterator check <br>
that doesn't rely on hardcore constraint solving. The current upstream <br>
version is not in good shape though; i just tried it on LLVM and it <br>
currently crashes all over the place with "Symbol comparison must be a <br>
`SymIntExpr`" assertion (pls ask me for repros if they aren't obvious). <br>
Also it has false mismatched iterator positives on `A.insert(B.begin(), <br>
B.end())`.<br>
<br>
<br>
>> In <a href="https://reviews.llvm.org/D57858#1501065" target="_blank">https://reviews.llvm.org/D57858#1501065</a>, @dkrupp wrote:<br>
> These are the alpha checkers that we are testing in Ericsson:<br>
<br>
Let me undig my last year's attempt to take a look at alpha checkers. <br>
The most common "limb" to "miss" in the alpha checkers is the "bug <br>
visitor" functionality that'd add checker-specific path notes to the <br>
report, which is almost inevitably necessary for any path-sensitive <br>
checker. Bug reports without path notes are hard to understand, but <br>
that's one thing that your users won't tell you: they often just don't <br>
have their good taste to realize that bug reports shouldn't be so hard <br>
to understand. The users often take it for granted that they have to <br>
figure out themselves where do these values come from, but it's still <br>
our job to not force them to.<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">The fundamental problem here is, in my opinion, that "alpha" doesn't describe many of these checkers too well. I think once a checker is stable and has a "reasonable" true positive/false positive ratio, we should move them out of alpha
status: not only would we be able to gather invaluable feedback for these, but users might appreciate the feature even with a couple shortcomings. However, just because they are not falling apart on their own, these aren't always production ready -- how about
we introduce a "beta" package?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Alpha checkers would be incomplete, incorrect and unstable by definition, and would be hidden from non-developers. Beta checkers would receive a disclaimer that they might emit too much false positives to be production ready and don't emit
ideal bug reports in terms of readability, but are considered stable and usable.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
> alpha.core.BoolAssignment<br>
<br>
Yes, i agree that this one's pretty useful. It's currently missing a <br>
visitor that explains why does the analyzer think that the value is <br>
non-true/false, which is often necessary to understand the warning.<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal">This would be an ideal candidate to be moved to a beta package.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
I'll be comfy to move it straight to core as long as someone stuffs a `bugreporter::trackExpressionValue()` into it and we both test it and see no obvious FP patterns specific to this checker. Unless we do this, i probably won't be comfy putting it into beta
:)<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
> alpha.core.CastSize<br>
<br>
This one's indeed relatively quiet, but i'm seeing ~50 false positives <br>
caused by stuffing metadata at the beginning of a dynamically allocated <br>
buffer. I.e., allocate a buffer that's 4 bytes larger than necessary, <br>
use these 4 bytes for our own bookkeeping and provide a pointer to the <br>
rest of the buffer to be used for the actual value. I don't see an easy <br>
way to fix these false positives, so i don't see how to move this out of <br>
alpha.<br>
<br>
<br>
> alpha.core.Conversion<br>
<br>
Interestingly, i haven't seen this one trigger on our codebases. So i <br>
don't have an opinion here. There's a chance it might be a good opt-in <br>
check. Do you have an open-source project in mind on which this check is <br>
actually useful?<br>
<br>
<br>
> alpha.core.DynamicTypeChecker<br>
<br>
I really root for enabling this checker (and generally improving our <br>
dynamic type-checking), but for now i'm seeing ~600 false positives in <br>
projects that use custom RTTI (something like `dyn_cast`) and ~300 more <br>
Objective-C-specific false positives. I can take a look and try to <br>
reduce some of the custom RTTI ones if you're interested in figuring out <br>
how to fix them; i don't remember if they are easy to fix.<br>
<br>
<br>
> alpha.core.SizeofPtr<br>
<br>
This checker does indeed find interesting bugs sometimes, but i'm <br>
overwhelmed by ~300 false positives in which the sizeof of a pointer is <br>
taken intentionally. This is especially annoying when the pointer is <br>
hidden behind a typedef and the user doesn't need to know whether it's a <br>
pointer or not.<br>
<br>
<br>
> alpha.core.TestAfterDivZero<br>
<br>
I don't see any positives of this checker, but this checker is crazy and <br>
shouldn't have been done this way. It's a "must-problem" check and we <br>
don't have any sort of infrastructure to even display this kind of bug <br>
report properly after we find it, let alone to properly find it. We need <br>
a more-or-less full-featured data flow analysis engine before we make an <br>
attempt on such checker.<br>
<br>
<br>
> alpha.cplusplus.DeleteWithNonVirtualDtor<br>
<br>
I don't see any positives of this checker. Is it any better than the <br>
compiler warning that we have for all polymorphic classes that have no <br>
virtual destructors?<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal">Maybe this one too. <o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
Dunno, why duplicate a compiler warning that's already on by default and more aggressive than the checker?<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal"><br>
> alpha.security.MallocOverflow<br>
<br>
This one's extremely loud for me (~1500 false positives). It looks as if <br>
it warns on every `malloc(x * sizeof(int))` (due to potential integer <br>
overflow during multiplication) so i just don't see it working as an <br>
AST-based check. We should probably rewrite it on top of taint analysis <br>
and then it'll need a constraint solver that knows how to multiply things.<br>
<br>
Like, this is the point where i'd like to ask how does this happen that <br>
you don't see these false positives. Is this checker actually quiet for <br>
you? Or are your users addressing these warnings somehow?<br>
<br>
<br>
> alpha.security.MmapWriteExec<br>
<br>
I don't see any positives of this checker. It probably needs a visitor <br>
(which is trivial) and it definitely needs a solution for different <br>
values of macros on different platforms that'd be better than just <br>
setting them as a config flag.<br>
<br>
<br>
> alpha.security.ReturnPtrRange<br>
<br>
I don't see any positives of this checker. It definitely needs a visitor <br>
and we might as well join it with the generic array bound checker. Also <br>
need to figure out how to deal with, say, vector::end() which is <br>
supposed to return an out-of-bound pointer.<br>
<br>
<br>
> alpha.security.taint.TaintPropagation<br>
<br>
I believe that the generic taint engine is currently very solid, but <br>
specific checks that we have on top of it are very much unfinished: they <br>
simply don't react on any sort of validation that can be used to remove <br>
the taint. Normally that'll involve consulting the constraint manager <br>
(in case of tainted integers) or modeling validation routines (in case <br>
of more complicated objects).<br>
<br>
<br>
> alpha.unix.BlockInCriticalSection<br>
<br>
I have ~10 positives and i didn't have a look at them back then; might <br>
be good. This checker needs a visitor to explain why do we think we're <br>
in a critical section.<br>
<br>
<br>
> alpha.unix.Chroot<br>
<br>
I don't see any positives of this checker. This checker needs a visitor <br>
to highlight chroot.<br>
<br>
<br>
> alpha.unix.PthreadLock<br>
<br>
Uhm, i have a few patches on this checker that i never committed:<br>
- <a href="https://reviews.llvm.org/D37806" target="_blank">https://reviews.llvm.org/D37806</a><br>
- <a href="https://reviews.llvm.org/D37807" target="_blank">https://reviews.llvm.org/D37807</a><br>
- <a href="https://reviews.llvm.org/D37809" target="_blank">https://reviews.llvm.org/D37809</a><br>
- <a href="https://reviews.llvm.org/D37812" target="_blank">https://reviews.llvm.org/D37812</a><br>
- <a href="https://reviews.llvm.org/D37963" target="_blank">https://reviews.llvm.org/D37963</a><br>
And it still needs a visitor. And support for more APIs, 'cause there <br>
are still false positives caused by unobvious POSIX APIs that release <br>
the mutex (sometimes conditionally). And once it's done, i'll be seeing <br>
no positives of this checker; it sounds like a good checker to have, but <br>
it doesn't seem to find mistakes that are *easy to make*.<br>
<br>
<br>
> alpha.unix.SimpleStream<br>
> alpha.unix.Stream<br>
<br>
Those need, at least:<br>
- A bug visitor.<br>
- A suppress-on-sink behavior. Otherwise they warn on every assert <br>
between open and close (~200 false positives for me).<br>
- Pointer escape support.<br>
Also i vaguely remember that the non-simple checker is known to cause <br>
more state splits than necessary.<br>
<br>
<br>
> alpha.unix.cstring.NotNullTerminated<br>
<br>
Hmm, this check looks like a walking .addTransition() bug (unintended <br>
state split) when invoked from getCStringLength(). Also it doesn't seem <br>
to be disabled when the checker is disabled, so i guess we're kinda <br>
"using" it too. But it's also too quiet to matter, as it pretty much <br>
only warns when you're trying to compute a strlen() of a function pointer.<br>
<br>
<br>
> alpha.unix.cstring.OutOfBounds<br>
<br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=41729" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=41729</a> and it also needs a visitor
<br>
for the index. <o:p></o:p></p>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</blockquote>
</div>
</div>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>