Date: Fri, 6 Sep 2024 09:15:45 -0400 From: Jan Knepper <jan@digitaldaemon.com> To: David Chisnall <theraven@freebsd.org>, Alan Somers <asomers@freebsd.org> Cc: Dmitry Salychev <dsl@freebsd.org>, freebsd-hackers@freebsd.org Subject: Re: The Case for Rust (in any system) Message-ID: <1dcfee7b-11e7-4bde-aa5f-6d90101a6022@digitaldaemon.com> In-Reply-To: <6FEF9D06-01DC-48DC-93D2-178F9726C1D3@freebsd.org> References: <CAOtMX2j=EA5XLQ6jG3_XRyLd7QPj4j-nKKoCMdiYA7QoMNmQZg@mail.gmail.com> <6FEF9D06-01DC-48DC-93D2-178F9726C1D3@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------SAXOS04t0oRybAieYmCfmxlX
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
On 9/6/24 03:02, David Chisnall wrote:
> On 5 Sep 2024, at 22:13, Alan Somers<asomers@freebsd.org> wrote:
>> I used to check it, years ago. But I gave up. The UI is too hard to
>> use and false alarms are both too frequent and too hard to suppress.
>> Plus, it's a real drag that I can't run the tool myself. Instead, I
>> need to wait for the next scheduled run.
> In general, it’s very hard to add static analysis to existing projects.
Not the experience I have.
It was as simple as wrapping the build with 'cov-<something>'.
(Also, Coverity would come and visit us, and assist with setting up and
getting analyzes started).
> The property that you want is that there are no *new* static analyser errors in a new commit, but that’s requires tracking all of the existing ones.
Coverity, when we used it did keep track of all the issues:
A false positive, when marked as false, would not be reported again.
A true positive, thus an actual problem, most (all!) of the time would
disappear after the code was fixed and Coverity was run again.
Every analyzer I have ever used gives false positives.
VeraCode was one of the worst.
Coverity was one of the best.
(But times have changed)
> In CHERIoT RTOS, we run the clang analyser in CI as one of the checks that must pass before a PR can be merged. This is possible because we started doing it very early on.
Good! :-)
> It may be possible for some individual parts of FreeBSD, but when we started with Coverity I looked at the reports and the first ten I looked at were all false positives.
That can happen. I think when we first started with Coverity the false
positives where just less then 50%. Which is truly extremely good for an
analyzer. Then Coverity was tuned/reconfigured a bit and it became much
better.
> There are probably some serious issues in there but the effort to find them is high. For a new project, that cost is a small incremental cost in each commit and code review (if the analyser finds something, reviewer has to agree that it’s a false positive).
>
Well, the cost is high on an existing project alike FreeBSD, because
this has not been done... My guess is... It could be considered
'Technical Debt'.
What helps quite a bit is tuning Coverity to the right configuration, so
the false positives are not overwhelming. Then, when the true positives
are resolved, dial the tuning back a little.
Jan
--------------SAXOS04t0oRybAieYmCfmxlX
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
On 9/6/24 03:02, David Chisnall wrote:<br>
<blockquote type="cite"
cite="mid:6FEF9D06-01DC-48DC-93D2-178F9726C1D3@freebsd.org">
<pre class="moz-quote-pre" wrap="">On 5 Sep 2024, at 22:13, Alan Somers <a class="moz-txt-link-rfc2396E" href="mailto:asomers@freebsd.org"><asomers@freebsd.org></a> wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
I used to check it, years ago. But I gave up. The UI is too hard to
use and false alarms are both too frequent and too hard to suppress.
Plus, it's a real drag that I can't run the tool myself. Instead, I
need to wait for the next scheduled run.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
In general, it’s very hard to add static analysis to existing projects.</pre>
</blockquote>
Not the experience I have. <br>
It was as simple as wrapping the build with 'cov-<something>'.<br>
(Also, Coverity would come and visit us, and assist with setting up
and getting analyzes started).<br>
<blockquote type="cite"
cite="mid:6FEF9D06-01DC-48DC-93D2-178F9726C1D3@freebsd.org">
<pre class="moz-quote-pre" wrap=""> The property that you want is that there are no *new* static analyser errors in a new commit, but that’s requires tracking all of the existing ones.</pre>
</blockquote>
Coverity, when we used it did keep track of all the issues: <br>
A false positive, when marked as false, would not be reported again.
<br>
A true positive, thus an actual problem, most (all!) of the time
would disappear after the code was fixed and Coverity was run again.<br>
<br>
Every analyzer I have ever used gives false positives. <br>
VeraCode was one of the worst. <br>
Coverity was one of the best.<br>
(But times have changed)<br>
<blockquote type="cite"
cite="mid:6FEF9D06-01DC-48DC-93D2-178F9726C1D3@freebsd.org">
<pre class="moz-quote-pre" wrap=""> In CHERIoT RTOS, we run the clang analyser in CI as one of the checks that must pass before a PR can be merged. This is possible because we started doing it very early on.</pre>
</blockquote>
Good! :-)<br>
<blockquote type="cite"
cite="mid:6FEF9D06-01DC-48DC-93D2-178F9726C1D3@freebsd.org">
<pre class="moz-quote-pre" wrap=""> It may be possible for some individual parts of FreeBSD, but when we started with Coverity I looked at the reports and the first ten I looked at were all false positives.</pre>
</blockquote>
That can happen. I think when we first started with Coverity the
false positives where just less then 50%. Which is truly extremely
good for an analyzer. Then Coverity was tuned/reconfigured a bit and
it became much better.<br>
<blockquote type="cite"
cite="mid:6FEF9D06-01DC-48DC-93D2-178F9726C1D3@freebsd.org">
<pre class="moz-quote-pre" wrap=""> There are probably some serious issues in there but the effort to find them is high. For a new project, that cost is a small incremental cost in each commit and code review (if the analyser finds something, reviewer has to agree that it’s a false positive).
</pre>
</blockquote>
Well, the cost is high on an existing project alike FreeBSD, because
this has not been done... My guess is... It could be considered
'Technical Debt'.<br>
What helps quite a bit is tuning Coverity to the right
configuration, so the false positives are not overwhelming. Then,
when the true positives are resolved, dial the tuning back a little.<br>
<br>
Jan<br>
<br>
<br>
</body>
</html>
--------------SAXOS04t0oRybAieYmCfmxlX--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1dcfee7b-11e7-4bde-aa5f-6d90101a6022>