patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
To: Luca Boccassi <bluca@debian.org>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
	"stable@dpdk.org" <stable@dpdk.org>,
	 "ktraynor@redhat.com" <ktraynor@redhat.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	 "Yigit, Ferruh" <ferruh.yigit@intel.com>
Subject: Re: [PATCH] cryptodev: fix stringop-overflow build failure with gcc 10
Date: Fri, 17 Dec 2021 18:50:01 +0100	[thread overview]
Message-ID: <CAATJJ0LwmKd12AWn0agdBzcO0OAqLwzbgqSm1k+PxdWP8dyAkQ@mail.gmail.com> (raw)
In-Reply-To: <dc0c773af33fab758c2e6f8bcc41d40b0772eb72.camel@debian.org>

On Fri, Dec 17, 2021 at 4:02 PM Luca Boccassi <bluca@debian.org> wrote:
>
> On Fri, 2021-12-17 at 12:09 +0000, Richardson, Bruce wrote:
> >
> > > -----Original Message-----
> > > From: Luca Boccassi <bluca@debian.org>
> > > Sent: Friday, December 17, 2021 12:01 PM
> > > To: christian.ehrhardt@canonical.com; stable@dpdk.org
> > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; ktraynor@redhat.com;
> > > Thomas Monjalon <thomas@monjalon.net>; Yigit, Ferruh
> > > <ferruh.yigit@intel.com>
> > > Subject: Re: [PATCH] cryptodev: fix stringop-overflow build failure with
> > > gcc 10
> > >
> > > On Fri, 2021-12-17 at 10:51 +0100, christian.ehrhardt@canonical.com
> > > wrote:
> > > > From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > > >
> > > > 19.11 LTS releases started to fail to build in some newer releases
> > > > due to newer gcc-10 compilers available. Those are mostly false
> > > > positives [1][2] and to some extend covered by the already present
> > > > backports of "4990929c60 eal/ppc: ignore GCC 10 stringop-overflow
> > > > warnings" and "df17bcb43b eal/x86: ignore gcc 10 stringop-overflow
> > > > warnings".
> > > >
> > > > But the combination of 19.11 and gcc 10.3.0 and LTO this can still
> > > > exposes the issue.
> > > >
> > > > [  549s] In function ‘_mm_storeu_si128’,
> > > > [  549s]     inlined from ‘rte_memcpy_generic’
> > > >   at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:508:2,
> > > > [  549s]     inlined from ‘rte_cryptodev_sym_session_set_user_data’
> > > >   at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:882:10:
> > > > [  549s] /usr/lib/gcc/x86_64-linux-gnu/10/include/emmintrin.h:727:8:
> > > >   error: writing 16 bytes into a region of size 0
> > > >   [-Werror=stringop-overflow=]
> > > > [  549s]   727 |   *__P = __B;
> > > > [  549s]       |        ^
> > > > [  549s] ../lib/librte_cryptodev/rte_cryptodev.c:
> > > >   In function ‘rte_cryptodev_sym_session_set_user_data’:
> > > > [  549s] ../lib/librte_cryptodev/rte_cryptodev.h:984:4: note:
> > > >   at offset 0 to object ‘sess_data’ with size 0 declared here
> > > > [  549s]   984 |  } sess_data[0];
> > > > [  549s]       |    ^
> > > >
> > > > The problem is that in this combination only at link time (LTO)
> > > > the compiler will (false-)detect the problematic size of the target
> > > > structure and break. To make things worse [3] fixed this problem
> > > > for the same issue at actual build time, but the pragma has no
> > > > effect at link time nor are any werror settings passed to the
> > > > linker.
> > > >
> > > > For affected compilers set -Wno-stringop-overflow globally for
> > > > the linker stage to avoid this breaking the build in any place.
> > > >
> > > > Buzilla ID: 908
> > > >
> > > > [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94335
> > > > [2]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92955
> > > > [3]: https://github.com/DPDK/dpdk/commit/b5b3ea803e47
> > > >
> > > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > > > ---
> > > >  config/meson.build | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/config/meson.build b/config/meson.build
> > > > index 4007b8f37c..25dec320ba 100644
> > > > --- a/config/meson.build
> > > > +++ b/config/meson.build
> > > > @@ -143,6 +143,14 @@ if numa_dep.found() and cc.has_header('numaif.h')
> > > >   dpdk_extra_ldflags += '-lnuma'
> > > >  endif
> > > >
> > > >
> > > > +# Some false positives can trigger at LTO stage, so this warning needs
> > > to be
> > > > +# disabled on the linker as well for gcc 10.
> > > > +# Fixed in gcc-11 and not present <gcc-10
> > > > +# https://bugs.dpdk.org/show_bug.cgi?id=908
> > > > +if cc.get_id() == 'gcc' and cc.version().version_compare('>=10.0') and
> > > cc.version().version_compare('<11.0')
> > > > + add_project_link_arguments('-Wno-stringop-overflow', language: 'c')
> > > > +endif
> > > > +
> > > >  has_libfdt = 0
> > > >  fdt_dep = cc.find_library('libfdt', required: false)
> > > >  if fdt_dep.found() and cc.has_header('fdt.h')
> > >
> > > Sounds reasonable to me. Is it possible to detect that LTO is used, to
> > > further restrict when this is applied?
> > >

I haven't found one as it could be enabled as default built into the
compiler even when not present in cflags.
And parsing cflags would already be brittle.
Therefore I'm in favor of just the version check as I submitted it.

> > Does it really matter that much? This is for backport only, so won't apply for main tree, meaning that all backported patches will have been already checked with the setting enabled on the mainline. Therefore, I would look to not bother trying to limit the impact of the flag much, and just enable it in the most convenient manner.
> >
> > /Bruce
>
> Just thinking about coverage for future backports, but it's not too
> important
>
> --
> Kind regards,
> Luca Boccassi



-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

      reply	other threads:[~2021-12-17 17:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-17  9:51 christian.ehrhardt
2021-12-17 12:00 ` Luca Boccassi
2021-12-17 12:09   ` Richardson, Bruce
2021-12-17 15:02     ` Luca Boccassi
2021-12-17 17:50       ` Christian Ehrhardt [this message]

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=CAATJJ0LwmKd12AWn0agdBzcO0OAqLwzbgqSm1k+PxdWP8dyAkQ@mail.gmail.com \
    --to=christian.ehrhardt@canonical.com \
    --cc=bluca@debian.org \
    --cc=bruce.richardson@intel.com \
    --cc=ferruh.yigit@intel.com \
    --cc=ktraynor@redhat.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /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;
as well as URLs for NNTP newsgroup(s).