On Wed, Jul 10, 2024 at 12:29 PM Siteshwar Vashisht <svashi...@redhat.com> wrote: > > > > On Wed, Jul 10, 2024 at 12:21 PM Daniel P. Berrangé <berra...@redhat.com> > wrote: >> >> On Wed, Jul 10, 2024 at 11:10:34AM +0200, Siteshwar Vashisht wrote: >> > 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]. >> >> 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 > > > Thanks for sharing the prototype! I have been using some internal scripts > that were used to create reports for RHEL, but we can develop something more > user friendly for the community for future mass scans.
We have started discussing this issue on the csmock[1] GitHub repository. Please leave any comments there. Thanks! > >> >> >> >> >> 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 >> :| >> [1] https://github.com/csutils/csmock/issues/177 -- _______________________________________________ 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
