On Fri, Jul 18, 2025 at 8:03 PM Jonathan Wakely <jwak...@redhat.com> wrote:
> The standard requires that std::atomic<integral-type>::fetch_add does
> not have undefined behaviour for signed overflow, instead it wraps like
> unsigned integers. The compiler ensures this is true for the atomic
> built-ins that std::atomic uses, but it's not currently true for the
> __gnu_cxx::__exchange_and_add and __gnu_cxx::__atomic_add functions
> defined in libstdc++, which operate on type _Atomic_word.
>
> For the inline __exchange_and_add_single function (used when there's
> only one thread in the process), we can copy the value to an unsigned
> long and do the addition on that, then assign it back to the
> _Atomic_word variable.
>
> The __exchange_and_add in config/cpu/generic/atomicity_mutex/atomicity.h
> locks a mutex and then performs exactly the same steps as
> __exchange_and_add_single. Calling __exchange_and_add_single instead of
> duplicating the code benefits from the fix just made to
> __exchange_and_add_single.
>
> For the remaining config/cpu/$arch/atomicity.h implementations, they
> either use inline assembly which uses wrapping instructions (so no
> changes needed), or we can fix them by compiling with -fwrapv.
>
> After ths change, UBsan no longer gives an error for:
>
> _Atomic_word i = INT_MAX;
> __gnu_cxx::__exchange_and_add_dispatch(&i, 1);
>
> /usr/include/c++/14/ext/atomicity.h:85:12: runtime error: signed integer
> overflow: 2147483647 + 1 cannot be represented in type 'int'
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/121148
> * config/cpu/generic/atomicity_mutex/atomicity.h
> (__exchange_and_add): Call __exchange_and_add_single.
> * include/ext/atomicity.h (__exchange_and_add_single): Use an
> unsigned type for the addition.
> * libsupc++/Makefile.am (atomicity.o): Compile with -fwrapv.
> * libsupc++/Makefile.in: Regenerate.
> ---
>
> Tested x86_64-linux.
>
This LGTM.
>
> This should not affect codegen, except maybe making it a bit faster with
> UBsan enabled because there's no instrumentation needed to detect signed
> overflows!
>
> .../cpu/generic/atomicity_mutex/atomicity.h | 5 +--
> libstdc++-v3/include/ext/atomicity.h | 35 +++++++++++++++++--
> libstdc++-v3/libsupc++/Makefile.am | 5 +++
> libstdc++-v3/libsupc++/Makefile.in | 5 +++
> 4 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/libstdc++-v3/config/cpu/generic/atomicity_mutex/atomicity.h
> b/libstdc++-v3/config/cpu/generic/atomicity_mutex/atomicity.h
> index b1328c025bcc..7d5772d54840 100644
> --- a/libstdc++-v3/config/cpu/generic/atomicity_mutex/atomicity.h
> +++ b/libstdc++-v3/config/cpu/generic/atomicity_mutex/atomicity.h
> @@ -44,10 +44,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> __exchange_and_add(volatile _Atomic_word* __mem, int __val) throw ()
> {
> __gnu_cxx::__scoped_lock sentry(get_atomic_mutex());
> - _Atomic_word __result;
> - __result = *__mem;
> - *__mem += __val;
> - return __result;
> + return __gnu_cxx::__exchange_and_add_single(__mem, __val);
> }
>
> void
> diff --git a/libstdc++-v3/include/ext/atomicity.h
> b/libstdc++-v3/include/ext/atomicity.h
> index 650b786cd8e6..0b970f33f4b6 100644
> --- a/libstdc++-v3/include/ext/atomicity.h
> +++ b/libstdc++-v3/include/ext/atomicity.h
> @@ -39,6 +39,9 @@
> #if __has_include(<sys/single_threaded.h>)
> # include <sys/single_threaded.h>
> #endif
> +#if __cplusplus >= 201103L
> +# include <type_traits> // make_unsigned_t
> +#endif
>
> namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
> {
> @@ -79,19 +82,47 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> __atomic_add(volatile _Atomic_word*, int) _GLIBCXX_NOTHROW;
> #endif
>
> +#if __cplusplus < 201103L
> + // The array bound will be ill-formed in the very unlikely case that
> + // _Atomic_word is wider than long and we need to use unsigned long long
> + // below in __exchange_and_add_single and __atomic_add_single.
> + typedef int
> + _Atomic_word_fits_in_long[sizeof(_Atomic_word) <= sizeof(long) ? 1 :
> -1];
> +#endif
> +
> inline _Atomic_word
> __attribute__((__always_inline__))
> __exchange_and_add_single(_Atomic_word* __mem, int __val)
> {
> _Atomic_word __result = *__mem;
> - *__mem += __val;
> + // Do the addition with an unsigned type so that overflow is well
> defined.
> +#if __cplusplus >= 201103L
> + std::make_unsigned<_Atomic_word>::type __u;
> +#else
> + // For most targets make_unsigned_t<_Atomic_word> is unsigned int,
> + // but 64-bit sparc uses long for _Atomic_word.
> + // Sign-extending to unsigned long works for both cases.
> + unsigned long __u;
> +#endif
> + __u = __result;
> + __u += __val;
> + *__mem = __u;
> return __result;
> }
>
> inline void
> __attribute__((__always_inline__))
> __atomic_add_single(_Atomic_word* __mem, int __val)
> - { *__mem += __val; }
> + {
> +#if __cplusplus >= 201103L
> + std::make_unsigned<_Atomic_word>::type __u;
> +#else
> + unsigned long __u; // see above
> +#endif
> + __u = *__mem;
> + __u += __val;
> + *__mem = __u;
> + }
>
> inline _Atomic_word
> __attribute__ ((__always_inline__))
> diff --git a/libstdc++-v3/libsupc++/Makefile.am
> b/libstdc++-v3/libsupc++/Makefile.am
> index 60e7d1594e6d..098d93719ce6 100644
> --- a/libstdc++-v3/libsupc++/Makefile.am
> +++ b/libstdc++-v3/libsupc++/Makefile.am
> @@ -132,6 +132,11 @@ atomicity_file =
> ${glibcxx_srcdir}/$(ATOMICITY_SRCDIR)/atomicity.h
> atomicity.cc: ${atomicity_file}
> $(LN_S) ${atomicity_file} ./atomicity.cc || true
>
> +atomicity.lo: atomicity.cc
> + $(LTCOMPILE) -fwrapv -c $<
> +atomicity.o: atomicity.cc
> + $(CXXCOMPILE) -fwrapv -c $<
> +
> if OS_IS_DARWIN
> # See PR 112397
> new_opvnt.lo: new_opvnt.cc
> diff --git a/libstdc++-v3/libsupc++/Makefile.in
> b/libstdc++-v3/libsupc++/Makefile.in
> index 732ab89c8a2e..b691915e396b 100644
> --- a/libstdc++-v3/libsupc++/Makefile.in
> +++ b/libstdc++-v3/libsupc++/Makefile.in
> @@ -973,6 +973,11 @@ cp-demangle.o: cp-demangle.c
> atomicity.cc: ${atomicity_file}
> $(LN_S) ${atomicity_file} ./atomicity.cc || true
>
> +atomicity.lo: atomicity.cc
> + $(LTCOMPILE) -fwrapv -c $<
> +atomicity.o: atomicity.cc
> + $(CXXCOMPILE) -fwrapv -c $<
> +
> # See PR 112397
> @OS_IS_DARWIN_TRUE@new_opvnt.lo: new_opvnt.cc
> @OS_IS_DARWIN_TRUE@ $(LTCXXCOMPILE) -fno-reorder-blocks-and-partition
> -I. -c $<
> --
> 2.50.1
>
>
