Re: [PATCH v5] eal: fix unaligned loads/stores in rte_memcpy_generic

[Luc Pelletier <lucp.at.work@gmail.com>]

Hi Konstantin,

> > Thanks for your input Konstantin. Much appreciated. Just to make sure
> > I understand, can you please confirm that we do not want to fix the
> > fact that unaligned access in C is undefined behaviour?
>
> Yes, I don't think it is a real problem in that particular case.

Perfect. Thank you.

> > Also, do you have any opinion on the strict aliasing violation (as was
> > pointed out by Georg)? I suggested adding __may_alias__ thinking it
> > would likely fix the issue, but I'd really like to get confirmation
> > from someone who has better knowledge of how the attribute works
> > exactly.
>
> Not sure I understand the problem you are referring to.
> Are you saying that original rte_memcpy() code breaks strict aliasing?
> If so, could you point where exactly?

As far as I understand, yes, it does break strict aliasing. For
example, in the following line:

*(uint64_t *)dstu = *(const uint64_t *)srcu;

IIUC, both casts break strict aliasing rules. While the src/dst
parameters are void* and can therefore be cast to something else
without breaking strict aliasing rules, the type of src/dst in the
calling code might be something other than uint64_t*. This can result
in src/dst pointers being cast to different unrelated types. AFAICT,
the fact that rte_memcpy is "always inline" increases the risk of the
compiler making an optimization that results in broken code.

I was able to come up with an example where the latest version of GCC
produces broken code when strict aliasing is enabled:

https://godbolt.org/z/3Yzvjr97c

With -fstrict-aliasing, it reorders a write and results in broken
code. If you change the compiler flags to -fno-strict-aliasing, it
produces the expected result.