Type mismatches in low level atomics

[Blas_Rodriguez_Somoz]

Hello

Building the tbb library with "-Wconversion" and clang, I've found that there is a general type mismatch in the definition of low level atomics at machine level and at generic level. With gcc (-Wconversion) or msvc (/W4), the compiler only get some of the warnings and those warning are removed with casts, but with clang it is easier to see the whole picture because it gets all the warnings.

The atomics functions implemented at machine level, use signed parameters, for all the implementations.For instance in Linux_ia32.h:

[bash]__TBB_MACHINE_DEFINE_ATOMICS(1,int8_t,"","=q")
__TBB_MACHINE_DEFINE_ATOMICS(2,int16_t,"","=r")
__TBB_MACHINE_DEFINE_ATOMICS(4,int32_t,"l","=r")
[/bash]

But atomic functions in tbb_machine.h use unsigned parameters. The __TBB_MaskedCompareAndSwap template states that parameters must be unsigned, and all the functions using it, use unsigned parameters.For instance:

[cpp]template<>
inline uint8_t __TBB_CompareAndSwapGeneric <1,uint8_t> (volatile void *ptr, uint8_t value, uint8_t comparand ) {
#if __TBB_USE_GENERIC_PART_WORD_CAS
    return __TBB_MaskedCompareAndSwap<1,uint8_t>((volatile uint8_t *)ptr,value,comparand);
#else
    return __TBB_machine_cmpswp1(ptr,value,comparand);
#endif
}[/cpp]

At line 6 in the previous code, the compiler should throw 3 warnings (when using W4 o -Wconversion), because there are 3 unsafe conversions, 2 from unsigned to signed (parameters) and 1 from signed to unsigned (return value).

In tbb_machine.h there are some defines which replace functions which use signed parameters with others which use unsigned ones (this is only an example) which probably mean that the coder was not aware of the issue:

[bash]#if __TBB_USE_GENERIC_PART_WORD_CAS
#define __TBB_machine_cmpswp1 tbb::internal::__TBB_CompareAndSwapGeneric<1,uint8_t>
#define __TBB_machine_cmpswp2 tbb::internal::__TBB_CompareAndSwapGeneric<2,uint16_t>
#endif
[/bash]

__TBB_machine_cmpswp1 is defined as using signed parameters in the machine headers

[bash]__TBB_machine_cmpswp1(volatile void *ptr, int8_t value, int8_t comparand) 
[/bash]

in gcc_generic.h, linux_ia32.h, linux_ia64.h, linux_intel64.h

[bash]__TBB_machine_cmpswp1 (volatile void *ptr, __int8 value, __int8 comparand )
[/bash]

in windows_ia32.h or windows_intel64.h

[RafSchietekat]

This can be resolved by making everything signed (except where needed in the implementation of __TBB_MaskedCompareAndSwap), and may already be under investigation.

[Blas_Rodriguez_Somoz]

If it is already under investigation, fine.

I only want to point the issue. It can be solved from the compiler POV with more casts, something that was partially done previously, apparently due to msvc warnings, but it seems it can be solved better taking the whole thing into account.