[Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Arnaud Charlet]

[-- Attachment #1: Type: text/plain, Size: 213 bytes --]

Use _NSGetEnviron to get environment.

Tested on x86_64-pc-linux-gnu, committed on trunk

2015-01-07  Tristan Gingold  <gingold@adacore.com>

	PR ada/64349
	* env.c (__gnat_environ): Adjust for darwin9/darwin10.


[-- Attachment #2: difs --]
[-- Type: text/plain, Size: 710 bytes --]

Index: env.c
===================================================================
--- env.c	(revision 219191)
+++ env.c	(working copy)
@@ -44,6 +44,12 @@
 #include <stdlib.h>
 #endif
 
+#if defined (__APPLE__) && !defined (__arm__)
+/* On Darwin, _NSGetEnviron must be used for shared libraries; but it is not
+   available on iOS.  */
+#include <crt_externs.h>
+#endif
+
 #if defined (__vxworks)
   #if defined (__RTP__)
     /* On VxWorks 6 Real-Time process mode, environ is defined in unistd.h.  */
@@ -212,6 +218,8 @@
 #elif ! (defined (__vxworks))
   extern char **environ;
   return environ;
+#elif defined (__APPLE__) && !defined (__arm__)
+  return *_NSGetEnviron ();
 #else
   return environ;
 #endif

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Iain Sandoe]

Hi Tristan,

On 7 Jan 2015, at 10:15, Arnaud Charlet wrote:

> Use _NSGetEnviron to get environment.
> 
> Tested on x86_64-pc-linux-gnu, committed on trunk
> 
> 2015-01-07  Tristan Gingold  <gingold@adacore.com>
> 
> 	PR ada/64349
> 	* env.c (__gnat_environ): Adjust for darwin9/darwin10.
> 
> <difs.txt>

So my original patch assumed that, while it was not legal to use environ from a shlib, it is legal to use _NSGetEnviron () from an application ...

.. and, OK fine, I see the point about ! defined (__arm__) .. but a few other comments.

ISTM that there's a partial implementation to distinguish between IN_RTS and application?

Index: env.c
===================================================================
--- env.c	(revision 219191)
+++ env.c	(working copy)
@@ -44,6 +44,12 @@
 #include <stdlib.h>
 #endif
 
+#if defined (__APPLE__) && !defined (__arm__)
+/* On Darwin, _NSGetEnviron must be used for shared libraries; but it is not
+   available on iOS.  */
+#include <crt_externs.h>
+#endif
+

This definition ^ fires for IN_RTS ...
... but there's no definition of _NSGetEnviron in the other branch unless it happens to get pulled in via config.h / system.h - I don't know that this is guaranteed, do you?

----

 #if defined (__vxworks)
   #if defined (__RTP__)
     /* On VxWorks 6 Real-Time process mode, environ is defined in unistd.h.  */
@@ -212,6 +218,8 @@
 #elif ! (defined (__vxworks))
   extern char **environ;
   return environ;

 ^ I don't expect __vxworks to be defined for a Darwin compiler ...
.. so not sure what the code below will achieve.

+#elif defined (__APPLE__) && !defined (__arm__)
+  return *_NSGetEnviron ();

 ^ here you use _NSGetEnviron () unconditionally (see comment re. IN_RTS).
 #else
   return environ;
 #endif

====

If we want to distinguish between in IN_RTS and application maybe sth pseudo-code like:

if IN_RTS
  if apple && ! arm
   include crt_externals.h
   define environ (*_ _NSGetEnviron ())
  end
else
  if apple and ! arm 
   extern "C" char ** environ;
  end
end

......

amend the vxworks case ... 

+#elif defined (__APPLE__) && !defined (__arm__)
+  return environ;

====

Am I missing some point here?
Iain

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Tristan Gingold]

> On 08 Jan 2015, at 13:49, Iain Sandoe <iain@codesourcery.com> wrote:
> 
> Hi Tristan,
> 
> On 7 Jan 2015, at 10:15, Arnaud Charlet wrote:
> 
>> Use _NSGetEnviron to get environment.
>> 
>> Tested on x86_64-pc-linux-gnu, committed on trunk
>> 
>> 2015-01-07  Tristan Gingold  <gingold@adacore.com>
>> 
>> 	PR ada/64349
>> 	* env.c (__gnat_environ): Adjust for darwin9/darwin10.
>> 
>> <difs.txt>
> 
> So my original patch assumed that, while it was not legal to use environ from a shlib, it is legal to use _NSGetEnviron () from an application ...
> 
> .. and, OK fine, I see the point about ! defined (__arm__) .. but a few other comments.
> 
> ISTM that there's a partial implementation to distinguish between IN_RTS and application?

