On Wed, 6 Jan 2021 18:02:49 +0000
Ferruh Yigit <ferruh.yi...@intel.com> wrote:
> On 12/5/2020 5:42 AM, George Prekas wrote:
> > Strict-aliasing rules are violated by cast to uint16_t* in flowgen.c
> > and the calculated IP checksum is wrong on GCC 9 and GCC 10.
> >
> > Signed-off-by: George Prekas <preka...@amazon.com>
> > ---
> > v2:
> > * Instead of a compiler barrier, use a compiler flag.
> > ---
> > app/test-pmd/meson.build | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build
> > index 7e9c7bdd6..5d24e807f 100644
> > --- a/app/test-pmd/meson.build
> > +++ b/app/test-pmd/meson.build
> > @@ -4,6 +4,7 @@
> > # override default name to drop the hyphen
> > name = 'testpmd'
> > cflags += '-Wno-deprecated-declarations'
> > +cflags += '-fno-strict-aliasing'
> > sources = files('5tswap.c',
> > 'cmdline.c',
> > 'cmdline_flow.c',
> >
>
> Hi George,
>
> I am trying to understand this, the relevant code is as below:
> ip_hdr->hdr_checksum = ip_sum((unaligned_uint16_t *)ip_hdr, sizeof(*ip_hdr));
>
> You are suspicious of strict aliasing rule violation, with more details:
> The concern is the "struct rte_ipv4_hdr *ip_hdr;" aliased to "const
> unaligned_uint16_t *hdr", and compiler can optimize out the calculations
> using
> data pointed by 'hdr' pointer, since the 'hdr' pointer is not used to alter
> the
> data and compiler may think data is not changed at all.
>
> 1) But the pointer "hdr" is assigned in the loop, from another pointer whose
> content is changing, why this is not helping to figure out that the data
> 'hdr'
> pointing is changed.
>
> 2) I tried to debug this, but I am not able to reproduce the issue,
> 'ip_sum()'
> called each time and checksum calculated correctly. Using gcc 10.2.1-9. Can
> you
> able to confirm the case with debug, or from the assembly/object file?
>
>
> And if the issue is strict aliasing rule violation as you said, compiler flag
> is
> an option but not sure how much it reduces the compiler optimization benefit,
> I
> guess other options also not so good, memcpy brings too much work on runtime
> and
> union requires bigger change and makes code complex.
> I wonder if making 'ip_sum()' a non inline function can help, can you please
> give a try since you can reproduce it?
If it is an aliasing problem, it should be fixed with a union instead of a
compiler flag.
