Messages in this thread | | | Date | Tue, 30 Aug 2022 21:52:32 +0200 | Subject | Re: [PATCH v3] overflow: Allow mixed type arguments | From | Rasmus Villemoes <> |
| |
On 30/08/2022 21.21, Kees Cook wrote: > When the check_[op]_overflow() helpers were introduced, all arguments were > required to be the same type to make the fallback macros simpler. However, > now that the fallback macros have been removed[1], it is fine to allow > mixed types, which makes using the helpers much more useful, as they > can be used to test for type-based overflows (e.g. adding two large ints > but storing into a u8), as would be handy in the drm core[2]. > > Remove the restriction, and add additional self-tests that exercise some > of the mixed-type overflow cases, and double-check for accidental macro > side-effects. >
> -/* > - * For simplicity and code hygiene, the fallback code below insists on > - * a, b and *d having the same type (similar to the min() and max() > - * macros), whereas gcc's type-generic overflow checkers accept > - * different types. Hence we don't just make check_add_overflow an > - * alias for __builtin_add_overflow, but add type checks similar to > - * below. > +/** check_add_overflow() - Calculate addition with overflow checking > + * > + * @a: first addend > + * @b: second addend > + * @d: pointer to store sum > + * > + * Returns 0 on success. > + * > + * *@d holds the results of the attempted addition, but is not considered > + * "safe for use" on a non-zero return value, which indicates that the > + * sum has overflowed or been truncated.
I don't like that wording. It makes it sound like there's some ambiguity or (implementation|un)-definedness involved in what the destination holds on overflow. The gcc documentation is perfectly clear that the result is the infinite-precision result truncated to N bits, with N being the bitwidth of d.
Rasmus
| |