[vlc-devel] [PATCH v2] thread: fix compilation with clang 7 or older

[Marvin Scholz]

On 2 Mar 2020, at 9:28, Steve Lhomme wrote:

> On 2020-03-02 8:53, Marvin Scholz wrote:
>> C11's atomic_load_explicit did not allow a const qualified first
>> argument, even though it never modifies it (see DR459).
>> Clang versions 7 or older did implement this quite strictly and
>> therefore errors when compiling this code:
>>
>>    address argument to atomic operation must be a pointer to 
>> non-const
>>    _Atomic type ('const _Atomic(const void *) *' invalid)
>>
>> See https://reviews.llvm.org/D47618 and 
>> https://reviews.llvm.org/D47613
>> ---
>>   src/misc/threads.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/misc/threads.c b/src/misc/threads.c
>> index ea78e37453..db90a27240 100644
>> --- a/src/misc/threads.c
>> +++ b/src/misc/threads.c
>> @@ -135,11 +135,21 @@ static _Thread_local char thread_self[1];
>>    bool vlc_mutex_held(const vlc_mutex_t *mtx)
>>   {
>> +#if defined(__clang__) && !defined(__apple_build_version__) && 
>> (__clang_major__ < 8)
>
> The Apple 7- versions don't have this issue ?
>
> IMO it's OK not to add the distinction for this compiler flavor. 
> Otherwsise we'd have to check which version from which vendor has the 
> issue.
>
> LGTM otherwise.

Apple Clang uses a different versioning compared to the Open Source 
releases.
I can’t easily check if whatever versions might correspond to the 
affected ones
have this issue.

I tried it with Xcode 10 and it was fine. Given that Xcode 10 is the 
oldest Xcode
version we currently support for 4.x, I believe there is no need to 
check.

>
>> +    /* The explicit cast to non-const is needed to workaround a 
>> clang
>> +     * error with clang 7 or lower, as atomic_load_explicit in C11 
>> did not
>> +     * allow its first argument to be const-qualified (see DR459)
>> +     */
>> +    vlc_mutex_t *tmp_mtx = (vlc_mutex_t *)mtx;
>> +#else
>> +    const vlc_mutex_t *tmp_mtx = mtx;
>> +#endif
>> +
>>       /* This comparison is thread-safe:
>>        * Even though other threads may modify the owner field at any 
>> time,
>>        * they will never make it compare equal to the calling thread.
>>        */
>> -    return THREAD_SELF == atomic_load_explicit(&mtx->owner,
>> +    return THREAD_SELF == atomic_load_explicit(&tmp_mtx->owner,
>>                                                  memory_order_relaxed);
>>   }
>>  -- 
>> 2.21.1 (Apple Git-122.3)
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel