[PATCH] build: force gcc to initialize padding bits

[PATCH] build: force gcc to initialize padding bits

[Stephen Hemminger]

With GCC 15, the compiler has changed the default behavior when
initialization is used for aggregate variables. The new default
is to follow the standard (C23) and not initialize everything by
default. This breaks assumptions in some drivers and can be
lead to other bugs.

Use the new zero initialization flag to force the old behavior
of initializing everything to zero.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 config/meson.build | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/config/meson.build b/config/meson.build
index 6aaad6d8a4..5c8b5a15f5 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -330,6 +330,9 @@ warning_flags = [
         # globally disabled warnings
         '-Wno-packed-not-aligned',
         '-Wno-missing-field-initializers',
+
+        # guarantee that everything is zero when using initialization
+        '-fzero-init-padding-bits=all',
 ]
 
 if not dpdk_conf.get('RTE_ARCH_64')
-- 
2.45.2

RE: [PATCH] build: force gcc to initialize padding bits

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, 23 January 2025 18.21
> 
> With GCC 15, the compiler has changed the default behavior when
> initialization is used for aggregate variables. The new default
> is to follow the standard (C23) and not initialize everything by
> default. This breaks assumptions in some drivers and can be
> lead to other bugs.
> 
> Use the new zero initialization flag to force the old behavior
> of initializing everything to zero.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  config/meson.build | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 6aaad6d8a4..5c8b5a15f5 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -330,6 +330,9 @@ warning_flags = [

Is warning_flags the right location for this?
Alternatively, should warning_flags be renamed?

>          # globally disabled warnings
>          '-Wno-packed-not-aligned',
>          '-Wno-missing-field-initializers',
> +
> +        # guarantee that everything is zero when using initialization

Maybe add ", like in the C23 standard" to the comment.

> +        '-fzero-init-padding-bits=all',
>  ]
> 
>  if not dpdk_conf.get('RTE_ARCH_64')
> --
> 2.45.2

I have read up on -fzero-init-padding-bits, and this is the correct solution.

With or without suggested changes:
Acked-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH] build: force gcc to initialize padding bits

[Bruce Richardson]

On Fri, Jan 24, 2025 at 08:48:45AM +0100, Morten Brørup wrote:
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Thursday, 23 January 2025 18.21
> > 
> > With GCC 15, the compiler has changed the default behavior when
> > initialization is used for aggregate variables. The new default
> > is to follow the standard (C23) and not initialize everything by
> > default. This breaks assumptions in some drivers and can be
> > lead to other bugs.
> > 
> > Use the new zero initialization flag to force the old behavior
> > of initializing everything to zero.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  config/meson.build | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/config/meson.build b/config/meson.build
> > index 6aaad6d8a4..5c8b5a15f5 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -330,6 +330,9 @@ warning_flags = [
> 
> Is warning_flags the right location for this?
> Alternatively, should warning_flags be renamed?
> 

+1 to renaming warning flags, to e.g. "global_cflags"?

> >          # globally disabled warnings
> >          '-Wno-packed-not-aligned',
> >          '-Wno-missing-field-initializers',
> > +
> > +        # guarantee that everything is zero when using initialization
> 
> Maybe add ", like in the C23 standard" to the comment.
> 
> > +        '-fzero-init-padding-bits=all',
> >  ]
> > 
> >  if not dpdk_conf.get('RTE_ARCH_64')
> > --
> > 2.45.2
> 
> I have read up on -fzero-init-padding-bits, and this is the correct solution.
> 

Does this flag give us additional guarantees of padding being
zero-initialized that were there before? From my reading of the gcc doc[1],
"..padding-bits=union" corresponds to the old behaviour, right?

This also means we will have different padding behaviour on clang and gcc,
since clang (at least v18 on my board) doesn't support this flag. Do we see
any issues with that?

/Bruce

[1] https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html
> With or without suggested changes:
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 

Re: [PATCH] build: force gcc to initialize padding bits

[Stephen Hemminger]

On Fri, 24 Jan 2025 09:38:20 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> >   
> 
> Does this flag give us additional guarantees of padding being
> zero-initialized that were there before? From my reading of the gcc doc[1],
> "..padding-bits=union" corresponds to the old behaviour, right?
> 
> This also means we will have different padding behaviour on clang and gcc,
> since clang (at least v18 on my board) doesn't support this flag. Do we see
> any issues with that?
> 
> /Bruce

I chose the setting based on some email with the Linux kernel hardening
project and their choice.

Clang decided to fix and just do the right thing.


https://github.com/llvm/llvm-project/commit/7a086e1b2dc05f54afae3591614feede727601fa

[PATCH v2] build: force gcc to initialize padding bits

[Stephen Hemminger]

With GCC 15, the compiler has changed the default behavior when
initialization is used for aggregate variables. The new default
is to follow the standard (C23) and not initialize everything by
default. This breaks assumptions in some drivers and can be
lead to other bugs. Use the new zero initialization flag to
force of all fields to zero.

Note: other compilers always initialize to zero in these cases.
Only GCC seems to have decided to start initializing less as
a silly attempt at optimization.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 config/meson.build | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 6aaad6d8a4..b6b3558e11 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -310,7 +310,7 @@ endif
 # enable extra warnings and disable any unwanted warnings
 # -Wall is added by default at warning level 1, and -Wextra
 # at warning level 2 (DPDK default)
-warning_flags = [
+global_cflags = [
         # additional warnings in alphabetical order
         '-Wcast-qual',
         '-Wdeprecated',
@@ -330,19 +330,22 @@ warning_flags = [
         # globally disabled warnings
         '-Wno-packed-not-aligned',
         '-Wno-missing-field-initializers',
+
+        # guarantee that all non-initialized parts of structure/union are zero
+        '-fzero-init-padding-bits=all',
 ]
 
 if not dpdk_conf.get('RTE_ARCH_64')
 # for 32-bit, don't warn about casting a 32-bit pointer to 64-bit int - it's fine!!
-    warning_flags += '-Wno-pointer-to-int-cast'
+    global_cflags += '-Wno-pointer-to-int-cast'
 endif
 if cc.get_id() == 'intel'
     warning_ids = [181, 188, 2203, 2279, 2557, 3179, 3656]
     foreach i:warning_ids
-        warning_flags += '-diag-disable=@0@'.format(i)
+        global_cflags += '-diag-disable=@0@'.format(i)
     endforeach
 endif
-foreach arg: warning_flags
+foreach arg: global_cflags
     if cc.has_argument(arg)
         add_project_arguments(arg, language: 'c')
     endif
-- 
2.45.2