Bug 14538 - *-rtems broken for gnat
*-rtems broken for gnat
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: ada
3.4.0
: P2 normal
: 3.4.1
Assigned To: Not yet assigned to anyone
:
Depends on: 14436
Blocks:
  Show dependency treegraph
 
Reported: 2004-03-11 19:43 UTC by Joel Sherrill
Modified: 2004-04-08 17:54 UTC (History)
5 users (show)

See Also:
Host:
Target: *-rtems
Build:
Known to work:
Known to fail:
Last reconfirmed: 2004-04-07 02:59:30


Attachments
*-rtems Ada patch (4.46 KB, text/plain)
2004-03-11 19:46 UTC, Joel Sherrill
Details
current version of patch (13.39 KB, patch)
2004-04-01 14:49 UTC, Joel Sherrill
Details | Diff
revised version of previous patch (12.80 KB, patch)
2004-04-02 01:12 UTC, Joel Sherrill
Details | Diff
Same patch modified for the CVS Head (12.26 KB, patch)
2004-04-08 17:50 UTC, Joel Sherrill
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joel Sherrill 2004-03-11 19:43:34 UTC
The atttached patch corrects the rtems specific Ada run-time files.  With this
patch powerpc-rtems and sparc-rtems built Ada successfully.  

NOTE: sparc-rtems also requires PR14537 in order to complete the build.
Comment 1 Joel Sherrill 2004-03-11 19:46:18 UTC
Created attachment 5901 [details]
*-rtems Ada patch

Please review so I can commit this before 3.4.0 is cut.
Comment 2 Mark Mitchell 2004-03-11 21:34:51 UTC
If an Ada maintainer signs off on this patch, it is OK for 3.4.0.
Comment 3 charlet 2004-03-12 06:24:24 UTC
Subject: Re:  *-rtems broken for gnat

> Please review so I can commit this before 3.4.0 is cut.

I'd suggest you post your patch with the missing ChangeLog on
gcc-patches@, following the usual patch submission rules.

The init.c change seems to be a no-op, so I'd rather not integrate it.

Arno
Comment 4 joel.sherrill 2004-03-12 13:29:33 UTC
Subject: Re:  *-rtems broken for gnat

charlet at act-europe dot fr wrote:

>------- Additional Comments From charlet at act-europe dot fr  2004-03-12 06:24 -------
>Subject: Re:  *-rtems broken for gnat
>
>  
>
>>Please review so I can commit this before 3.4.0 is cut.
>>    
>>
>
>I'd suggest you post your patch with the missing ChangeLog on
>gcc-patches@, following the usual patch submission rules.
>
>The init.c change seems to be a no-op, so I'd rather not integrate it.
>
>  
>
Actually it isn't a nop.  It puts the simple __rtems__ check ahead of  a 
check that
is something like the following that I believe indicates the cpp 
predefines for one of the
vxworks targets is broken:

if (AIX or unix) and !vxworks

At least some of the embedded sparc targets were built using solaris 
config files and
ended up with unix defined.  I can see where this could happen on 
another embedded
target cpu configuration and wanted to avoid it accidentally being 
tripped.  In fact,
I submitted a fix for sparc to address this for sparc-rtems.  I simply 
moved a very
straight forward check on an OS specific predefine that is guaranteed to 
be there
ahead of one that is questionable and could be unintentally tripped.

This same problem also resulted in sys/termios.h being included from the 
<ctrl>c
code since it ended up thinking it was on a unix system when it wasn't.

I will also post the patch to the bugs list once I get in.

>Arno
>
>
>  
>


Comment 5 charlet 2004-03-12 13:35:17 UTC
Subject: Re:  *-rtems broken for gnat

> Actually it isn't a nop.  It puts the simple __rtems__ check ahead of  a 

That's why ChangeLogs are so important :-) So we'll wait for your
complete submision.

> check that
> is something like the following that I believe indicates the cpp 
> predefines for one of the
> vxworks targets is broken:
> 
> if (AIX or unix) and !vxworks

That's wrong, vxworks does not define unix, you are misinterpreting the above.

Arno
Comment 6 Joel Sherrill 2004-03-12 14:42:47 UTC
Add a ChangeLog and note that this has been posted to gcc-patches as
http://gcc.gnu.org/ml/gcc-patches/2004-03/msg01037.html

2004-03-12      Joel Sherrill <joel@OARcorp.com>

        * ada/5rosinte.adb, ada/5rosinte.ads, ada/5rtpopsp.adb,
         ada/5rosinte.adb: Update RTEMS specific files to account
        for modifications to shared code.
        * ada/init.c: Move RTEMS code ahead of others using conditionals
        which mix checks for embedded systems (particularly vxworks) and
        Unix hosts. 
