lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z9tpTfHa4DquTzAT@google.com>
Date: Wed, 19 Mar 2025 18:03:09 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Leo Yan <leo.yan@....com>, James Clark <james.clark@...aro.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	"Liang, Kan" <kan.liang@...ux.intel.com>,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf build: Restore {0} initializer since GCC-15

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@....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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