Bug 24533 - FAIL: a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***
Summary: FAIL: a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ada (show other bugs)
Version: 4.1.0
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-26 02:41 UTC by John David Anglin
Modified: 2007-09-30 16:26 UTC (History)
1 user (show)

See Also:
Host: hppa-unknown-linux-gnu
Target: hppa-unknown-linux-gnu
Build: hppa-unknown-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John David Anglin 2005-10-26 02:41:20 UTC
This occurs on many ada tasking tests:

splitting /home/dave/gnu/gcc-4.0/objdir/gcc/testsuite/ada/acats/tests/a/a85013b.
ada into:
   a85013b.adb
BUILD a85013b.adb
gnatmake --GCC="/home/dave/gnu/gcc-4.0/objdir/gcc/xgcc -B/home/dave/gnu/gcc-4.0/
objdir/gcc/" -gnatws -g -O2 -I/home/dave/gnu/gcc-4.0/objdir/gcc/testsuite/ada/ac
ats/support a85013b.adb -largs --GCC="/home/dave/gnu/gcc-4.0/objdir/gcc/xgcc -B/
home/dave/gnu/gcc-4.0/objdir/gcc/"
/home/dave/gnu/gcc-4.0/objdir/gcc/xgcc -c -B/home/dave/gnu/gcc-4.0/objdir/gcc/ -
gnatws -g -O2 -I/home/dave/gnu/gcc-4.0/objdir/gcc/testsuite/ada/acats/support a8
5013b.adb
gnatbind -aO./ -I/home/dave/gnu/gcc-4.0/objdir/gcc/testsuite/ada/acats/support -
I- -x a85013b.ali
gnatlink a85013b.ali -g --GCC=/home/dave/gnu/gcc-4.0/objdir/gcc/xgcc -B/home/dav
e/gnu/gcc-4.0/objdir/gcc/
RUN a85013b

,.,. A85013B ACATS 2.5 05-10-25 16:01:51
---- A85013B CHECK THAT A SUBPROGRAM CAN BE RENAMED WITHIN ITS OWN BODY
                AND THAT THE NEW NAME CAN BE USED IN A RENAMING
                DECLARATION.
*** glibc detected *** free(): invalid pointer: 0x00062a00 ***
/home/dave/gnu/gcc-4.0/gcc/gcc/testsuite/ada/acats/run_all.sh: line 15:  7892 Ab
orted                 (core dumped) $*
FAIL:   a85013b

Looking at this with gdb, it looks as if the pointer passed to free might
be off by 8 bytes.

Breakpoint 1, <__gnat_free> (ptr=403968) at s-memory.adb:107
107        procedure Free (Ptr : System.Address) is
Current language:  auto; currently ada
(gdb) bt
#0  <__gnat_free> (ptr=403968) at s-memory.adb:107
#1  0x0001f5d8 in system.task_primitives.operations.finalize_tcb (t=0x62a00)
    at s-taprop.adb:827
#2  0x00030824 in system.tasking.stages.vulnerable_complete_master (
    self_id=0x61880) at s-tassta.adb:1560
#3  0x00033be0 in a87b59a__B_4___clean___1150 () at a87b59a.adb:33
#4  0x00033f18 in a87b59a () at a87b59a.adb:144
#5  0x0001523c in main (argc=1, argv=3227612440, envp=3227612448)
    at b~a87b59a.adb:168
(gdb) p/x ptr
$1 = 0x62a00
Comment 1 John David Anglin 2005-10-26 02:45:59 UTC
Oops, somehow how ended up debugging in the wrong directory.  However,
it has the same error.
Comment 2 John David Anglin 2005-10-26 04:00:18 UTC
The same errors occur on the 4.0 branch using libc6 2.3.5-6.0.1.
Comment 3 dave 2006-01-01 17:11:58 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

I've looked at the failure of a85013b.adb.

The problem seems to be that "new" is applying a kind of rounding
to the pointer returned by malloc.  Then, the rounded pointer is used
for the call to free.  This is the cause of the invalid pointer
error and the test being aborted by free().

The rounding occurs in this function:

   --------------
   -- New_ATCB --
   --------------

   function New_ATCB (Entry_Num : Task_Entry_Index) return Task_Id is
   begin
     return new Ada_Task_Control_Block (Entry_Num);
   end New_ATCB

This is the assembly code (-O0):
(gdb) disass
Dump of assembler code for function system.task_primitives.operations.new_atcb:
0x0003186c <system.task_primitives.operations.new_atcb+0>:      stw rp,-14(,sp)
0x00031870 <system.task_primitives.operations.new_atcb+4>:      copy r3,r1
0x00031874 <system.task_primitives.operations.new_atcb+8>:      copy sp,r3
0x00031878 <system.task_primitives.operations.new_atcb+12>:     stw,ma r1,40(,sp)
0x0003187c <system.task_primitives.operations.new_atcb+16>:     stw r4,8(,r3)
0x00031880 <system.task_primitives.operations.new_atcb+20>:     stw r26,-24(,r3)
0x00031884 <system.task_primitives.operations.new_atcb+24>:     ldw -24(,r3),ret0
0x00031888 <system.task_primitives.operations.new_atcb+28>:     cmpiclr,< 0,ret0,r0
0x0003188c <system.task_primitives.operations.new_atcb+32>:     ldi 0,ret0
0x00031890 <system.task_primitives.operations.new_atcb+36>:     depw,z ret0,28,29,ret0
0x00031894 <system.task_primitives.operations.new_atcb+40>:     ldo 757(ret0),ret0
0x00031898 <system.task_primitives.operations.new_atcb+44>:     depwi 0,31,4,ret0
0x0003189c <system.task_primitives.operations.new_atcb+48>:     ldo 10(ret0),ret0
0x000318a0 <system.task_primitives.operations.new_atcb+52>:     copy ret0,r26
---Type <return> to continue, or q <return> to quit---
0x000318a4 <system.task_primitives.operations.new_atcb+56>:     b,l 0x2e438 <__gnat_malloc>,rp
0x000318a8 <system.task_primitives.operations.new_atcb+60>:     nop
0x000318ac <system.task_primitives.operations.new_atcb+64>:     copy ret0,r20
0x000318b0 <system.task_primitives.operations.new_atcb+68>:     copy r20,ret0
0x000318b4 <system.task_primitives.operations.new_atcb+72>:     sub r0,ret0,ret0
0x000318b8 <system.task_primitives.operations.new_atcb+76>:     extrw,u ret0,31,4,r19
0x000318bc <system.task_primitives.operations.new_atcb+80>:     copy r20,ret0
0x000318c0 <system.task_primitives.operations.new_atcb+84>:     add,l ret0,r19,ret0
0x000318c4 <system.task_primitives.operations.new_atcb+88>:     copy ret0,r4
0x000318c8 <system.task_primitives.operations.new_atcb+92>:     copy r4,ret0
0x000318cc <system.task_primitives.operations.new_atcb+96>:     copy ret0,r26
0x000318d0 <system.task_primitives.operations.new_atcb+100>:    ldw -24(,r3),r25
0x000318d4 <system.task_primitives.operations.new_atcb+104>:    b,l 0x32698 <system__tasking__ada_task_control_blockIP>,rp
0x000318d8 <system.task_primitives.operations.new_atcb+108>:    nop
0x000318dc <system.task_primitives.operations.new_atcb+112>:    copy r4,ret0
0x000318e0 <system.task_primitives.operations.new_atcb+116>:    ldw -14(,r3),rp
0x000318e4 <system.task_primitives.operations.new_atcb+120>:    ldw 8(,r3),r4
---Type <return> to continue, or q <return> to quit---
0x000318e8 <system.task_primitives.operations.new_atcb+124>:    ldo 40(r3),sp
0x000318ec <system.task_primitives.operations.new_atcb+128>:    ldw,mb -40(,sp),r3
0x000318f0 <system.task_primitives.operations.new_atcb+132>:    bv,n r0(rp)
End of assembler dump(gdb) bt
#0  0x000318cc in system.task_primitives.operations.new_atcb (entry_num=1)
    at s-taprop.adb:752
#1  0x000512f8 in system.tasking.stages.create_task (priority=-1,
    size=-2147483648, task_info=default_scope, num_entries=1, master=4,
    state=0xc068c746, discriminants=3228092176, elaborated=0xc068c714, chain=,
    task_image=0x5edf4, created_task=0x0) at s-tassta.adb:574
#2  0x00055498 in a85013b__tTKVIP___789 () at a85013b.adb:65
#3  0x000551c8 in a85013b () at a85013b.adb:65
#4  0x00014d84 in main ()

