Re: Fix a bug in emutls

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Feb 17, 2009 at 10:07:43PM +0800, Jie Zhang wrote:
> Jakub Jelinek wrote:
>> On Fri, Feb 13, 2009 at 06:04:36PM +0800, Jie Zhang wrote:
>>> The number of the elements of data[] in struct __emutls_array is 
>>> (size -  1), not size. I found this when I was looking at several 
>>> FAILs in  libgomp testsuite for Blackfin, which uses emutls to 
>>> emulate TLS. This  patch fixes it. OK?
>>
>> I'd say the bug is elsewhere, there is no need to not use the whole
>> allocated area.  Later on we access arr->data[offset - 1], so there is
>> no problem not reallocating when offset == arr->size, arr->data[offset - 1]
>> is still valid.
>>
> From the existing code of emutls.c, it seems that the whole allocated  
> area is for the whole
>
> struct __emutls_array
> {
>   pointer size;
>   void **data[];
> };
>
> So the number of the elements of data[] is (size - 1). The first of the  
> allocated elements is for "pointer size".

Sorry for the delay, I've missed your reply and only remembered it
when seeing PR40024 being filed yesterday.
You're right, I think your patch would be fine, though perhaps the code
would be more readable if arr->size counted number of arr->data entries.
Again, totally untested.

I'll leave that decision to the reviewer.

2009-05-06  Jakub Jelinek  <jakub@redhat.com>

	PR other/40024
	* emutls.c (__emutls_get_address): Change arr->size to mean number
	of allocated arr->data entries instead of # of slots + 1.

--- gcc/emutls.c.jj	2009-04-14 16:33:55.000000000 +0200
+++ gcc/emutls.c	2009-05-06 17:32:47.000000000 +0200
@@ -155,23 +155,23 @@ __emutls_get_address (struct __emutls_ob
   if (__builtin_expect (arr == NULL, 0))
     {
       pointer size = offset + 32;
-      arr = calloc (size, sizeof (void *));
+      arr = calloc (size + 1, sizeof (void *));
       if (arr == NULL)
 	abort ();
       arr->size = size;
       __gthread_setspecific (emutls_key, (void *) arr);
     }
-  else if (__builtin_expect (offset >= arr->size, 0))
+  else if (__builtin_expect (offset > arr->size, 0))
     {
       pointer orig_size = arr->size;
       pointer size = orig_size * 2;
-      if (offset >= size)
+      if (offset > size)
 	size = offset + 32;
-      arr = realloc (arr, size * sizeof (void *));
+      arr = realloc (arr, (size + 1) * sizeof (void *));
       if (arr == NULL)
 	abort ();
       arr->size = size;
-      memset (arr->data + orig_size - 1, 0,
+      memset (arr->data + orig_size, 0,
 	      (size - orig_size) * sizeof (void *));
       __gthread_setspecific (emutls_key, (void *) arr);
     }


	Jakub