On 11/17/20 1:44 PM, Richard Earnshaw (lists) wrote:
> On 03/11/2020 15:08, Bernd Edlinger wrote:
>> Hi,
>>
>> this fixes a problem with a missing symbol __sync_synchronize
>> which happens when newlib is used together with libstdc++ for
>> the non-threaded simulator target arm-none-eabi.
>>
>> There are several questions on stackoverflow about this issue.
>>
>> I would like to add a weak symbol for this target, since this
>> is only a default implementation and not meant to override a
>> possibly more sophisticated synchronization function from the
>> c-runtime.
>>
>>
>> Regression tested successfully on arm-none-eabi with newlib-3.3.0.
>>
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
> 
> I seem to recall that this was a deliberate decision - you can't guess
> this correctly, at least when trying to build portable code - you just
> have to know which runtime you will be using.
> 

Therefore I suggest to use the weak attribute.  It is on purpose not
implementing all of the atomics.

The use case, is a C++ program which initializes a local static variable.

$ cat test.cc
#include <string>
main(int argc, char **argv)
{
  static std::string x = "test";
  return 0;
}

compiles to this:
        sub     sp, sp, #20
        str     r0, [fp, #-24]
        str     r1, [fp, #-28]
        ldr     r3, .L14
        ldrb    r4, [r3]
        bl      __sync_synchronize
        and     r3, r4, #255
        and     r3, r3, #1
        cmp     r3, #0
        moveq   r3, #1
        movne   r3, #0
        and     r3, r3, #255
        cmp     r3, #0
        beq     .L8
        ldr     r0, .L14
        bl      __cxa_guard_acquire
        mov     r3, r0

so __sync_synchronize is not defined in newlib since the target (arm-sim)
is known to be not multi-threaded,
but __cxa_guard_acquire is also not a thread safe function,
because __GTHREADS is not defined by libgcc, since it is known
at configure time, that the target does not support threads.
So libstdc++ does not try to use a mutex or any atomics either,
because it is not compiled with __GTHREADS.

I can further narrow down the patch by only defining this function when
__GTHREADS is not defined, to make it more clear.


> I think Ramana had some changes in the works at one point to address
> (some) of this, but I'm not sure what happened to them.  Ramana?
> 
> 
> +#if defined (__ARM_ARCH_6__) || defined (__ARM_ARCH_6J__)       \
> +    || defined (__ARM_ARCH_6K__) || defined (__ARM_ARCH_6T2__)  \
> +    || defined (__ARM_ARCH_6Z__) || defined (__ARM_ARCH_6ZK__)  \
> +    || defined (__ARM_ARCH_7__) || defined (__ARM_ARCH_7A__)
> +#if defined (__ARM_ARCH_7__) || defined (__ARM_ARCH_7A__)
> 
> Ug, no!  Use the ACLE macros to avoid this sort of mess.
> 

Ah, thanks, copy-paste from freebsd-atomic.c :)


I've attached the updated patch.
Is it OK?


Thanks
Bernd.