(gdb) p $r19
$8 = 8
(gdb) p $r20
$9 = 562904
(gdb) p $ret0
$10 = 562912

The value in r20 is the result that was returned in ret0 in the call
to __gnat_malloc.  The value in $ret0 at this point is the value that
system.task_primitives.operations.new_atcb returns and is subsequently
used in the call to free().

I don't know ada well enough to know where this bit of code comes from

0x000318b0 <system.task_primitives.operations.new_atcb+68>:     copy r20,ret0
0x000318b4 <system.task_primitives.operations.new_atcb+72>:     sub r0,ret0,ret0
0x000318b8 <system.task_primitives.operations.new_atcb+76>:     extrw,u ret0,31,4,r19
0x000318bc <system.task_primitives.operations.new_atcb+80>:     copy r20,ret0
0x000318c0 <system.task_primitives.operations.new_atcb+84>:     add,l ret0,r19,ret0

but it appears to round the address returned by malloc up to a 16-byte
boundary.  The rounded address is used in the call to free.  This
is undefined:

       free() frees the memory space pointed to by ptr, which must  have  been
       returned by a previous call to malloc(), calloc() or realloc().  Other-
       wise, or  if  free(ptr)  has  already  been  called  before,  undefined
       behaviour occurs.

It seems that the 16-byte alignment used for TCBs is a result of using
16-byte alignment for atomic_lock_t.  Changing the alignment to 8 fixes
the testsuite failures.  The current glibc implementation handles lock
objects that aren't aligned to 16-byte boundaries by dynamically picking
an aligned lock word:

  /* We need 128-bit alignment for the ldcw semaphore.  At most, we are
     assured of 64-bit alignment for stack locals and malloc'd data.  Thus,
     we use a struct with four ints for the atomic lock type.  The locking
     code will figure out which of the four to use for the ldcw semaphore.  */
  typedef volatile struct {
    int lock[4];
  } __attribute__ ((aligned(16))) __atomic_lock_t;

It's unclear to me whether changing the alignment for atomic_lock_t
in s-osinte-linux-hppa.ads is correct as it changes struct layouts,
and could cause problems if an object containing an atomic_lock_t object
is copied.

It is clear that the pointer returned by malloc must be retained for
the free operation.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)


Comment 4 Arnaud Charlet 2006-01-03 13:28:58 UTC
The bug is that the following line in s-osinte-linux-hppa.ads is wrong:

   for atomic_lock_t'Alignment use 8 * 16;

The alignment clause takes *bytes*, not *bits*, so you need to use instead:

   for atomic_lock_t'Alignment use 16;

The inconsistency between new and free for objects aligned more than
Standard'Maximum_Alignment is indeed a known latent issue that
is being worked on and is not trivial to fix, but should not affect the Ada
run-time itself (except when a wrong clause is defined as was the case here).

Change suggested above pre-approved.

Arno
Comment 5 dave 2006-01-03 15:03:11 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> The alignment clause takes *bytes*, not *bits*, so you need to use instead:
> 
>    for atomic_lock_t'Alignment use 16;
> 
> The inconsistency between new and free for objects aligned more than
> Standard'Maximum_Alignment is indeed a known latent issue that
> is being worked on and is not trivial to fix, but should not affect the Ada
> run-time itself (except when a wrong clause is defined as was the case here).
> 
> Change suggested above pre-approved.

This has been done

2005-12-28  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

        * s-osinte-linux-hppa.ads: Correct alignment of atomic_lock_t.

and it doesn't fix the invalid tcb pointers being passed to free().
Reducing the alignment to 8, fixes the pointer problem.  This will
work from a locking standpoint, but it's not correct from a struct
layout standpoint.  However, it might be ok depending on how ada uses
atomic_lock_t objects.

If ada is going to align malloc'd pointers, then it should keep track
of the adjustment or the original pointer so that the memory can be
freed using the original pointer.

Dave
Comment 6 charlet@adacore.com 2006-01-03 15:10:31 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> and it doesn't fix the invalid tcb pointers being passed to free().
> Reducing the alignment to 8, fixes the pointer problem.  This will
> work from a locking standpoint, but it's not correct from a struct
> layout standpoint.  However, it might be ok depending on how ada uses
> atomic_lock_t objects.

