Re: Findings by static analyzers in Fedora 42 Critical Path Packages

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

 



Just to add some positive news after my previous (and somewhat negative emails) - I rebuilt polkit with GCC analyzer and LTO which yielded four warnings and all of them were real issues (well, the last one was a potential one, but I understand why GCC analyzer reported it). So it seems to work fine on a smaller scale.

On 11/20/24 09:34, František Šumšal wrote:


On 11/19/24 18:07, David Malcolm wrote:
On Tue, 2024-11-19 at 17:25 +0100, František Šumšal wrote:
On 11/19/24 10:22, František Šumšal wrote:
On 11/19/24 09:07, Kamil Dudka wrote:
On Monday, November 18, 2024 6:57:03 PM GMT+1 František Šumšal
wrote:
Right after I sent this I got a response [0] to the gcc bug and
turns out that the culprit is disabled LTO. And indeed, if I
build systemd with `-fanalyzer -flto=auto`, (almost) all
reports disappear:

With just -fanalyzer (meson setup build -Dc_args="-fanalyzer"):
$ grep warning: log-fanalyzer.txt | wc -l
1519

With -fanalyzer -lto=auto (meson setup build -Dc_args="-
fanalyzer -lto=auto"):
$ grep warning: log-fanalyzer-lto.txt | wc -l
1

The LTO is being disabled in the csgcca wrapper since [1]
because of some issues between -fanalyzer and LTO, but looks
like disabling it has some disadvantages as well (not sure if
they outweigh the advantages, I tested this only on systemd so
far).

[0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117662#c2
[1]
https://github.com/csutils/cscppc/commit/d8d49b4cc92e0109b2654e9034c85e169bdc0566

Thanks for the analysis!  Unfortunately, it does not mean that
systemd is
free of gcc analyzer findings.  It rather means that -flto=auto
prevented
gcc analyzer from working.  I have reported this upstream:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117667

... and got an immediate reply.  If you want gcc analyzer to work
with LTO,
you need to pass -fanalyzer to the linker, too (in meson via -
Dc_link_args).
The only problem is that it does not work because it runs forever
and eats
all the memory.  So it is much better to use csgcca after all.

Ah, thank you, I knew it was too good to be true :)

I'll play around with the LTO stuff a bit then to see if it
generates some useful results, since currently the results from gcc
analyzer are far from ideal/usable, due to the number of false
positives.

Oh well, you were absolutely correct. I tested this on a somewhat
beefy machine (x86_64, 64 CPUs, 256 GB of RAM) in hopes I could get
at least one build done, but gcc kept getting killed by OOM killer
after a while. Eventually I managed to get it run longer by limiting
the build parallelization to a single job, where the single gcc
process got satisfied with mere 221 GB of RAM, but even after 3 hours
the build was nowhere near completion [0] (but it wasn't stuck, at
least according to ltrace). So this really doesn't seem to be a
viable solution for OSH.

This, however, leads me to the question of usefulness of the gcc
analyzer reports, since according to [0] gcc analyzer needs LTO to
generate reasonable results (or at least to significantly reduce the
number of false positives). And all the reports I've investigated so
far were false positives. I'm not sure if this is the case for other
packages, but for systemd these results are quite unusable. One
possible workaround would be to disable checks that generate most of
the false positives, but that's cumbersome and it could hide real
issues in the future.

I'm cc'ing David in case he's aware of any trick that could help
here.

Note that LTO with -fanalyzer isn't in good shape yet, sorry.  It works
on toy examples, and I have some basic test coverage for that in the
testsuite, but there are likely to be algorithmic explosions when
trying it on real projects.

So I can't recommend enabling LTO yet, sorry.

I *could* make a big push in GCC 16 to try to make it usable - though I
already have a full plate of work, so I'd probably have to drop
something else.

In the meantime, the best action is to pick a minimal subset of -
fanalyzer GCC warnings that don't generate too many false positives;
sorry that this isn't ideal.

No worries, I was just curious if there's something obvious I'm missing to make things behave a bit better. I'll continue to play with gcc analyzer on other (smaller) packages, where it should fare much better, and try it on systemd again sometime in the future.

Thanks!

Dave


Cheers,
Frantisek

[0] https://mrc0mmand.fedorapeople.org/gcc-fanalyzer-lto.png
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117662


Cheers,
Frantisek


Kamil

On 11/18/24 18:25, František Šumšal wrote:
Hey,

Thank you for doing this!

I started taking apart systemd findings and reported a first
issue against gcc [0], so we can hopefully squash the false
positives from the results (at least the ones repored by
gcc's -fanalyzer) and make them more useful.

One thing that comes to mind (especially after the battle in
[1]): how does the gcc's -fanalyzer handle optimized builds?
In Fedora we currently (IIRC) build with -O2, and if I do a
systemd build locally with both -O0 and -O2, the latter one
generates more warnings. There's also several notes in the
official documentation [2] that some of the checks might not
work when optimizations are enabled, since the analyzer runs
quite late in the compilation process. Which also raises
question - does optimization make -fanalyzer more prone to
false positives? And does it make the findings less accurate?

Cheers,
Frantisek

[0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117662
[1] https://github.com/csutils/cscppc/issues/46
[2]
https://gcc.gnu.org/onlinedocs/gcc/Static-Analyzer-Options.html


On 11/14/24 08:47, Siteshwar Vashisht wrote:
Hello,

I am writing this message to get feedback from the
community on new
findings by static analyzers in Critical Path Packages that
have
changed in Fedora 42.

TLDR: This report[1] contains 37330 findings. Please review
the report
and provide feedback.

A mass scan was performed this week on the packages that
have changed
in Fedora 42. This report[1] contains all the new findings
that have
been identified in the packages listed in Critical Path
Packages.
Newly added findings since Fedora 41 are listed under ‘+’
column.
Please review the report and fix or report any findings
upstream that
may be real bugs. Not all findings reported by OpenScanHub
may be
actual bugs, so please verify reported findings 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[2]. If you want to
see the full
logs of the scans, they are available on the tasks[3] page.
User
documentation for performing a scan is available on the
Fedora
wiki[4].

Constructive feedback is appreciated. Thank you!

[1]
https://svashisht.fedorapeople.org/openscanhub/mass-scans/f42-13-Nov-2024/
[2]
https://lists.fedoraproject.org/archives/list/openscanhub@xxxxxxxxxxxxxxxxxxxxxxx/
[3] https://openscanhub.fedoraproject.org/task/
[4] https://fedoraproject.org/wiki/OpenScanHub











--
Frantisek Sumsal
GPG key ID: 0xFB738CE27B634E4B
--
_______________________________________________
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