[PATCH] D19085: [clang-analyzer] fix warnings emitted on compiler-rt code base

[Apelete Seketeli via llvm-commits]

On Thu, Apr-14-2016 at 01:12:09 AM +0000, George Burgess IV wrote:
> 
> It looks like the static analyzer might think we're allocating an
> array of `emutls_address_array` here. If we want to make it be
> quiet, we can probably just swap to using
> `sizeof(emutls_address_array)`, but I'm not sure if that's better
> than `sizeof(void*)`, given that the former makes it look like we
> were actually trying to allocate an array of arrays (IMO).

I refrained from swapping to use 'sizeof(emutls_address_array)'
because the result would have been not correct to me.
Let's take the following lines:

164.    emutls_address_array* array = pthread_getspecific(emutls_pthread_key);
165.    if (array == NULL) {
166.        uintptr_t new_size = emutls_new_data_array_size(index);
167.        array = (emutls_address_array *) calloc(new_size + 1, sizeof(void*));
168.        emutls_cmheck_array_set_size(array, new_size);
169.    }

My understanding is that the code is currently allocating the 'void*
data' array within emutls_address_array object by also taking into
account the size of the extra member 'uintptr_t size' within
emutls_address_array. Which means we are allocating at once the needed
space to store the value of 'array' since variable 'new_size' is
assigned the total size of emutls_address_array.

Swapping to use 'sizeof(emutls_address_array)' would indeed allocate
an array of arrays (ie. an array of emutls_address_array objects),
which does not look like what the code is trying to do.

I will gladly correct if I'm wrong here, just let me know.

> 
> > but shouldn't the alloc return value be casted for correctness ?
> 
> 
> C allows implicit conversions from `void *` to other pointer types,
> so I don't think an explicit cast here would cause the code to be
> more/less correct.

Here I was thinking that implicit conversions were only allowed in C
(but not in C++), and that this part of the code would be processed as
C++ code.
I realise now that there is actually no C++ construct or feature in
the whole file.

Just for my own information, since I'm new to compiler technology and
LLVM, is this file processed as C or C++ code ?

> Either way, if this patch lands (I don't have an opinion on whether
> or not it should), please also add a cast to the malloc at line 53,
> so we have a consistent style within emutls.c. :)

Ok, will do in next revision.

Thanks for you review.
-- 
        Apelete