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

Leo Yan posted 1 patch 8 months, 3 weeks ago
There is a newer version of this series
tools/scripts/Makefile.include | 13 +++++++++++++
1 file changed, 13 insertions(+)
[PATCH] tools build: Use -fzero-init-padding-bits=all
Posted by Leo Yan 8 months, 3 weeks ago
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.

As a result, this new behaviour might cause unexpected data when we
initialize a union with using the '{ 0 }' initializer.

Since commit dce4aab8441d ("kbuild: Use -fzero-init-padding-bits=all"),
the kernel has enabled -fzero-init-padding-bits=all to zero padding bits
in unions and structures.  This commit applies the same option for tools
building.

The option is not supported neither by any version older than GCC 15 and
is also not supported by LLVM, this patch adds the cc-option function to
dynamically detect the compiler option.

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

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

This patch was originally from [1]. After consideration, the top level
of the tools directory is a better place to accommodate this option
rather than perf folder.

[1] https://lore.kernel.org/linux-perf-users/20250320105235.3498106-1-leo.yan@arm.com/

 tools/scripts/Makefile.include | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index 0aa4005017c7..e912a10afd89 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -40,6 +40,19 @@ 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))
+
+# Explicitly clear padding bits with the initializer '{ 0 }'
+CFLAGS += $(call cc-option,-fzero-init-padding-bits=all)
+
 # Makefiles suck: This macro sets a default value of $(2) for the
 # variable named by $(1), unless the variable has been set by
 # environment or command line. This is necessary for CC and AR
-- 
2.34.1
Re: [PATCH] tools build: Use -fzero-init-padding-bits=all
Posted by Ian Rogers 8 months, 3 weeks ago
On Fri, Mar 28, 2025 at 6:52 AM Leo Yan <leo.yan@arm.com> 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.
>
> As a result, this new behaviour might cause unexpected data when we
> initialize a union with using the '{ 0 }' initializer.
>
> Since commit dce4aab8441d ("kbuild: Use -fzero-init-padding-bits=all"),
> the kernel has enabled -fzero-init-padding-bits=all to zero padding bits
> in unions and structures.  This commit applies the same option for tools
> building.
>
> The option is not supported neither by any version older than GCC 15 and
> is also not supported by LLVM, this patch adds the cc-option function to
> dynamically detect the compiler option.
>
> [1] https://gcc.gnu.org/gcc-15/changes.html
>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>
> This patch was originally from [1]. After consideration, the top level
> of the tools directory is a better place to accommodate this option
> rather than perf folder.
>
> [1] https://lore.kernel.org/linux-perf-users/20250320105235.3498106-1-leo.yan@arm.com/
>
>  tools/scripts/Makefile.include | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
> index 0aa4005017c7..e912a10afd89 100644
> --- a/tools/scripts/Makefile.include
> +++ b/tools/scripts/Makefile.include
> @@ -40,6 +40,19 @@ EXTRA_WARNINGS += -Wwrite-strings
>  EXTRA_WARNINGS += -Wformat
>  EXTRA_WARNINGS += -Wno-type-limits
>

It'd be nice to bring in the comment for try-run that's in
./scripts/Makefile.compiler:
```
# try-run
# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
# Exit code chooses option. "$$TMP" serves as a temporary file and is
# automatically cleaned up.
```

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

I see differences with the ./scripts/Makefile.compiler version of
these functions:
```
# __cc-option
# Usage: MY_CFLAGS += $(call
__cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
__cc-option = $(call try-run,\
       $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))

cc-option = $(call __cc-option, $(CC),\
       $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS),$(1),$(2))
```
I'm just wondering why as if we need to update these in the future
it'd be easier if the two were identical.

Thanks,
Ian

> +# Explicitly clear padding bits with the initializer '{ 0 }'
> +CFLAGS += $(call cc-option,-fzero-init-padding-bits=all)
> +
>  # Makefiles suck: This macro sets a default value of $(2) for the
>  # variable named by $(1), unless the variable has been set by
>  # environment or command line. This is necessary for CC and AR
> --
> 2.34.1
>
Re: [PATCH] tools build: Use -fzero-init-padding-bits=all
Posted by Leo Yan 8 months, 3 weeks ago
Hi Ian,

On Fri, Mar 28, 2025 at 09:39:41AM -0700, Ian Rogers wrote:

[...]

> It'd be nice to bring in the comment for try-run that's in
> ./scripts/Makefile.compiler:
> ```
> # try-run
> # Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> # Exit code chooses option. "$$TMP" serves as a temporary file and is
> # automatically cleaned up.
> ```

It is fine for me to reuse the comment from Makefile.compiler and add
a temporary folder for the try result.

> > +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))
> > +
> 
> I see differences with the ./scripts/Makefile.compiler version of
> these functions:
> ```
> # __cc-option
> # Usage: MY_CFLAGS += $(call
> __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
> __cc-option = $(call try-run,\
>        $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
> 
> cc-option = $(call __cc-option, $(CC),\
>        $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS),$(1),$(2))
> ```
> I'm just wondering why as if we need to update these in the future
> it'd be easier if the two were identical.

Note, I do not see a requirement for passing two options for tools
building.  In the next spin, I will keep to support only one option.

Thanks for review!

Leo