Hmm, so that means that 16 is bigger than Standard'Maximum_Alignment...

Is it really the case that the C headers mandate an alignment of 16 for this
type which is not guaranteed by malloc ? These alignment issues are
very tricky, and GCC also has several notions of maximum alignments, so
this are is definitely of that is causing many troubles with Ada when using
GCC 3.x or 4.x

Ada does not do anything with these directly, so things should be OK,
except that it's easy to get a subtle alignment issue/discrepency.

> If ada is going to align malloc'd pointers, then it should keep track
> of the adjustment or the original pointer so that the memory can be
> freed using the original pointer.

Right, and this is a non trivial task, hence my previous comment.

Arno
Comment 7 dave 2006-01-03 15:49:46 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> Hmm, so that means that 16 is bigger than Standard'Maximum_Alignment...

Unfortunately, yes.  The fundamental issue is that "ldcw" is the
only atomic memory operation available in PA 1.x, and it requires
a 16-byte alignment.  In PA 2.0, this is relaxed to 4-byte alignment
when the operation is coherent.  The linux folks weren't willing
to to throw out their PA 1.1 hardware, so we have this 16-byte
alignment...

> Is it really the case that the C headers mandate an alignment of 16 for this
> type which is not guaranteed by malloc ? These alignment issues are
> very tricky, and GCC also has several notions of maximum alignments, so
> this are is definitely of that is causing many troubles with Ada when using
> GCC 3.x or 4.x

Agreed.  Because of these issues, I came up with the scheme to make
the size of the atomic_lock_t type 16 bytes.  The pthread code dynamically
picks the aligned 4-byte integer for the ldcw.  Thus, the atomic_lock_t
type now only needs 4-byte alignment.  However, the 16-byte alignment
attribute was retained for compatibility.  There are some limitations to
this scheme and I think the TLS thread implementation will do locking
with a light-weight syscall rather than ldcw.  This might reduce the
alignment requirement to 4 bytes.

> Ada does not do anything with these directly, so things should be OK,
> except that it's easy to get a subtle alignment issue/discrepency.
> 
> > If ada is going to align malloc'd pointers, then it should keep track
> > of the adjustment or the original pointer so that the memory can be
> > freed using the original pointer.
> 
> Right, and this is a non trivial task, hence my previous comment.

Ok, then I think the alignment should be reduced to 4 or 8.  I've
done a number of builds using an alignment of 8.  There's only one
fail in 4.0.3:

http://gcc.gnu.org/ml/gcc-testresults/2006-01/msg00025.html

RUN cxg1005

,.,. CXG1005 ACATS 2.5 06-01-01 01:38:37
---- CXG1005 Check that the subprograms defined in the package
                Generic_Complex_Elementary_Functions provide correct
		results.
   * CXG1005 Non-imaginary result from Function Log with a minus one
		input value.
   * CXG1005 Non-real result from Function Arcsin with a minus one input
		value.
   * CXG1005 Non-real result from Function Arccos with a minus one input
		value.
**** CXG1005 FAILED ****************************.
FAIL:   cxg1005

I checked the first issue in C and clog(-1.0) seems ok.

Dave
Comment 8 hainque@adacore.com 2006-01-03 16:25:05 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

charlet at adacore dot com wrote:
> Hmm, so that means that 16 is bigger than Standard'Maximum_Alignment...

 Yes, the latter is 8, computed from GCC's BIGGEST_ALIGNMENT.

> Is it really the case that the C headers mandate an alignment of 16 for this
> type which is not guaranteed by malloc ?

 IIUC, it asks for a 16 bytes alignment consciously knowing that it likely
 won't get it in terms of objects' _addresses_. Now the request will
 still impact component _offsets_ within records (make them multiple
 of 16), as Dave points out, and not matching it in Ada might indeed
 cause various troubles.

 The big difference between Ada and C here is that the alignment must
 also be obeyed for objects addresses in Ada, which is why the
 allocator return address is rounded up (together with the size
 argument).

> > If ada is going to align malloc'd pointers, then it should keep track
> > of the adjustment or the original pointer so that the memory can be
> > freed using the original pointer.
> 
> Right, and this is a non trivial task, hence my previous comment.

 Right. Preliminary attempts at fixing this a while ago made it necessary
 to touch expand_expr and is_alignin_offset, fairly deep in the back end.

 Will look it up again.


