Messages in this thread | | | Date | Tue, 14 Feb 2023 09:27:10 -0700 | From | Nathan Chancellor <> | Subject | Re: [PATCH] EDAC/amd64: Shut up an -Werror,-Wsometimes-uninitialized clang false positive |
| |
On Tue, Feb 14, 2023 at 03:04:35PM +0000, Yazen Ghannam wrote: > On Tue, Feb 14, 2023 at 07:32:36AM -0700, Nathan Chancellor wrote: > > On Tue, Feb 14, 2023 at 10:55:51AM +0100, Borislav Petkov wrote: > > > From: Yazen Ghannam <yazen.ghannam@amd.com> > > > > > > Yeah, the code's fine even without this. > > > > > > What this is fixing is a compiler which is overeager to report false > > > positives which then get automatically enabled in -Wall builds and when > > > CONFIG_WERROR is set in allmodconfig builds, the build fails. > > > > > > It doesn't happen with gcc. > > > > > > Maybe clang should be more conservative when enabling such warnings > > > under -Wall as, apparently, this has an impact beyond just noisy output. > > > > For the record, this is the first false positive that I have seen from > > this warning in quite some time. You can flip through our issue tracker > > and see how many instances of the uninitialized warnings there have been > > and the vast majority of the ones in 2022 at least are all true > > positives: > > > > https://github.com/ClangBuiltLinux/linux/issues?q=label%3A-Wsometimes-uninitialized%2C-Wuninitialized > > > > So I disagree with the characterization that clang is "overeager to > > report false positives" and I think the opinionated parts of the commit > > message could be replaced with some of the technical analysis that Tom > > and I did to show why this is a false positive but not one clang can > > reason about with the way the code is structured (since the warning does > > not perform interprocedural analysis). However, not my circus, not my > > monkeys, so feel free to ignore all this :) > > > > Regardless, my review still stands and thank you again for the fix. > > > > Thanks Nathan for the feedback and thanks Boris for the patch. > > Nathan, > I see there's a ClangBuiltLinux/continuous-integration2 project on github. > Is this something developers should try to leverage? Maybe just fork it and > update the action/workflows to use test branches?
Our continuous integration relies on TuxSuite [1], which in turn requires access to their service. TuxMake [2] is the backend for TuxSuite, which is what I use doing a lot of my build testing. It can use your local toolchains or it can use Docker/Podman to build in their curated containers, which have a wide variety of versions, if that matters to you.
I have thought about writing a wrapper around tuxmake to build our TuxSuite configurations (the tuxsuite/ folder within our repo) locally, maybe this is time to do so :) it would be useful to have something like
$ scripts/build-local.py tuxsuite/tip-clang-15.yml tuxsuite/tip-clang-16.yml
which would allow people to easily test the configurations that we generally care about for -tip with recent/stable versions of clang/LLVM. Otherwise, a simple
$ tuxmake -a x86_64 -k allmodconfig -t llvm default
or
$ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 allmodconfig all
is generally good enough to catch the majority of problems visible with clang, assuming your distribution has a version of LLVM that the kernel supports (11.x+).
[1]: https://tuxsuite.com [2]: https://tuxmake.org
Cheers, Nathan
| |