Re: [PATCH v1] perf build: Revert "enable -fno-strict-aliasing"

[Ian Rogers <irogers@google.com>]

On Thu, Sep 4, 2025 at 1:59 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Sep 04, 2025 at 10:41:17PM +0200, Peter Zijlstra wrote:
> > On Thu, Sep 04, 2025 at 10:44:30AM -0700, Ian Rogers wrote:
> > > This reverts commit 55a18d2f3ff7 ("perf build: enable
> > > -fno-strict-aliasing"). With (get|put)_unaligned_* using memcpy
> > > -fno-strict-aliasing is no longer necessary as memcpys are assumed to
> > > possibly alias.
> >
> > I don't think this is a good idea. Much of tools/ includes kernel
> > headers and various kernel code, all of which is written in the
> > understanding that this (often called broken) C language feature does
> > not exist.
> >
> > As such, I would strongly suggest all of tools is built with
> > -fno-strict-aliasing.
>
> Similarly I would strongly suggest having -fwrapv on all code that
> includes kernel headers.

Given we can build and run with sanitizers, ubsan covers fwrapv and
type sanitizer is in development to detect strict aliasing violations.
So we can have correct code without hamstringing the compiler.

There's lots wrong with C, a particular favorite of mine is:
```
void foo(const int *p) {
   int x = *p;
   if (x == 10) {
      mutex_lock(&global_mutex);
      if (x == 10) {
          // Code should always run ...
```
The compiler can save registers by re-loading from p in the code
above, meaning the second "if" may not always run.

Anyway, the intent with adding -fno-strict-aliasing was that it was
temporary as part of removing the libcrypto dependency and when the
unaligned code landed we'd do this revert.

Wrt the kernel headers, I think we may be better breaking away from
them. I can see sense in using stdint types rather than __u32 or u32,
using stdatomic.h rather than linux/atomic.h, etc. This wasn't an
option 15 years ago, but things have progressed a little since then.

Thanks,
Ian

> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > > This patch needs:
> > > https://lore.kernel.org/lkml/20250722215754.672330-1-irogers@google.com/
> > > which have been merged into the tip timers/vdso branch.
> > > ---
> > >  tools/perf/Makefile.config | 4 ----
> > >  1 file changed, 4 deletions(-)
> > >
> > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > index 5a5832ee7b53..306b8334b788 100644
> > > --- a/tools/perf/Makefile.config
> > > +++ b/tools/perf/Makefile.config
> > > @@ -19,10 +19,6 @@ detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected)
> > >  CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
> > >  HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
> > >
> > > -# This is required because the kernel is built with this and some of the code
> > > -# borrowed from kernel headers depends on it, e.g. put_unaligned_*().
> > > -CFLAGS += -fno-strict-aliasing
> > > -
> > >  # Enabled Wthread-safety analysis for clang builds.
> > >  ifeq ($(CC_NO_CLANG), 0)
> > >    CFLAGS += -Wthread-safety
> > > --
> > > 2.51.0.355.g5224444f11-goog
> > >