[bug #33392] Multi-thread bug in NSObject retain and release

[Eric Wasylishen]

Follow-up Comment #2, bug #33392 (project gnustep):

Just as a casual observer, it's good to see this bug identified and hopefully
fixed. :-)

However, I wondered why the ASM code was being used in the first place instead
of the GCC builtins:

#elif defined(__llvm__) || (defined(USE_ATOMIC_BUILDINS) && (__GNUC__ > 4 ||
(__GNUC__ ==      4 && __GNUC_MINOR__ >= 1)))
/* Use the GCC atomic operations with recent GCC versions */

The  USE_ATOMIC_BUILDINS is not defined anywhere, so the gcc builtins will
never be used unless the person building base defines it themselves. We should
simply delete the defined(USE_ATOMIC_BUILTINS) part so the builtins are always
used if available (they should be for most people as gcc 4.1.0 is now 5 years
old).



Possibly answering my own question, I believe we use the wrong gcc builtins!
see: http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html

Currently we have:

typedef int32_t volatile *gsatomic_t;
#define GSATOMICREAD(X) (*(X))
#define GSAtomicIncrement(X)    __sync_fetch_and_add(X, 1)
#define GSAtomicDecrement(X)    __sync_fetch_and_sub(X, 1)

which means GSAtomicIncrement/Decrement return the old value. Don't we want
__sync_add_and_fetch and __sync_sub_and_fetch which return the new value? From
what I can tell the various ASM implementations return the new value as does
the windows call InterlockedIncrement.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?33392>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/