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 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].

 
s being lost
somewhere in openscanhub's reporting pipeline. FWIW GCC 13 onwards can
emit its reports in SARIF format, and other recent analyzer tools can
do so too (see e.g. https://github.com/oasis-tcs/sarif-spec/wiki/Known-
Producers-and-Consumers#known-producers-of-sarif
).  For a future GCC
(15?) I've been experimenting with exposing GCC's path-visualization
code via a new libdiagnostics.so shared library
(https://gcc.gnu.org/wiki/libdiagnostics), perhaps with a precanned way
to "replay" .sarif files, which should make it fairly easy to generate
nice HTML from SARIF showing more context for a warning.  Siteshwar,
let me know if you want to work with me on getting that working in your
pipeline (if so, I can raise the priority of it in my own task list).

Either me or somebody from the OpenScanHub team should be able to coordinate with you about it.
 

I can look at specific bugs in GCC's -fanalyzer if there are patterns
of false positives (ideally please file them in upstream gcc bugzilla
with simple reproducers), or help with specific examples, but right now
without (a) to me the output is unusable noise, sorry.

Siteshwar, I'm hoping that you or somoneone else associated with
openscanhub will volunteer to do (a) and/or (b), or know people who
will; I think these are the two next steps that openscanhub as a
project should be taking.

Yes, we should be able to coordinate on this. Kamil Dudka, the lead of OpenScanHub, is currently on PTO. I would get back to you on this when he is back next week.
 

[...snip...]

Hope this is constructive

Thanks for the feedback!
 
Dave

--
_______________________________________________
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


[1] https://fedoraproject.org/wiki/OpenScanHub
-- 
_______________________________________________
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