Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug
Bug#: 14538
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Not yet assigned to anyone <unassigned@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: Joel Sherrill <joel@gcc.gnu.org>
Add CC:
CC:
Remove selected CCs
Build:
URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
ada.diff *-rtems Ada patch text/plain 2004-03-11 19:46 1.47 KB Edit
rtems_ada.diff current version of patch patch 2004-04-01 14:49 4.11 KB Edit | Diff
rtems_ada-20040401.diff revised version of previous patch patch 2004-04-02 01:12 4.05 KB Edit | Diff
rtems_ada-20040408-HEAD.diff Same patch modified for the CVS Head patch 2004-04-08 17:50 3.88 KB Edit | Diff
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 14538 depends on: 14436 Show dependency tree
Show dependency graph
Bug 14538 blocks:

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Last confirmed: 2004-04-07 02:59 Opened: 2004-03-11 19:43
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 From Joel Sherrill 2004-03-11 19:46 -------
Created an attachment (id=5901) [edit]
*-rtems Ada patch

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

------- Comment #2 From Mark Mitchell 2004-03-11 21:34 -------
If an Ada maintainer signs off on this patch, it is OK for 3.4.0.

------- Comment #3 From charlet@act-europe.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.

Arno

------- Comment #4 From joel.sherrill@oarcorp.com 2004-03-12 13:29 -------
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 From charlet@act-europe.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.

> 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 From Joel Sherrill 2004-03-12 14:42 -------
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 From Joel Sherrill 2004-03-12 14:43 -------
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 From charlet@act-europe.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.

Arno

------- Comment #9 From joel.sherrill@oarcorp.com 2004-03-13 15:14 -------
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 From Arnaud Charlet 2004-03-19 09:25 -------
Note that the run time clean up in *taprop.adb has been committed.

Arno

------- Comment #11 From Joel Sherrill 2004-04-01 14:49 -------
Created an attachment (id=6024) [edit]
current version of patch

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

------- Comment #12 From charlet@act-europe.fr 2004-04-01 15:04 -------
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 From Joel Sherrill 2004-04-02 01:12 -------
Created an attachment (id=6031) [edit]
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 From Joel Sherrill 2004-04-02 01:17 -------
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 From Arnaud Charlet 2004-04-02 15:58 -------
Patch is fine, OK for commit.

Arno

------- Comment #16 From Andrew Pinski 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.

------- Comment #17 From Joel Sherrill 2004-04-07 13:06 -------
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 From Mark Mitchell 2004-04-07 22:05 -------
This patch is OK for 3.4.0.

------- Comment #19 From CVS Commits 2004-04-08 17:40 -------
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 From Joel Sherrill 2004-04-08 17:50 -------
Created an attachment (id=6054) [edit]
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 From CVS Commits 2004-04-08 17:54 -------
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 From Joel Sherrill 2004-04-08 17:54 -------
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.


Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug