On Tue, Jul 9, 2024 at 10:10 PM David Malcolm <dmalc...@redhat.com> 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é > > <berra...@redhat.com> > > 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 > <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@lists.fedoraproject.org > To unsubscribe send an email to devel-le...@lists.fedoraproject.org > 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@lists.fedoraproject.org > Do not reply to spam, report it: > https://pagure.io/fedora-infrastructure/new_issue > [1] https://fedoraproject.org/wiki/OpenScanHub
-- _______________________________________________ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org 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@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