Comment 7 Joel Sherrill 2004-03-12 14:43:12 UTC
Subject: Re:  *-rtems broken for gnat

charlet at act-europe dot fr wrote:
> ------- Additional Comments From charlet at act-europe dot fr  2004-03-12 13:35 -------
> Subject: Re:  *-rtems broken for gnat
> 
> 
>>Actually it isn't a nop.  It puts the simple __rtems__ check ahead of  a 
> 
> 
> That's why ChangeLogs are so important :-) So we'll wait for your
> complete submision.

Yep.  And this one is pretty complicated to explain.  It makes me
wonder how many targets have questionable sets of cpp predefines
and no one has noticed or cares.  I know that rtems has tripped
across a number of these type of cases over the years.

>>check that
>>is something like the following that I believe indicates the cpp 
>>predefines for one of the
>>vxworks targets is broken:
>>
>>if (AIX or unix) and !vxworks
> 
> 
> That's wrong, vxworks does not define unix, you are misinterpreting the above.

I won't argue that it shouldn't define it but at least the embedded
sparc ports have historically been based upon solaris config files
with some tweaks so you end up unix predefined and weird checks
like this from ada/init.c

#elif defined (sun) && defined (__SVR4) && !defined (__vxworks)

That pretty clearly tells me that the sparc-vxworks target
suffered from the same problem at some point.  And this one
from the same file:

#elif defined(__alpha__) && defined(__osf__) && ! defined(__alpha_vxworks)

which tells me that osf was probably defined for the alpha-vxworks
target.

So the init.c is moving the check for rtems ahead of all these
cases where weirdness exists.  I suspect that if the vxworks
code were moved up, then you could simplify those conditionals.

--joel


Comment 8 charlet 2004-03-13 12:35:51 UTC
Subject: Re:  *-rtems broken for gnat

> So the init.c is moving the check for rtems ahead of all these
> cases where weirdness exists.  I suspect that if the vxworks
> code were moved up, then you could simplify those conditionals.

Thaks for the hint. We'll try to have a look at it when when start
building GCC 3.4 with VxWorks, since the VxWorks ports have been completely
revamped in 3.4 anyway.

Arno
Comment 9 joel.sherrill 2004-03-13 15:14:00 UTC
Subject: Re:  *-rtems broken for gnat

charlet at act-europe dot fr wrote:

>------- Additional Comments From charlet at act-europe dot fr  2004-03-13 12:35 -------
>Subject: Re:  *-rtems broken for gnat
>
>  
>
>>So the init.c is moving the check for rtems ahead of all these
>>cases where weirdness exists.  I suspect that if the vxworks
>>code were moved up, then you could simplify those conditionals.
>>    
>>
>
>Thaks for the hint. We'll try to have a look at it when when start
>building GCC 3.4 with VxWorks, since the VxWorks ports have been completely
>revamped in 3.4 anyway.
>
>  
>
Does that mean you are OK with me moving the rtems check up or that you 
want it to stay where
it is?

>Arno
>
>
>  
>


Comment 10 Arnaud Charlet 2004-03-19 09:25:33 UTC
Note that the run time clean up in *taprop.adb has been committed.

Arno
Comment 11 Joel Sherrill 2004-04-01 14:49:17 UTC
Created attachment 6024 [details]
current version of patch

Arnaud .. please review so this can make it in the 3.4 series.
Comment 12 charlet 2004-04-01 15:04:15 UTC
Subject: Re:  *-rtems broken for gnat

> Arnaud .. please review so this can make it in the 3.4 series.

I can't review patches without their corresponding changelog.

A quick look shows problems, in particular you can't remove the
ATCB_Key in 7staprop.adb

In any case, you ca never comment out code without adding comments
justifying it.

In this case, the ATCB_Key is needed, since 7stpopsp.adb depends on it.

Arno
Comment 13 Joel Sherrill 2004-04-02 01:12:43 UTC
Created attachment 6031 [details]
revised version of previous patch

Since this patch has not been reviewed, I wanted to be sure it was
the latest version and had a nice ChangeLog entry.

2004-04-01  Joel Sherrill <joel@oarcorp.com>

	* ada/5rosinte.adb: Remove fake mprotect() body.
	* ada/5rosinte.ads: Add SA_SIGINFO.  Make pthread_key_t a type
	which can be set since Finalize_TCB in 7staprop.adb does not
	go through the Set_Specific interface.
	* ada/5rtpopsp.adb: Rewrite to use new interface.
	* ada/init.c: Reorder so the simple single OS conditional __rtems__
	is tested before more complex ones which mix UNIX and embedded
	systems in the conditional.