Comment 9 Laurent GUERBY 2006-01-03 19:24:28 UTC
For most (if not all) s-osinte*.ads C type redeclarations, I believe it should
be sufficient to use a record containing a System.Storage_Elements.Storage_Array of the C sizeof(struct), plus may be an alignement clause (I don't know if C or GNU C allows to retrieve the alignment of a struct like sizeof for size).

Arnaud, do you remember non opaque C types in s-osinte?
Comment 10 Andrew Pinski 2006-01-03 19:31:26 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> 
> 
> 
> ------- Comment #9 from laurent at guerby dot net  2006-01-03 19:24 -------
> For most (if not all) s-osinte*.ads C type redeclarations, I believe it should
> be sufficient to use a record containing a
> System.Storage_Elements.Storage_Array of the C sizeof(struct), plus may be an
> alignement clause (I don't know if C or GNU C allows to retrieve the alignment
> of a struct like sizeof for size).

GNU C does, __alignof__ (struct a).


-- Pinski
Comment 11 Laurent GUERBY 2006-01-03 19:36:37 UTC
(In reply to comment #7)
> There's only one fail in 4.0.3:
> 
> http://gcc.gnu.org/ml/gcc-testresults/2006-01/msg00025.html
> 
> RUN cxg1005
> 
> ,.,. CXG1005 ACATS 2.5 06-01-01 01:38:37
> ---- CXG1005 Check that the subprograms defined in the package
>                 Generic_Complex_Elementary_Functions provide correct
>                 results.
>    * CXG1005 Non-imaginary result from Function Log with a minus one
>                 input value.
>    * CXG1005 Non-real result from Function Arcsin with a minus one input
>                 value.
>    * CXG1005 Non-real result from Function Arccos with a minus one input
>                 value.
> **** CXG1005 FAILED ****************************.
> FAIL:   cxg1005
> 
> I checked the first issue in C and clog(-1.0) seems ok.
> 
> Dave
> 

This is PR20754

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20754

I'm adding some comments there.
Comment 12 charlet@adacore.com 2006-01-04 09:58:35 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> For most (if not all) s-osinte*.ads C type redeclarations, I believe it should
> be sufficient to use a record containing a
> System.Storage_Elements.Storage_Array of the C sizeof(struct), plus may be an
> alignement clause (I don't know if C or GNU C allows to retrieve the alignment
> of a struct like sizeof for size).

If you use a storage_array, you definitely also need the proper
alignment clause, yes.

> Arnaud, do you remember non opaque C types in s-osinte?

I do not understand your question, could you clarify ?

Arno
Comment 13 Laurent GUERBY 2006-01-04 11:45:14 UTC
(In reply to comment #12)
> > Arnaud, do you remember non opaque C types in s-osinte?
> 
> I do not understand your question, could you clarify ?

"opaque" means that the Ada runtime never access an internal C record component, or does not use knowledge that something is an int and do +1 on it for example.

So my question is wether the record+storage array+align will work for all the C types in s-osinte* or is there an exception (ie a non opaque C type)?
 
Laurent
Comment 14 charlet@adacore.com 2006-01-04 11:54:47 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> So my question is wether the record+storage array+align will work for all
> the C types in s-osinte* or is there an exception (ie a non opaque C type)?

Now I understand your question ;-)
The answer is no, this approach can't be applied blindly to all C types.

This approach could most likely be applied on some platforms to all the
*private* C types, although this is not a good idea because the current files
are written to be used on many architectures (e.g.
s-osinte-hpux.ads can be used under both pa and ia64), so this approach
would simply break it (System.Address has a different size under pa and
ia64, and the default alignments are very different from one architecture/ABI
to the other).

That's why I said a few times already that creating s-osinte-linux-<cpu>.ads
files was not a good idea and that a clean up will need to be done at
some point to merge back those files. Your suggestion would simply
go one step further in making this task close to impossible instead of
difficult.

Arno
Comment 15 dave 2006-01-17 03:49:36 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> ------- Comment #8 from hainque at adacore dot com  2006-01-03 16:25 -------
> Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer:
> 0x00062a00 ***

>  Right. Preliminary attempts at fixing this a while ago made it necessary
>  to touch expand_expr and is_alignin_offset, fairly deep in the back end.

As I understand the situation, fixing the above problem is quite involved.
When the parisc-linux project moved to libc6 2.3.5, free was changed and
the above error changed.

The enclosed change is a work-around for the above problem.  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.

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?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

2006-01-16  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	PR ada/24533
	* s-osinte-linux-hppa.ads: Reduce alignment of atomic_lock_t to 8.

Index: s-osinte-linux-hppa.ads
===================================================================
--- s-osinte-linux-hppa.ads	(revision 109788)
+++ s-osinte-linux-hppa.ads	(working copy)
@@ -508,7 +508,7 @@
       lock : lock_array;
    end record;
    pragma Convention (C, atomic_lock_t);
-   for atomic_lock_t'Alignment use 16;
+   for atomic_lock_t'Alignment use 8;
 
    type struct_pthread_fast_lock is record
       spinlock : atomic_lock_t;
Comment 16 charlet@adacore.com 2006-01-17 08:56:28 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

> OK?

Assuming you add a proper "???" comment explaining why we use an alignment of
8 in this file (basically summarizing this PR), this is OK.

> 2006-01-16  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
> 
>         PR ada/24533
>         * s-osinte-linux-hppa.ads: Reduce alignment of atomic_lock_t to 8.
> 
> Index: s-osinte-linux-hppa.ads
> ===================================================================
> --- s-osinte-linux-hppa.ads     (revision 109788)
> +++ s-osinte-linux-hppa.ads     (working copy)
> @@ -508,7 +508,7 @@
>        lock : lock_array;
>     end record;
>     pragma Convention (C, atomic_lock_t);
> -   for atomic_lock_t'Alignment use 16;
> +   for atomic_lock_t'Alignment use 8;
> 
>     type struct_pthread_fast_lock is record
>        spinlock : atomic_lock_t;
Comment 17 hainque@adacore.com 2006-01-17 16:29:18 UTC
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.




  
Comment 18 charlet@adacore.com 2006-01-17 16:33:06 UTC
Subject: Re:  FAIL:   a85013b: *** glibc detected *** free(): invalid pointer: 0x00062a00 ***

>  I'll let Arno state the definite approval.

As discussed live, I gave my OK this morning already, with the same comment
about ??? ;-)

Arno
Comment 19 John David Anglin 2006-01-20 14:30:38 UTC
Subject: Bug 24533

Author: danglin
Date: Fri Jan 20 14:30:33 2006
New Revision: 110025

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110025
Log:
	PR ada/24533
	* s-osinte-linux-hppa.ads: Reduce alignment of atomic_lock_t to 8.


Modified:
    trunk/gcc/ada/ChangeLog
    trunk/gcc/ada/s-osinte-linux-hppa.ads

Comment 20 John David Anglin 2006-01-20 14:32:18 UTC
Subject: Bug 24533

Author: danglin
Date: Fri Jan 20 14:32:10 2006
New Revision: 110026

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110026
Log:
	PR ada/24533
	* s-osinte-linux-hppa.ads: Reduce alignment of atomic_lock_t to 8.


Modified:
    branches/gcc-4_1-branch/gcc/ada/ChangeLog
    branches/gcc-4_1-branch/gcc/ada/s-osinte-linux-hppa.ads

Comment 21 John David Anglin 2006-01-20 14:34:32 UTC
Subject: Bug 24533

Author: danglin
Date: Fri Jan 20 14:34:29 2006
New Revision: 110027

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110027
Log:
	PR ada/24533
	* s-osinte-linux-hppa.ads: Reduce alignment of atomic_lock_t to 8.


Modified:
    branches/gcc-4_0-branch/gcc/ada/ChangeLog
    branches/gcc-4_0-branch/gcc/ada/s-osinte-linux-hppa.ads

Comment 22 John David Anglin 2007-09-30 16:26:59 UTC
Fixed.
Comment 23 Arnaud Charlet 2008-05-20 12:44:45 UTC
Subject: Bug 24533

Author: charlet
Date: Tue May 20 12:43:59 2008
New Revision: 135614

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=135614
Log:
2008-05-20  Arnaud Charlet  <charlet@adacore.com>

	* s-linux-hppa.ads (atomic_lock_t): Put back proper alignment now that
	the underlying issue with malloc/free has been fixed. Remove associated
	comments.
	Minor reformatting.
	Related to PR ada/24533


Modified:
    trunk/gcc/ada/s-linux-hppa.ads