Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-05 Thread marco . bodrato
Ciao, 4 set 2023, 15:19 da vinc...@vinc17.net: > On 2023-09-04 09:52:23 +0200, marco.bodr...@tutanota.com wrote: > >> Should the value ~0 be written as ~0U to be correctly assigned to an >> unsigned? >> > > IMHO, this would be better as this would make the signedness of > the types more

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-04 Thread Vincent Lefevre
On 2023-09-04 09:52:23 +0200, marco.bodr...@tutanota.com wrote: > Should the value ~0 be written as ~0U to be correctly assigned to an > unsigned? IMHO, this would be better as this would make the signedness of the types more consistent (the goal being to have an unsigned int for all these

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-04 Thread Niels Möller
marco.bodr...@tutanota.com writes: > Would a comment "It returns 0 or ~0, depending on the sign of the result" > added to all the mpn_toom_eval_ functions suffice? Yes. It would be nice if there were some reasonable place to document that only once, but don't know where that could be. > From

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-04 Thread marco . bodrato
Ciao, 4 set 2023, 08:02 da ni...@lysator.liu.se: > As Vincent suggested, it would be good to document somewhere what the > convention is (values 0 or ~0). > Would a comment "It returns 0 or ~0, depending on the sign of the result" added to all the mpn_toom_eval_ functions suffice? > And maybe

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-04 Thread Andrew Teylu
On Sun, Sep 3, 2023 at 11:40 PM wrote: > I attach a possible patch. > As an outsider, does it make sense to also change instances of "neg" to be "sign" (if that's the interpretation) for consistency? For someone who isn't literate in the GMP code, neg vs. sign seems to be used slightly

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-04 Thread Niels Möller
marco.bodr...@tutanota.com writes: > Yes, unsigned is the best choice, it used to store a positive or > negative number, but now it actually is a mask: 0 or ~0. > > I attach a possible patch. Makes sense, I think. As Vincent suggested, it would be good to document somewhere what the convention

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-03 Thread marco . bodrato
Ciao, 3 set 2023, 22:16 da ni...@lysator.liu.se: > Andrew Teylu writes: > > I see no good reason to involve any signed values here, though. Maybe > the variable neg, and the return value, should be changed to mp_limb_t, > or at least unsigned int? > Yes, unsigned is the best choice, it used

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-03 Thread Vincent Lefevre
On 2023-09-03 22:16:21 +0200, Niels Möller wrote: > Andrew Teylu writes: > > >> I am not sure the arithmetic on unsigned types is what clang is unhappy > >> about, though. Perhaps it dislikes the xor with "neg", which is a > >> signed variable. > > I can't say precisely what implicit

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-03 Thread Andrew Teylu
On Sun, Sep 3, 2023 at 9:16 PM Niels Möller wrote: > Does it make any difference if you change the "1" constants to "1u" ? > I'll try this in the morning and see if the runtime error goes away or not. > I see no good reason to involve any signed values here, though. Maybe > the variable neg,

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-03 Thread Niels Möller
Andrew Teylu writes: >> I am not sure the arithmetic on unsigned types is what clang is unhappy >> about, though. Perhaps it dislikes the xor with "neg", which is a >> signed variable. I can't say precisely what implicit conversions happen according to the spec: Unsigned to signed is always

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-03 Thread Andrew Teylu
On Sun, Sep 3, 2023 at 7:16 PM Torbjörn Granlund wrote: > > Andrew Teylu writes: > > When I run `multiply.c` from gmpbench [https://gmplib.org/gmpbench], > I'm seeing an unsigned integer overflow in `toom_eval_pm2.c` on this > line: > > neg ^= ((k & 1) -

Re: Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-03 Thread Torbjörn Granlund
Andrew Teylu writes: When I run `multiply.c` from gmpbench [https://gmplib.org/gmpbench], I'm seeing an unsigned integer overflow in `toom_eval_pm2.c` on this line: neg ^= ((k & 1) - 1) I fully appreciate that unsigned integer overflow is implementation defined, but I am not

Unsigned integer overflow in `toom_eval_pm2.c`

2023-09-03 Thread Andrew Teylu
Hi, I'm working with gmp-6.3.0 compiled with clang 16 and its `-fsanitize=integer` flag. When I run `multiply.c` from gmpbench [https://gmplib.org/gmpbench], I'm seeing an unsigned integer overflow in `toom_eval_pm2.c` on this line: ``` neg ^= ((k & 1) - 1) ``` The values we're norm