Pekka Paalanen <ppaala...@gmail.com> writes: > [ Unknown signature status ] > On Fri, 1 Sep 2017 11:55:15 -0700 > Eric Anholt <e...@anholt.net> wrote: > >> --- >> >> Pekka - that link didn't help, because we still need a correct >> "result" value. I don't believe that the compiler could break uint -> >> int conversions with the high bit, but here's the patch I think we >> would need for that. I still think v1 is the better version. > > Hi, > > sorry, but I'm confused. What is the correct "result" value in case of > an overflow?
The 2s complement addition/subtraction result. >> include/misc.h | 21 +++++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/include/misc.h b/include/misc.h >> index 0feeaebc7c1a..fc1a55dac343 100644 >> --- a/include/misc.h >> +++ b/include/misc.h >> @@ -327,13 +327,21 @@ bswap_32(uint32_t x) >> static inline Bool >> checked_int64_add(int64_t *out, int64_t a, int64_t b) >> { >> - int64_t result = a + b; >> + /* Note that overflow behavior with signed ints in C is undefined, >> + * and the compiler might optimize our check away if we do so. In >> + * the discussion about it, people raised the concern that even >> + * casting from uint to int would be undefined, so we stick with >> + * all of our math in uint and memcpy the result, out of extreme >> + * paranoia. >> + */ >> + uint64_t result = (uint64_t)a + (uint64_t)b; >> /* signed addition overflows if operands have the same sign, and >> * the sign of the result doesn't match the sign of the inputs. >> */ >> - Bool overflow = (a < 0) == (b < 0) && (a < 0) != (result < 0); >> + Bool result_negative = (result & (1ull << 63)) != 0; >> + Bool overflow = (a < 0) == (b < 0) && (a < 0) != result_negative; >> >> - *out = result; >> + memcpy(out, &result, sizeof(result)); > > You might hate the memcpy() and so do I, but better ideas seem scarce. > > One might be a union { int64_t; uint64_t; } for the "casting". > > Another would be to write the code any way you please, but add a test > that ensures the possibly-not-guaranteed behaviour you rely on is > actually there and correct. > > This is more of a learning experience for me as well, than already > knowing what's a good way. I already wrote the unit test, it's in patch 5 that we're replying to.
signature.asc
Description: PGP signature
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel