Re: [PATCH] x86/atomic: Fix UB on various atomic helpers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.10.2026 17:38, Alejandro Vallejo wrote:
> On Wed Oct 14, 2026 at 4:33 PM CEST, Jan Beulich wrote:
>> On 14.10.2026 15:35, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/include/asm/atomic.h
>>> +++ b/xen/arch/x86/include/asm/atomic.h
>>> @@ -122,7 +122,7 @@ static inline void atomic_add(int i, atomic_t *v)
>>>  
>>>  static inline int atomic_add_return(int i, atomic_t *v)
>>>  {
>>> -    return i + arch_fetch_and_add(&v->counter, i);
>>> +    return (unsigned int)i + arch_fetch_and_add(&v->counter, i);
>>>  }
>>
>> I'm concerned seeing this kind of change: It's merely papering over an
>> issue in the caller. There's not going to be UB if the caller passes
>> suitable values. And the atomic types hardly are meant to be used in
>> contexts where overflow could occur.
>>
>> Furthermore different parts of the C spec say different things here [1],
>> [2]. Hence whether overflow in signed calculations is undefined or
>> implementation-defined doesn't become entirely clear, according to my
>> reading.
> 
> I won't dwell on interpreting the standard, because I only care how
> GCC and Clang developers interpreted it; and that's a hard trap on
> signed overflow with sanitisers enabled.
> 
> I can live without this patch (it's just paranoia), but thinking about
> it some more, perhaps we ought to pass -fno-strict-overflow, like Linux
> does, to the compiler. This brings forward post-C23 overflow semantics
> and removes this whole class of UB wholesale. Here and elsewhere.
> 
> Thoughts?

Doing so sounds dangerous for an existing code base, as in risking to hide
real issues. As said in the earlier reply, atomic_t uses very likely are
intended to never overflow. If there were use cases, we may need to
introduce uatomic_t (the unsigned counterpart that you mentioned as an
option to _convert_ to), to be used there.

Jan