The change to make pthread_key_t a type which is not private
would not be needed if Finalize_TCB in	7staprop.adb called 
Specific.Set instead of pthread_setspecific.
Comment 14 Joel Sherrill 2004-04-02 01:17:06 UTC
I noticed that this PR has been outstanding for 3 weeks, unassigned,
and even though Arnaud has commented, he wasn;t on the cc list.  
Hopefully we can the review of this wrapped up. 
Comment 15 Arnaud Charlet 2004-04-02 15:58:20 UTC
Patch is fine, OK for commit.

Arno
Comment 16 Andrew Pinski 2004-04-07 02:59:28 UTC
I know that this patch was approved for the mainline, what about 3.4.0? I figure it is too 
late for 3.4.0 so I would say hold off until 3.4.1 to ask.  But Joel please apply this patch to 
the mainline as it is already approved.
Comment 17 Joel Sherrill 2004-04-07 13:06:49 UTC
Subject: Re:  *-rtems broken for gnat

pinskia at gcc dot gnu dot org wrote:
> ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-04-07 02:59 -------
> I know that this patch was approved for the mainline, what about 3.4.0? I figure it is too 
> late for 3.4.0 so I would say hold off until 3.4.1 to ask.  But Joel please apply this patch to 
> the mainline as it is already approved.
> 
Arnaud only approved my 2 Ada PRs over the weekend.  He made
them conditional on successfully passing test run.  It was
only overnight that I got those test results.  So I was
going to commit them this morning.

Mark has approved both PRs for 3.4.0 and I actually just updated
my local 3.4 CVS tree to commit them.  I will go into a holding
pattern and let Mark comment on this and the gnatmake broken for
cross one.


Comment 18 Mark Mitchell 2004-04-07 22:05:04 UTC
This patch is OK for 3.4.0.
Comment 19 CVS Commits 2004-04-08 17:40:19 UTC
Subject: Bug 14538

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	joel@gcc.gnu.org	2004-04-08 17:40:08

Modified files:
	gcc            : ChangeLog 
	gcc/ada        : 5rosinte.adb 5rosinte.ads 5rtpopsp.adb init.c 

Log message:
	2004-04-08  Joel Sherrill <joel@oarcorp.com>
	
	PR ada/14538
	* ada/5rosinte.adb: Remove fake mprotect() body.
	* ada/5rosinte.ads: Add SA_SIGINFO.  Make pthread_key_t a type
	which can be set since Finalize_TCB in 7staprop.adb does not
	go through the Set_Specific interface.
	* ada/5rtpopsp.adb: Rewrite to use new interface.
	* ada/init.c: Reorder so the simple single OS conditional __rtems__
	is tested before more complex ones which mix UNIX and embedded
	systems in the conditional.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.386&r2=2.2326.2.387
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.adb.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3&r2=1.3.16.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.ads.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.5&r2=1.5.16.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rtpopsp.adb.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2&r2=1.2.32.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/init.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.22&r2=1.22.4.1

Comment 20 Joel Sherrill 2004-04-08 17:50:58 UTC
Created attachment 6054 [details]
Same patch modified for the CVS Head

Patch applied to 3.4 branch worked around a nit that Arnaud fixed on the head.
Comment 21 CVS Commits 2004-04-08 17:54:10 UTC
Subject: Bug 14538

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	joel@gcc.gnu.org	2004-04-08 17:54:03

Modified files:
	gcc            : ChangeLog 
	gcc/ada        : 5rosinte.adb 5rosinte.ads 5rtpopsp.adb init.c 

Log message:
	2004-04-08  Joel Sherrill <joel@oarcorp.com>
	
	PR ada/14538
	* ada/5rosinte.adb: Remove fake mprotect() body.
	* ada/5rosinte.ads: Add SA_SIGINFO.
	* ada/5rtpopsp.adb: Rewrite to use new interface.
	* ada/init.c: Reorder so the simple single OS conditional __rtems__
	is tested before more complex ones which mix UNIX and embedded
	systems in the conditional.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3364&r2=2.3365
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.adb.diff?cvsroot=gcc&r1=1.3&r2=1.4
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rosinte.ads.diff?cvsroot=gcc&r1=1.5&r2=1.6
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/5rtpopsp.adb.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ada/init.c.diff?cvsroot=gcc&r1=1.30&r2=1.31

Comment 22 Joel Sherrill 2004-04-08 17:54:45 UTC
Attached patches applied to both head and 3.4 branch.

This is the ChangeLog for the HEAD.

2004-04-08  Joel Sherrill <joel@oarcorp.com>

	PR ada/14538
	* ada/5rosinte.adb: Remove fake mprotect() body.
	* ada/5rosinte.ads: Add SA_SIGINFO.
	* ada/5rtpopsp.adb: Rewrite to use new interface.
	* ada/init.c: Reorder so the simple single OS conditional __rtems__
	is tested before more complex ones which mix UNIX and embedded
	systems in the conditional.