Yes you're right.  The added code should have been added after the #endif for IN_RTS.

I will fix that.  Current code should compile, possibly with warnings.

Tristan.

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Iain Sandoe]

On 8 Jan 2015, at 13:52, Tristan Gingold wrote:

> 
>> On 08 Jan 2015, at 13:49, Iain Sandoe <iain@codesourcery.com> wrote:
>> 
>> Hi Tristan,
>> 
>> On 7 Jan 2015, at 10:15, Arnaud Charlet wrote:
>> 
>>> Use _NSGetEnviron to get environment.
>>> 
>>> Tested on x86_64-pc-linux-gnu, committed on trunk
>>> 
>>> 2015-01-07  Tristan Gingold  <gingold@adacore.com>
>>> 
>>> 	PR ada/64349
>>> 	* env.c (__gnat_environ): Adjust for darwin9/darwin10.
>>> 
>>> <difs.txt>
>> 
>> So my original patch assumed that, while it was not legal to use environ from a shlib, it is legal to use _NSGetEnviron () from an application ...
>> 
>> .. and, OK fine, I see the point about ! defined (__arm__) .. but a few other comments.
>> 
>> ISTM that there's a partial implementation to distinguish between IN_RTS and application?
> 
> Yes you're right.  The added code should have been added after the #endif for IN_RTS.

How about this?
It uses the interface where needed, avoids it for main exes and gets rid of the negative conditional (which IMO makes the code a little more readable).

Iain

P.S. this is not Darwin9/10 - specific the only reason it doesn't fail on Darwin >= 11 is because they default to -undefined dynamic_lookup .. and so find the symbol from the exe.

====

ada:

	PR ada/64349
	* env.c (__gnat_environ): Adjust environ access for Darwin.

Index: gcc/ada/env.c
===================================================================
--- gcc/ada/env.c	(revision 219325)
+++ gcc/ada/env.c	(working copy)
@@ -215,12 +215,12 @@
 #elif defined (sun)
   extern char **_environ;
   return _environ;
-#elif ! (defined (__vxworks))
-  extern char **environ;
+#elif defined (IN_RTS) && defined (__APPLE__) && !defined (__arm__)
+  return *_NSGetEnviron ();
+#elif defined (__vxworks)
   return environ;
-#elif defined (__APPLE__) && !defined (__arm__)
-  return *_NSGetEnviron ();
 #else
+  extern char **environ;
   return environ;
 #endif
 }

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Tristan Gingold]

> On 09 Jan 2015, at 00:42, Iain Sandoe <iain@codesourcery.com> wrote:
> 
> 
> On 8 Jan 2015, at 13:52, Tristan Gingold wrote:
> 
>> 
>>> On 08 Jan 2015, at 13:49, Iain Sandoe <iain@codesourcery.com> wrote:
>>> 
>>> Hi Tristan,
>>> 
>>> On 7 Jan 2015, at 10:15, Arnaud Charlet wrote:
>>> 
>>>> Use _NSGetEnviron to get environment.
>>>> 
>>>> Tested on x86_64-pc-linux-gnu, committed on trunk
>>>> 
>>>> 2015-01-07  Tristan Gingold  <gingold@adacore.com>
>>>> 
>>>> 	PR ada/64349
>>>> 	* env.c (__gnat_environ): Adjust for darwin9/darwin10.
>>>> 
>>>> <difs.txt>
>>> 
>>> So my original patch assumed that, while it was not legal to use environ from a shlib, it is legal to use _NSGetEnviron () from an application ...
>>> 
>>> .. and, OK fine, I see the point about ! defined (__arm__) .. but a few other comments.
>>> 
>>> ISTM that there's a partial implementation to distinguish between IN_RTS and application?
>> 
>> Yes you're right.  The added code should have been added after the #endif for IN_RTS.
> 
> How about this?
> It uses the interface where needed, avoids it for main exes and gets rid of the negative conditional (which IMO makes the code a little more readable).
> 
> Iain
> 
> P.S. this is not Darwin9/10 - specific the only reason it doesn't fail on Darwin >= 11 is because they default to -undefined dynamic_lookup .. and so find the symbol from the exe.

