Re: [PATCH] EDAC/amd64: Shut up an -Werror,-Wsometimes-uninitialized clang false positive

From: Yazen Ghannam
Date: Tue Feb 14 2023 - 10:04:57 EST


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@xxxxxxx>
> >
> > 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