------- Comment #17 from hainque at adacore dot com 2006-01-17 16:29 -------
Subject: Re: FAIL: a85013b: *** glibc detected *** free(): invalid pointer:
0x00062a00 ***
John David Anglin wrote:
> As I understand the situation, fixing the above problem is quite involved.
Indeed. I have dug out the patches involved when this was first attempted,
and look into them again. This was back in June 2004, so a lot has changed
since then.
> The problem is that the alignment provided by malloc is less than
> that needed for atomic_lock_t objects. This causes the ada runtime
> to round the pointer that it receives from malloc; but it doesn't
> retain the adjustment and the free operation has a 50% chance of
> failing.
Close :) The problem as it currently stands is that the alignment required
for atomic_lock_t is larger than BIGGEST_ALIGNMENT, which causes the compiler
to generate special code to handle an allocation request from the default
storage pool (aka bare malloc).
Indeed the generated code arranges for the user visible address (the Ada
allocator value) to be a multiple of the requested alignment by rounding
up the malloc returned address, and the rounded value is erroneously
passed back to free on deallocation request.
> The enclosed change is a work-around for the above problem.
> The linux libc code can accomodate an unaligned atomic_lock_t object
> under most circumstances. The main issue is that ada may detemine
> an incorrect struct layout.
>
> I have tested the above change on hppa-unknown-linux-gnu, 4.0 through
> trunk. Without this change, ada is essentially unusable. Thus, I
> recommend installing the change as a work-around until a proper fix
> can be developed.
>
> OK?
> - for atomic_lock_t'Alignment use 16;
> + for atomic_lock_t'Alignment use 8;
Since it definitely enhances the situation according to your testing
(thanks), I'd go for it with a "???" accompaning comment.
I'll let Arno state the definite approval.
It is not easy to ensure it is really OK because of ripple effects.
It appears fine from the local osinte perspective:
<< type lock_array is array (1 .. 4) of int;
type atomic_lock_t is record
lock : lock_array;
end record;
>>
The base size is 16 bytes, and wouldn't change from an alignment
upgrade to 16.
<< type struct_pthread_fast_lock is record
spinlock : atomic_lock_t;
>>
The field is at beginning here so there is no offset rounding mistake
to fear, and the size is right so following fields are laid out identically.
Besides, in
<< type pthread_mutex_t is record
m_reserved : int;
m_count : int;
m_owner : System.Address;
m_kind : int;
m_lock : struct_pthread_fast_lock;
end record;
>>
the m_lock offset is a multiple of 16 here by virtue of the
preceeding components (4 all 4 bytes long).
Now, I think a 16 bytes alignment for atomic_lock_t would force a 16
bytes alignment for struct_pthread_fast_lock + pthread_mutex_t, and
double checking the potential effects of that is fairly involved.
Thanks for your feedback.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24533
