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

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

[Ian Rogers]

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.

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

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

[Peter Zijlstra]

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.

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

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

[Peter Zijlstra]

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.

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

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

[Ian Rogers]

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

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

[Ian Rogers]

On Thu, Sep 4, 2025 at 2:33 PM Ian Rogers <irogers@google.com> wrote:
> 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.

Ping.

Thanks,
Ian

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

[Namhyung Kim]

On Thu, Mar 12, 2026 at 01:58:31PM -0700, Ian Rogers wrote:
> On Thu, Sep 4, 2025 at 2:33 PM Ian Rogers <irogers@google.com> wrote:
> > 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.

I don't remember what's going on here but as it's not there originally,
I think we can revert to the change for now.

But it doesn't apply, care to rebase?

Thanks,
Namhyung

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

[Peter Zijlstra]

On Thu, Sep 04, 2025 at 02:33:24PM -0700, Ian Rogers wrote:
> 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.

mutex_lock() should imply barrier() which should very much inhibit this.

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

[Ian Rogers]

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.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Makefile.config | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index a8dc72cfe48e..1d3dcefd8c95 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
-
 # Set target flag and options when using clang as compiler.
 ifeq ($(CC_NO_CLANG), 0)
   CLANG_TARGET_FLAGS_arm	:= arm-linux-gnueabi
-- 
2.53.0.959.g497ff81fa9-goog

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

[Ian Rogers]

On Fri, Mar 20, 2026 at 3:42 PM Ian Rogers <irogers@google.com> 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.

Sashiko noted strict aliasing violations in hashmap, so I sent out a fix [1]:

The commit message notes that put_unaligned_*() no longer requires this
flag, but does this change safely account for the hashmap API's type punning?

The hashmap API heavily uses type punning via hashmap_cast_ptr(), casting
pointer-to-pointer arguments (e.g., struct syscall_stats **stats or
struct hashmap_entry **entry) into long * for functions like hashmap_find().

Inside hashmap_find(), the pointer is written via *value = entry->value;
where value is of type long *. Under C strict aliasing rules, a long and a
pointer type are incompatible. When -fstrict-aliasing is active (the default
at -O2 or -O3), the compiler is allowed to assume that the write through
long * does not modify the caller's pointer variable.

If perf is compiled with Link-Time Optimization (LTO), the compiler can
analyze the cross-module call and conclude that hashmap_find() cannot
modify the caller's pointer variable.

Could this cause the compiler to optimize out the assignment, leading to
silent NULL pointer dereferences or the use of uninitialized memory?

[1] https://lore.kernel.org/lkml/20260321024446.692008-1-irogers@google.com/T/#t

Thanks,
Ian

> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/Makefile.config | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index a8dc72cfe48e..1d3dcefd8c95 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
> -
>  # Set target flag and options when using clang as compiler.
>  ifeq ($(CC_NO_CLANG), 0)
>    CLANG_TARGET_FLAGS_arm       := arm-linux-gnueabi
> --
> 2.53.0.959.g497ff81fa9-goog
>