Re: [Patch, libstdc++] Fix data races in basic_string implementation

[Dmitry Vyukov <dvyukov at google dot com>]

On Tue, Sep 1, 2015 at 5:08 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 01/09/15 16:56 +0200, Dmitry Vyukov wrote:
>>
>> I don't understand how a new gcc may not support __atomic builtins on
>> ints. How it is even possible? That's a portable API provided by
>> recent gcc's...
>
>
> The built-in function is always defined, but it might expand to a call
> to an external function in libatomic, and it would be a regression for
> code using std::string to start requiring libatomic (although maybe it
> would be necessary if it's the only way to make the code correct).
>
> I don't know if there are any targets that define __GTHREADS and also
> don't support __atomic_load(int*, ...) without libatomic. If such
> targets exist then adding a new configure check that only depends on
> __atomic_load(int*, ...) would mean we keep supporting those targets.
>
> Another option would be to simply do:
>
>         bool
>         _M_is_shared() const _GLIBCXX_NOEXCEPT
> #if defined(__GTHREADS)
> +        { return __atomic_load(&this->_M_refcount, __ATOMIC_ACQUIRE) > 0; }
> +#else
>         { return this->_M_refcount > 0; }
> +#endif
>
> and see if anyone complains!

I like this option!
If a platform uses multithreading and has non-inlined atomic loads,
then the way to fix this is to provide inlined atomic loads rather
than to fix all call sites.

Attaching new patch. Please take another look.

Attachment: patch.diff
Description: Text document