Sorry for the late answer.  We did something slightly different: always #include crt_externs.h on no-arm Darwin.

Tristan.

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Iain Sandoe]

On 14 Jan 2015, at 09:03, Tristan Gingold wrote:

> 
>> On 09 Jan 2015, at 00:42, Iain Sandoe <iain@codesourcery.com> wrote:
>> 
>> 
>> On 8 Jan 2015, at 13:52, Tristan Gingold wrote:
>> 
>>> 
>>>> On 08 Jan 2015, at 13:49, Iain Sandoe <iain@codesourcery.com> wrote:
>>>> 
>>>> Hi Tristan,
>>>> 
>>>> On 7 Jan 2015, at 10:15, Arnaud Charlet wrote:
>>>> 
>>>>> Use _NSGetEnviron to get environment.
>>>>> 
>>>>> Tested on x86_64-pc-linux-gnu, committed on trunk
>>>>> 
>>>>> 2015-01-07  Tristan Gingold  <gingold@adacore.com>
>>>>> 
>>>>> 	PR ada/64349
>>>>> 	* env.c (__gnat_environ): Adjust for darwin9/darwin10.
>>>>> 
>>>>> <difs.txt>
>>>> 
>>>> So my original patch assumed that, while it was not legal to use environ from a shlib, it is legal to use _NSGetEnviron () from an application ...
>>>> 
>>>> .. and, OK fine, I see the point about ! defined (__arm__) .. but a few other comments.
>>>> 
>>>> ISTM that there's a partial implementation to distinguish between IN_RTS and application?
>>> 
>>> Yes you're right.  The added code should have been added after the #endif for IN_RTS.
>> 
>> How about this?
>> It uses the interface where needed, avoids it for main exes and gets rid of the negative conditional (which IMO makes the code a little more readable).
>> 
>> Iain
>> 
>> P.S. this is not Darwin9/10 - specific the only reason it doesn't fail on Darwin >= 11 is because they default to -undefined dynamic_lookup .. and so find the symbol from the exe.
> 
> Sorry for the late answer.  We did something slightly different: always #include crt_externs.h on no-arm Darwin.

Any news on when this might hit trunk?
 - it is a bootstrap issue (although on older targets).
thanks
Iain

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Arnaud Charlet]

> Any news on when this might hit trunk?
>  - it is a bootstrap issue (although on older targets).

Right, and you have local patches/a work around.

I have been on paternity leave, so with no time to sync our changes (and
with other priorities :-)).

My next sync won't be before next week.

Let us know if you'd like to see Tristan's patch before that, we can send it
to you in the mean.

Arno

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Iain Sandoe]

On 20 Jan 2015, at 10:53, Arnaud Charlet wrote:

>> Any news on when this might hit trunk?
>> - it is a bootstrap issue (although on older targets).
> 
> Right, and you have local patches/a work around.
> 
> I have been on paternity leave, so with no time to sync our changes (and
> with other priorities :-)).

indeed :-) 

> My next sync won't be before next week.
> 
> Let us know if you'd like to see Tristan's patch before that, we can send it
> to you in the mean.

That's fine - as you say we have a wrok-around in the meantime,
thanks
Iain

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349).

[Dominique d'Humières]

> Le 20 janv. 2015 à 11:59, Iain Sandoe <iain@codesourcery.com> a écrit :
> 
> 
> On 20 Jan 2015, at 10:53, Arnaud Charlet wrote:
> 
>>> Any news on when this might hit trunk?
>>> - it is a bootstrap issue (although on older targets).
>> 
>> Right, and you have local patches/a work around.
>> 
>> I have been on paternity leave, so with no time to sync our changes (and
>> with other priorities :-)).
> 
> indeed :-) 
> 
>> My next sync won't be before next week.
>> 
>> Let us know if you'd like to see Tristan's patch before that, we can send it
>> to you in the mean.
> 
> That's fine - as you say we have a wrok-around in the meantime,
> thanks
> Iain

Could you please post (or mail me) Tristan's patch? I’ld like to test it before it is committed (chat échaudé craint l’eau froide!).

