On Tue, Aug 27, 2024 at 04:27:47PM +0200, Florian Obser wrote:
> On 2024-08-27 15:35 +02, Janne Johansson <icepic.dz@gmail.com> wrote:
> > Den mån 26 aug. 2024 kl 00:14 skrev Tom Smyth <tom.smyth@wirelessconnect.eu>:
> >> Folks,
> >> Im just wondering what other porters experience of scan-build for the projects that you are maintaining ?
> >> has it been useful in identifying bugs?... or is the analysis engine too basic or shallow to properly analyse code ?
> >
> > When I run it on "openbsd" code, the things it finds are often super
> > deep, requiring 37 steps and that syscalls or libc calls return 0 when
>
> yeah, everything that's more than 10 deep is probably BS. Things that
> are 5 or less deep are actionable in my experience.
>
> [...]
>
> > It does find trivial things like
> > http://c66.it.su.se:8080/obsd/2019-10-25/scan-build-2019-10-25-192004-30128-1/report-36d1ed.html#EndPath
>
> I like to look at dead stores, they are either there for symmetry
> reasons or they indicate that something was not quite thought through.
>
> In both cases just removing the dead store and sending a patch is
> wrong. In the first case you are destroying the symmetry and you are
> just creating noise, and in the 2nd case you didn't put in the effort to
> figure out how that whole function could be written better.
>
> I think scan-build can guide someone who has dabbled in C before where
> to look.
>
> scan-build guidance is not always well received.
>
> Story time: I once considered using and contributing to a project, so
> first step was to run it through scan-build to get a feel for the
> structure of the code and have some ideas where the skeletons are
> buried or where someone was sloppy. So I carefully analysed the reports
> and submitted some patches. They were all happily accepted.
>
> I never mentioned that this came out of scan-build, because it was 99%
> my work anyway. So by patch 10 or 11 I write something like, hey,
> scan-build pointed me at this other thing, it technically can not happen
> because you get lucky all the way over there, but it is a pretty well
> aimed foot gun, and this should be written more defensive over
> here. Patch attached.
>
> Their (only!) response: Yeah, we are not interested in scan-build
> reports.
>
> OK then, good luck to you I guess...
Some years back I spent quite some time reviewing hundreds of scan-build
reports against Got. in the end there were 6 patches I could commit, all
of which fixed minor issues which either wouldn't ever have mattered or
might have bitten someone eventually, with an obvious diagnosis and fix
(NULL pointer derefts in error cases and such). The tool did not manage to
find any of the important bugs that were eventually found in other ways
over the next few years.
I had to wade through hundreds of false positives because scan-build is not
clever enough to see when variables are initialized across function calls.
This happens all the time in Got because, by convention, callees initialize
output arguments for callers. This alone made the reports much too noisy.
I'd have to check each such report to verify that it was indeed a false
postiive ("oh right, the scanner made this mistake *again*"), which felt
like a waste of my time.
I recall you managed to find a few more issues when you went through the
same process some other time, and more fixes were committed. I'm grateful
for that. Having done it once I wouldn't have the energy to wade through
hundreds of scan-build results myself anymore.
Maybe scan-build works better with other code bases which use a different
style. Though I recently looked at a few results from nsh's code base and
noticed that the scanner is still making the same mistake.
No comments:
Post a Comment