Re: [Qemu-devel] [PATCH 1/1] tci: eliminate UB due to unaligned reads

[Anatoly Trosinenko]

Ping.
Patchwork link: http://patchwork.ozlabs.org/patch/866732/
Patchew link: http://patchew.org/QEMU/20180127134908.24095-1-anatoly.
address@hidden/

The code in tcg/tci.c reads some data from TCI bytecode through
pointer dereferencing. As far as I know unaligned reads in such a way are
undefined behavior and compiling with -fsanitize=undefined enumerated
them as such at run-time.

I have replaced such reads with invocations of ld{l,q}_he_p.
A comment in include/qemu/bswap.h:310 suggests they should be properly
translated by the compiler. I didn't added signed/unsigned casts
since bswap.h does contain separate signed/unsigned versions
for 16-bit integers but does not for 32- and 64-bit ones, so I supposed
the developers of the bswap.h already arranged everything so
integer promotions don't mess things up. I can add casts in case I'm
not right about it.
http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg06516.html

2018-01-28 9:42 GMT+03:00 Anatoly Trosinenko <address@hidden>:

> My patch is kind of trivial quick fix that just eliminates these unaligned
> reads and doesn't seem to require complicated testing supposing my code
> properly handles integer promotion (and hope it will not slow the
> interpreter down).
>
> Aligning everything, on the other hand, can not only remove the UB but
> also speed things up, but if I get it right, requires O(opcode count)
> manual work and subsequent less trivial testing that every opcode's
> argument layout match on generation and interpretation side (errors should
> be significantly localized due to present assertion on operation size,
> though). So my patch may be considered as temporary solution.
>
> In fact, I had to similarly make these unaligned reads explicit when
> porting QEMU to JavaScript because Emscripten hugely relies on absence of
> some kinds of UB such as "implicit" unaligned accesses, and such fix seemed
> to resolve this issue for me on "host with special alignment requirement".
>
>
> 2018-01-27 19:38 GMT+03:00 Stefan Weil <address@hidden>:
>
>> Am 27.01.2018 um 14:49 schrieb Anatoly Trosinenko:
>> > Use ldl_he_p / ldq_he_p functions instead of a plain memory access
>> > through pointer.
>> >
>> > Signed-off-by: Anatoly Trosinenko <address@hidden>
>> > ---
>> >  tcg/tci.c | 16 +++++++++++-----
>> >  1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> A better alternative might be aligning the relevant data when generating
>> the bytecode. See also my comment on alignment in tcg/tci/README.
>>
>> Stefan
>>
>>
>
>
> --
> С уважением,
> Анатолий Тросиненко
> e-mail: address@hidden
>



-- 
С уважением,
Анатолий Тросиненко
e-mail: address@hidden