Re: Flaws detected by static analyzers in Fedora 41 Critical Path Packages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 10, 2024 at 11:10:34AM +0200, Siteshwar Vashisht wrote:
> On Tue, Jul 9, 2024 at 10:10 PM David Malcolm <dmalcolm@xxxxxxxxxx> wrote:
> 
> > On Tue, 2024-07-09 at 13:37 +0200, Siteshwar Vashisht wrote:
> > > On Tue, Jul 9, 2024 at 1:16 PM Daniel P. Berrangé
> > > <berrange@xxxxxxxxxx>
> > > wrote:
> > >
> > > > On Sat, Jul 06, 2024 at 02:05:37AM +0200, Siteshwar Vashisht wrote:
> > > > > Hello,
> > > > >
> > > > > I am writing this message to get feedback from the community on
> > > > > possibly
> > > > > new defects identified by static analyzers in Critical Path
> > > > > Packages that
> > > > > have changed in Fedora 41. For context, please see my previous
> > > > > email[1].
> > > > >
> > > > > TLDR: This report[2] contains 73976 identified defects. Please
> > > > > review the
> > > > > report and provide feedback.
> > > >
> > > > Calling these "Identified defects" is way too strong & a misleading
> > > > portrayal of package quality IMHO.
> > > >
> > > > These are identified code locations which may or may not be
> > > > defects.
> > > > We've no idea what the actual defect level is, amongst the false
> > > > positives, unless humans analyse each report.
> > > >
> > > > >
> > > > > A mass scan was performed this week on the packages that have
> > > > > changed in
> > > > > Fedora 41. This report[2] contains all the new defects that have
> > > > > been
> > > > > identified in the packages listed in Critical Path Packages.
> > > > > Please
> > > > review
> > > > > the report and fix or report any defects to upstream that may be
> > > > > real
> > > > bugs.
> > > > > Not all defects reported by OpenScanHub may be actual bugs, so
> > > > > please
> > > > > verify reported defects before investing time into fixing or
> > > > > reporting
> > > > > them. We hope this is helpful for the packages you maintain and
> > > > > for the
> > > > > upstream projects. Questions can be asked on the OpenScanHub
> > > > > mailing
> > > > > list[3]. If you want to see the full logs of the scans, they are
> > > > available
> > > > > on the tasks[4] page. User documentation for performing a scan is
> > > > available
> > > > > on the Fedora wiki[5].
> > > > >
> > > > > Please remember this is currently an early production stage for
> > > > OpenScanHub
> > > > > scanning. Constructive feedback is appreciated. Thank you!
> > > >
> > > > For packages I'm involved in (QEMU, libvirt), there are a huge
> > > > number of
> > > > reported "flaws". The false positive error reports level is way too
> > > > high
> > > > for me to spend time looking at these reports in any detail though.
> > > >
> > > > The biggest problem is that the clang 'warning[unix.Malloc]' check
> > > > doesn't
> > > > understand that __attribute__((cleanup)) functions (via the glib
> > > > g_autofree
> > > > / g_autoptr macros) will free memory. On libvirt this accounts for
> > > > 35% of
> > > > all warnings list, and QEMU it accounts for about 20% of warnings.
> > > > There
> > > > are probably some real memory leaks there, but it is impractical to
> > > > search
> > > > for them amongst the noise.
> > > >
> > > > Another 30% are "DeadStore" warnings which, while correct, are also
> > > > harmless
> > > > and not something we intend to fix since this is generated code &
> > > > making
> > > > the
> > > > generator more complex is not desired.
> > > >
> > >
> > > I request somebody from the tools team to comment on these concerns.
> > > We
> > > only report the defects identified by gcc, clang etc.
> >
> >
> > I'm on RH's tools team (I work on upstream GCC), and I'll comment a
> > little on the specifics of the above in a separate mail.
> >
> > That said, I think there are two high-level issues here, which someone
> > (probably on the openscanhub team???) needs to be responsible for:
> >
> > (a) improving the readability of these generated reports so that if
> > someone clicks on a report it gives them enough information to assess
> > it, otherwise the report is effectively "noise".
> >
> > (b) "curating" the warnings: doing an initial pass through the taxonomy
> > of warnings, and prioritizing some subset that seems worth the
> > attention of the package maintainers, and focusing on that (and
> > gradually tuning/expanding this).
> >
> 
> Good point. We need to come up with some new (or reuse existing) tooling to
> mark important warnings.
> 
> 
> >
> >
> > Regarding (a) I've spent a *lot* of work in upstream GCC to try to make
> > -fanalyzer's reports readable e.g. showing predicted execution paths
> > that trigger a problem, both in terms of capturing the data, and
> > visualizing it.  However, looking at e.g.
> >
> > https://openscanhub.fedoraproject.org/task/242/log/units-2.22-6.fc39/scan-results.html#def5
> > these aren't visible in the reports you linked to, simply the site of
> > the final problem.  This isn't helpful, and is frustrating, given that
> > GCC *is* emitting the pertinent information, but it'
> 
> 
> This has been discussed in the past, and you can use below command to see
> more verbose output:
> 
> curl -s "
> https://openscanhub.fedoraproject.org/task/242/log/units-2.22-6.fc39/scan-results.js?format=raw";
> | csgrep
> 
> It is actually documented in the wiki[1].

Having ability to process data from the CLI is great, but I'd still
encourage you to look at making the HTML reports more usable.

One improvement that is fairly easy is to present a menu at the top
of the page listing all checkers, and all check types within that
checker, and allow each checker overall, individual checks, to be
toggled visible.

eg this semi-working crude mockup :

  https://berrange.fedorapeople.org/scan.html


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

-- 
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Fedora Announce]     [Fedora Users]     [Fedora Kernel]     [Fedora Testing]     [Fedora Formulas]     [Fedora PHP Devel]     [Kernel Development]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [PAM]     [Red Hat Development]     [Gimp]     [Yosemite News]

  Powered by Linux