The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Leo Yan <leo.yan@arm.com>, James Clark <james.clark@linaro.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	"Liang, Kan" <kan.liang@linux.intel.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf build: Restore {0} initializer since GCC-15
Date: Wed, 19 Mar 2025 18:03:09 -0700	[thread overview]
Message-ID: <Z9tpTfHa4DquTzAT@google.com> (raw)
In-Reply-To: <CAP-5=fVkCaX9DRokAEqDq-3Bvr8txM7oBmw63R+O2ibg2AzDuQ@mail.gmail.com>

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@arm.com> 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@google.com
> > > 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


  reply	other threads:[~2025-03-20  1:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-19 11:04 [PATCH] perf build: Restore {0} initializer since GCC-15 Leo Yan
2025-03-19 11:19 ` James Clark
2025-03-19 13:30   ` Leo Yan
2025-03-19 15:26     ` Ian Rogers
2025-03-19 18:28       ` Leo Yan
2025-03-19 21:05         ` Ian Rogers
2025-03-20  1:03           ` Namhyung Kim [this message]
2025-03-20 11:36   ` James Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z9tpTfHa4DquTzAT@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=leo.yan@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox