On Fri, Sep 12, 2025 at 12:11:28PM +0100, Jonathan Wakely wrote: > On Fri, 12 Sept 2025 at 08:02, Jonathan Wakely <[email protected]> wrote: > > > > On Fri, 12 Sept 2025 at 06:44, Dimitar Dimitrov <[email protected]> wrote: > > > > > > On Mon, Sep 08, 2025 at 01:35:59PM +0100, Jonathan Wakely 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. > > > > --- > > > > > > > > v2: no changes to this first patch in the series. > > > > > > > > Tested powerpc64le-linux. > > > > > > > > .../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); > > > > } > > > > > > > > > > Hi Jonathan, > > > > > > This broke the build for pru-unknown-elf target. I suspect it's either > > > because it uses newlib instead of glibc. Or because this target does not > > > define its own atomic ops. > > > > > > The error is: > > > atomicity.cc: In function ‘_Atomic_word > > > __gnu_cxx::__exchange_and_add(volatile _Atomic_word*, int)’: > > > atomicity.cc:47:49: error: invalid conversion from ‘volatile > > > _Atomic_word*’ {aka ‘volatile int*’} to ‘_Atomic_word*’ {aka ‘int*’} > > > [-fpermissive] > > > 47 | return __gnu_cxx::__exchange_and_add_single(__mem, __val); > > > | ^~~~~ > > > | | > > > | volatile > > > _Atomic_word* {aka volatile int*} > > > In file included from atomicity.cc:25: > > > > > > /home/dinux/projects/pru/testbot-workspace/pru-gcc-build/pru/libstdc++-v3/include/ext/atomicity.h:95:43: > > > note: initializing argument 1 of ‘_Atomic_word > > > __gnu_cxx::__exchange_and_add_single(_Atomic_word*, int)’ > > > 95 | __exchange_and_add_single(_Atomic_word* __mem, int __val) > > > | ~~~~~~~~~~~~~~^~~~~ > > > > > > Ugh, there's really no reason for these functions to take a volatile > > pointer. > > > > I'll fix it today... > > This should be fixed at r16-3820-g4fe3b8b8db5c19
Thank you for the swift response. Build is fixed now. Regards, Dimitar
