[PATCH v2] perf build: Use -fzero-init-padding-bits=all

[PATCH v2] perf build: Use -fzero-init-padding-bits=all

[Leo Yan]

GCC-15 release claims [1]:

 {0} initializer in C or C++ for unions no longer guarantees clearing
 of the whole union (except for static storage duration initialization),
 it just initializes the first union member to zero. If initialization
 of the whole union including padding bits is desirable, use {} (valid
 in C23 or C++) or use -fzero-init-padding-bits=unions option to
 restore old GCC behavior.

This new behaviour might cause unexpected data when we define a union
with using the '{ 0 }' initializer.  Currently, the perf tool has ruled
out these cases with the sanitizer "-fsanitize=undefined".

But the sanitizer is not enabled by default, we need to manually enable
it with EXTRA_CFLAGS='-fsanitize=undefined'.  This means developers
might encounter issues caused by the initializer with new compilers.

Enable -fzero-init-padding-bits=all to zero padding bits in unions and
structures that might otherwise be left uninitialized.

[1] https://gcc.gnu.org/gcc-15/changes.html

Signed-off-by: Leo Yan <leo.yan@arm.com>
---

Changes from v1:
- Changed to use '-fzero-init-padding-bits=all' to replace
  '-fzero-init-padding-bits=unions'. (Namhyung)
- Updated commit log for a bit background info. (Ian)

 tools/perf/Makefile.config | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index a148ca9efca9..b4f6d656c729 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -323,6 +323,8 @@ FEATURE_CHECK_LDFLAGS-libaio = -lrt
 FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
 FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
 
+# Explicitly clear padding bits with the initializer '{ 0 }'
+CORE_CFLAGS += $(call cc-option,-fzero-init-padding-bits=all)
 CORE_CFLAGS += -fno-omit-frame-pointer
 CORE_CFLAGS += -Wall
 CORE_CFLAGS += -Wextra
-- 
2.34.1

Re: [PATCH v2] perf build: Use -fzero-init-padding-bits=all

[James Clark]

On 20/03/2025 10:52 am, Leo Yan wrote:
> GCC-15 release claims [1]:
> 
>   {0} initializer in C or C++ for unions no longer guarantees clearing
>   of the whole union (except for static storage duration initialization),
>   it just initializes the first union member to zero. If initialization
>   of the whole union including padding bits is desirable, use {} (valid
>   in C23 or C++) or use -fzero-init-padding-bits=unions option to
>   restore old GCC behavior.
> 
> This new behaviour might cause unexpected data when we define a union
> with using the '{ 0 }' initializer.  Currently, the perf tool has ruled
> out these cases with the sanitizer "-fsanitize=undefined".
> 
> But the sanitizer is not enabled by default, we need to manually enable
> it with EXTRA_CFLAGS='-fsanitize=undefined'.  This means developers
> might encounter issues caused by the initializer with new compilers.
> 
> Enable -fzero-init-padding-bits=all to zero padding bits in unions and
> structures that might otherwise be left uninitialized.
> 
> [1] https://gcc.gnu.org/gcc-15/changes.html
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
> 
> Changes from v1:
> - Changed to use '-fzero-init-padding-bits=all' to replace
>    '-fzero-init-padding-bits=unions'. (Namhyung)
> - Updated commit log for a bit background info. (Ian)
> 
>   tools/perf/Makefile.config | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index a148ca9efca9..b4f6d656c729 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -323,6 +323,8 @@ FEATURE_CHECK_LDFLAGS-libaio = -lrt
>   FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
>   FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
>   
> +# Explicitly clear padding bits with the initializer '{ 0 }'
> +CORE_CFLAGS += $(call cc-option,-fzero-init-padding-bits=all)
>   CORE_CFLAGS += -fno-omit-frame-pointer
>   CORE_CFLAGS += -Wall
>   CORE_CFLAGS += -Wextra

I don't think this makes its way to libperf. I don't have a compiler 
that supports it, but "-std=gnu11" is in CORE_CFLAGS and that's not 
there on any libperf compile commands so I'm assuming CORE_CFLAGS is 
only local to Perf.

Re: [PATCH v2] perf build: Use -fzero-init-padding-bits=all

[Leo Yan]

On Thu, Mar 20, 2025 at 11:46:03AM +0000, James Clark wrote:

[...]

> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -323,6 +323,8 @@ FEATURE_CHECK_LDFLAGS-libaio = -lrt
> >   FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
> >   FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
> > +# Explicitly clear padding bits with the initializer '{ 0 }'
> > +CORE_CFLAGS += $(call cc-option,-fzero-init-padding-bits=all)
> >   CORE_CFLAGS += -fno-omit-frame-pointer
> >   CORE_CFLAGS += -Wall
> >   CORE_CFLAGS += -Wextra
> 
> I don't think this makes its way to libperf. I don't have a compiler that
> supports it, but "-std=gnu11" is in CORE_CFLAGS and that's not there on any
> libperf compile commands so I'm assuming CORE_CFLAGS is only local to Perf.

Indeed.  Thanks for pointing out.

Since the commit 91009a3a9913 ("perf build: Install libperf locally when
building"), though the CORE_CFLAGS is appended to LIBPERF_CFLAGS, the
later is never populated to libperf.

I can add the compiler option into tools/lib/perf as well, and remove
the unused LIBPERF_CFLAGS variable.  This can allow the libperf to be
built independently from the perf tool.

To be honest, I am a bit concerned this is still insufficient as Perf
also links other libs (e.g. libbpf).  Another option is to place the
compiler option into tools/scripts/Makefile.include, something like:

diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index 0aa4005017c7..86d1a318a9f6 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -40,6 +40,18 @@ EXTRA_WARNINGS += -Wwrite-strings
 EXTRA_WARNINGS += -Wformat
 EXTRA_WARNINGS += -Wno-type-limits

+try-run = $(shell set -e;               \
+        if ($(1)) >/dev/null 2>&1;      \
+        then echo "$(2)";               \
+        else echo "$(3)";               \
+        fi)
+
+__cc-option = $(call try-run,\
+        $(1) -Werror $(2) -c -x c /dev/null -o /dev/null,$(2),)
+cc-option = $(call __cc-option, $(CC),$(1))
+
+CFLAGS += $(call cc-option,-fzero-init-padding-bits=all)
+

Thanks,
Leo