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] [day] [month] [year] [list]

Message-ID: <20250531192226.6af9bbde@pumpkin>
Date: Sat, 31 May 2025 19:22:26 +0100
From: David Laight <david.laight.linux@...il.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, 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 Fri, 30 May 2025 15:33:33 -0700
Namhyung Kim <namhyung@...nel.org> wrote:

...
> > > No, I meant we may not need the __builtin_unreachable() at the callsites.
> > >
> > > Would it complain this code?
> > >
> > >   if (some_bad_option_use)
> > >     usage_with_options(...);
> > >
> > >   /* normal code path */  
> > 
> > Right that would fix -Wunreachable, but the existing code would be:
> > 
> >   if (some_bad_option_use) {
> >     usage_with_options(...);
> >     exit(..);
> >   }
> >   /* normal code path */
> > 
> > Letting you know that "normal code path" couldn't be fallen into after
> > usage_with_options(...). To make the behavior more obvious we could
> > rename usage_with_options():
> > 
> >   if (some_bad_option_use)
> >     usage_with_options_and_exit(...);
> > 
> >   /* normal code path */
> > 
> > What I've done is:
> > 
> >   if (some_bad_option_use) {
> >     usage_with_options(...);
> >     __builtin_unreachable();
> >   }
> >   /* normal code path */

Isn't that just wrong?
Doesn't __builtin_unreachable() tell the compiler the code can't be executed.
In this case you want the opposite - something that the compiler will
whinge about if it finds a path that does execute the code.

In terms of source code readability it is a also horrid.
It is pretty common for an application to have a lot of 'usage' errors
and you want them to take as little vertical code space as possible.
That is the whole reason why it is common for a usage() function to
call exit() rather than return.

	David

> > 
> > My reasoning is that usage_with_options() doesn't obviously on the
> > face of it call exit and never return. To make that clear we could add
> > a comment:
> > 
> >   if (some_bad_option_use) {
> >     usage_with_options(...);
> >     /* usage_with_options never returns as it always calls exit */
> >   }
> >   /* normal code path */
> > 
> > But my preference is to use __builtin_unreachable as that is the same
> > as the comment but is also something the compiler can trap on were it
> > not true.  
> 
> I see, thanks for the explanation.  That part looks ok then.
> 
> Thanks,
> Namhyung
> 
>