TIA

Dominique

[Ada] Fix bootstrapping on darwin9/10 (PR ada/64349)

[Arnaud Charlet]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Avoid possible warning on darwin during compiler build.

Should hopefully close PR 64349, committed on trunk

2015-01-30  Tristan Gingold  <gingold@adacore.com>

	PR ada/64349
	* env.c: Move vxworks and darwin includes out of #ifdef IN_RTS.


[-- Attachment #2: difs --]
[-- Type: text/plain, Size: 1664 bytes --]

Index: env.c
===================================================================
--- env.c	(revision 220273)
+++ env.c	(working copy)
@@ -6,7 +6,7 @@
  *                                                                          *
  *                          C Implementation File                           *
  *                                                                          *
- *            Copyright (C) 2005-2014, Free Software Foundation, Inc.       *
+ *            Copyright (C) 2005-2015, Free Software Foundation, Inc.       *
  *                                                                          *
  * GNAT is free software;  you can  redistribute it  and/or modify it under *
  * terms of the  GNU General Public License as published  by the Free Soft- *
@@ -30,15 +30,21 @@
  ****************************************************************************/
 
 #ifdef IN_RTS
-#include "tconfig.h"
-#include "tsystem.h"
+# include "tconfig.h"
+# include "tsystem.h"
 
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <time.h>
-#ifdef VMS
-#include <unixio.h>
-#endif
+# include <sys/stat.h>
+# include <fcntl.h>
+# include <time.h>
+# ifdef VMS
+#  include <unixio.h>
+# endif
+/* We don't have libiberty, so use malloc.  */
+# define xmalloc(S) malloc (S)
+#else /* IN_RTS */
+# include "config.h"
+# include "system.h"
+#endif /* IN_RTS */
 
 #if defined (__MINGW32__)
 #include <stdlib.h>
@@ -71,13 +77,6 @@
   #endif
 #endif
 
-/* We don't have libiberty, so use malloc.  */
-#define xmalloc(S) malloc (S)
-#else /* IN_RTS */
-#include "config.h"
-#include "system.h"
-#endif /* IN_RTS */
-
 #ifdef __cplusplus
 extern "C" {
 #endif

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349)

[Iain Sandoe]

Hi Tristan,

On 30 Jan 2015, at 15:13, Arnaud Charlet wrote:

> Avoid possible warning on darwin during compiler build.

it's not "just a warning" it's a documented incorrect usage which causes a link error (and thus bootstrap fail) on systems that are not using the catch-all "-Wl, -undefined, dynamic_lookup".

> Should hopefully close PR 64349,

I don't think this is going to work because ... 
... as I pointed out before... 
(and provided a working patch to resolve)....

char **
__gnat_environ (void)
{
#if defined (VMS) || defined (RTX)
  /* Not implemented */
  return NULL;
#elif defined (__MINGW32__)
  return _environ;
#elif defined (sun)
  extern char **_environ;
  return _environ;
#elif ! (defined (__vxworks))

^^^^^^ __vxworks will not be defined by anything other than a vxworks compiler, I'd assume (it is certainly not defined by Darwin toolchains)

  extern char **environ;
  return environ;

vvvvvvv so I don't see how this case will ever be exercised.
#elif defined (__APPLE__) && !defined (__arm__)
  return *_NSGetEnviron ();
#else
  return environ;
#endif
}



> committed on trunk
> 
> 2015-01-30  Tristan Gingold  <gingold@adacore.com>
> 
> 	PR ada/64349
> 	* env.c: Move vxworks and darwin includes out of #ifdef IN_RTS.
> 
> <difs.txt>

Re: [Ada] Fix bootstrapping on darwin9/10 (PR ada/64349)

[Tristan Gingold]

> #elif ! (defined (__vxworks))
> 
> ^^^^^^ __vxworks will not be defined by anything other than a vxworks compiler, I'd assume (it is certainly not defined by Darwin toolchains)
> 
>  extern char **environ;
>  return environ;
> 
> vvvvvvv so I don't see how this case will ever be exercised.
> #elif defined (__APPLE__) && !defined (__arm__)
>  return *_NSGetEnviron ();
> #else
>  return environ;
> #endif
> }

Ehh, you're right.  They must be swapped.  I missed the '!' in the vxworks case, and that wasn't detected by our build.

Tristan.