On 6 April 2014 08:09, Michael Tokarev <m...@tls.msk.ru> wrote: > 28.03.2014 19:12, Peter Maydell wrote: >> Add casts when we're performing arithmetic on the .hi parts of an >> Int128, to avoid undefined behaviour. > [] >> static inline Int128 int128_sub(Int128 a, Int128 b) >> { >> - return (Int128){ a.lo - b.lo, a.hi - b.hi - (a.lo < b.lo) }; >> + return (Int128){ a.lo - b.lo, (uint64_t)a.hi - b.hi - (a.lo < b.lo) }; > > What was wrong with this one? I don't think casting to unsigned here is > a good idea.
This patch is fixing these three clang sanitizer warnings: /home/petmay01/linaro/qemu-from-laptop/qemu/include/qemu/int128.h:81:40: runtime error: signed integer overflow: 0 - -9223372036854775808 cannot be represented in type 'long' /home/petmay01/linaro/qemu-from-laptop/qemu/include/qemu/int128.h:81:47: runtime error: signed integer overflow: -9223372036854775808 - 1 cannot be represented in type 'long' /home/petmay01/linaro/qemu-from-laptop/qemu/include/qemu/int128.h:56:47: runtime error: left shift of negative value -9223372036854775807 of which the first two are in this function. Note that int128_add() already has a cast. The alternative would be to say that Int128 should have undefined behaviour on underflow/overflow and the test code is wrong, but that doesn't seem very useful to me. thanks -- PMM