linux-kernel - Re: [RFC PATCH v1] perf build: Fix build for clang's -Wunreachable-code

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]

Message-ID: <CAP-5=fUycjUUWW=hoSSvxfUVPXcqAk5KHnknFuUDOr7+Zf=M2A@mail.gmail.com>
Date: Wed, 28 May 2025 11:35:00 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [RFC PATCH v1] perf build: Fix build for clang's -Wunreachable-code

On Wed, May 28, 2025 at 11:24 AM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Tue, May 27, 2025 at 01:53:37PM -0700, Ian Rogers wrote:
> > On Fri, Apr 11, 2025 at 3:14 PM Ian Rogers <irogers@...gle.com> wrote:
> > >
> > > On Fri, Apr 11, 2025 at 2:34 PM Namhyung Kim <namhyung@...nel.org> wrote:
> > > >
> > > > Hi Ian,
> > > >
> > > > On Thu, Apr 10, 2025 at 01:26:47PM -0700, Ian Rogers wrote:
> > > > > Clang's unreachable code warning is able to catch bugs like the famous
> > > > > "goto fail" which gcc's unreachable code warning fails to warn about
> > > > > (it will complain about misleading indent). The changes here are
> > > > > sufficient to get perf building with clang with -Wunreachable code,
> > > > > but they don't really fix any bugs. Posting as an RFC to see if anyone
> > > > > things this is worth pursuing.
> > > >
> > > > I'm not sure if it's useful and don't see what kind of bugs it can
> > > > address.  The proposed changes don't look like an improvement.
> > >
> > > The goto fail case was in OpenSSL the code from a bad merge:
> > > ```
> > > if (...)
> > >   goto fail;
> > >   goto fail;
> > > ```
> > > Meaning the fail path was always taken and checking on the non-fail
> > > code never executed. Newer GCCs will warn of this because of the
> > > "misleading indent" but  clang won't. It is easy to imagine similar
> > > mistakes creeping in, so using compiler warnings to avoid the bug
> > > could be useful.
>
> It doesn't look very convincing to me but it might be valuable in some
> rare cases.  But the proposed changes - basically replace exit() to
> __builtin_unreachable() - seem weird.  Why is calling it a problem?  I
> guess it already has some kind of annotation like "noreturn"?

Yep. The exit is incorrect (depending on your notion of correct, I'd
go with clang's notion as they've had to consider this for a while) as
it can never be executed. I've added the __builtin_unreachable() to
document that you can never get to that statement, as otherwise it can
make the code readability harder with the code looking like it will
fall through after calling something like usage_with_options (which is
noreturn). In unoptimized builds __builtin_unreachable() will fail if
executed, so it is a bit more active than just a comment.

Thanks,
Ian

> Thanks,
> Namhyung
>