Re: [PATCH] perf build: Restore {0} initializer since GCC-15
From: Namhyung Kim
Date: Wed Mar 19 2025 - 21:03:18 EST
Hello,
On Wed, Mar 19, 2025 at 02:05:17PM -0700, Ian Rogers wrote:
> On Wed, Mar 19, 2025 at 11:28 AM Leo Yan <leo.yan@xxxxxxx> wrote:
> >
> > On Wed, Mar 19, 2025 at 08:26:10AM -0700, Ian Rogers wrote:
> >
> > [...]
> >
> > > > > Adding options that allow people to add more non standard code doesn't feel
> > > > > very portable or in the spirit of doing it the right way. Maybe there's an
> > > > > argument that it guards against future mistakes, but it's not mentioned in
> > > > > the commit message.
> > > >
> > > > I think Linux perf shares the same understanding with "we do expect
> > > > initializers that always initialize the whole variable fully" (quote
> > > > in [1]). Furthermore, the reply mentioned:
> > > >
> > > > The exact same problem happens with "{ 0 }" as happens with "{ }".
> > > > The bug is literally that some versions of clang seem to implement
> > > > BOTH of these as "initialize the first member of the union", which
> > > > then means that if the first member isn't the largest member, the
> > > > rest of the union is entirely undefined.
> > > >
> > > > So I think it is reasonable to imposes a compiler option to make
> > > > compiler's behavouir consistent.
> > >
> > > We have encountered this problem, here is a fix for a case:
> > > https://lore.kernel.org/r/20241119230033.115369-1-irogers@xxxxxxxxxx
> > > It would be nice if rather than -fzero-init-padding-bits=unions there
> > > were some kind of warning flag we could enable, or worse use a tool
> > > like clang-tidy to identify these problems. In the linked change the
> > > problem was identified with -fsanitize=undefined but IIRC perf didn't
> > > quit with a sanitizer warning message, just things broke and needed
> > > fixing.
> >
> > I searched the GCC online doc [2], I found below options but none of
> > them is used for such kind of warning:
> >
> > -Wmissing-braces
> > -Wuninitialized
> > -Wmissing-field-initializers
> >
> > For the "-Wmissing-field-initializers" option, it says "In C this
> > option does not warn about the universal zero initializer ‘{ 0 }’".
> >
> > Linux kernel has added the "-fzero-init-padding-bits=all" option in
> > the commit:
> >
> > dce4aab8441d kbuild: Use -fzero-init-padding-bits=all
> >
> > Maybe we can do the same thing for perf? This could help developers
> > and maintainers avoid endlessly struggling with potential bugs caused
> > by "{0}".
>
> It looks like clang may be just changing the default to zero
> initialize everything:
> https://github.com/llvm/llvm-project/pull/110051/files
> https://github.com/llvm/llvm-project/pull/97121
Then I think it's better to add "-fzero-init-padding-bits=all" for GCC
to minimize surprises.
Thanks,
Namhyung