Messages in this thread | | | Date | Tue, 14 Feb 2023 15:04:35 +0000 | From | Yazen Ghannam <> | Subject | Re: [PATCH] EDAC/amd64: Shut up an -Werror,-Wsometimes-uninitialized clang false positive |
| |
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?
Thanks, Yazen
| |