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

[Apelete Seketeli via llvm-commits]

apelete added a comment.

In http://reviews.llvm.org/D19085#400579, @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 your review.


http://reviews.llvm.org/D19085