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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Feb 2023 15:04:35 +0000
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     Nathan Chancellor <nathan@...nel.org>
Cc:     Borislav Petkov <bp@...en8.de>, Tom Rix <trix@...hat.com>,
        tony.luck@...el.com, james.morse@....com, mchehab@...nel.org,
        rric@...nel.org, linux-edac@...r.kernel.org,
        linux-kernel@...r.kernel.org
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@....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