From: Eric Biggers <ebiggers@google.com>

perf pulls in code from kernel headers that assumes it is being built
with -fno-strict-aliasing, namely put_unaligned_*() from
<linux/unaligned.h> which write the data using packed structs that lack
the may_alias attribute.  Enable -fno-strict-aliasing to prevent
miscompilations in sha1.c which would otherwise occur due to this issue.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 tools/perf/Makefile.config | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index d1ea7bf449647..1691b47c4694c 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -17,10 +17,14 @@ detected     = $(shell echo "$(1)=y"       >> $(OUTPUT).config-detected)
 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
 endif
 
-- 
2.49.0

On Fri, Jun 13, 2025 at 9:43 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> perf pulls in code from kernel headers that assumes it is being built
> with -fno-strict-aliasing, namely put_unaligned_*() from
> <linux/unaligned.h> which write the data using packed structs that lack
> the may_alias attribute.  Enable -fno-strict-aliasing to prevent
> miscompilations in sha1.c which would otherwise occur due to this issue.

Wow, good catch! I wonder if -fsanitize=type could be used to capture
when perf's code is broken like this? Perhaps we should just remove
linux/unaligned.h in tools because of this, the alternative of using
memcpy doesn't look particularly burdensome. Given the memcpys are of
a known/fixed size I'd expect the compiler to be able to optimize
things just as well. Perhaps we should rewrite unaligned.h in tools
but perhaps the kernel too. Something like:

#define __get_unaligned_t(type, ptr) ({
                 \
        const struct { type x; } __packed * __get_pptr =
(typeof(__get_pptr))(ptr);     \
        __get_pptr->x;
                 \
})

becomes:

#define __get_unaligned_t(type, ptr) ({
                 \
        type __get_val; memcpy(&__get_val, ptr, sizeof(__get_val));     \
        __get_val;
             \
})

Thanks,
Ian

> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  tools/perf/Makefile.config | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index d1ea7bf449647..1691b47c4694c 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -17,10 +17,14 @@ detected     = $(shell echo "$(1)=y"       >> $(OUTPUT).config-detected)
>  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
>  endif
>
> --
> 2.49.0
>

On Sun, Jun 15, 2025 at 04:40:45PM -0700, Ian Rogers wrote:
> On Fri, Jun 13, 2025 at 9:43 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > From: Eric Biggers <ebiggers@google.com>
> >
> > perf pulls in code from kernel headers that assumes it is being built
> > with -fno-strict-aliasing, namely put_unaligned_*() from
> > <linux/unaligned.h> which write the data using packed structs that lack
> > the may_alias attribute.  Enable -fno-strict-aliasing to prevent
> > miscompilations in sha1.c which would otherwise occur due to this issue.
> 
> Wow, good catch! I wonder if -fsanitize=type could be used to capture
> when perf's code is broken like this? Perhaps we should just remove
> linux/unaligned.h in tools because of this, the alternative of using
> memcpy doesn't look particularly burdensome. Given the memcpys are of
> a known/fixed size I'd expect the compiler to be able to optimize
> things just as well. Perhaps we should rewrite unaligned.h in tools
> but perhaps the kernel too. Something like:
> 
> #define __get_unaligned_t(type, ptr) ({
>                  \
>         const struct { type x; } __packed * __get_pptr =
> (typeof(__get_pptr))(ptr);     \
>         __get_pptr->x;
>                  \
> })
> 
> becomes:
> 
> #define __get_unaligned_t(type, ptr) ({
>                  \
>         type __get_val; memcpy(&__get_val, ptr, sizeof(__get_val));     \
>         __get_val;
>              \
> })

As far as I know, the packed struct method of doing unaligned memory accesses is
obsolete these days, and memcpy() generates the desired code on all supported
architectures and compilers.

- Eric

On Sun, Jun 15, 2025 at 6:14 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> As far as I know, the packed struct method of doing unaligned memory accesses is
> obsolete these days, and memcpy() generates the desired code on all supported
> architectures and compilers.

Thanks Eric, I sent:
https://lore.kernel.org/lkml/20250617005800.1410112-1-irogers@google.com/
let's see how it goes.

Thanks,
Ian