diff mbox series

Darwin, libgfortran : Do not use environ directly from the library.

Message ID 573D3BC5-53B5-4B29-8C03-5B8D60B5B485@sandoe.co.uk
State New
Headers show
Series Darwin, libgfortran : Do not use environ directly from the library. | expand

Commit Message

Iain Sandoe Nov. 20, 2020, 8:54 p.m. UTC
Hi,

not sure if this is covered directly by my Darwin maintainer’s hat so …

-----

On macOS / Darwin, the environ variable can be used directly in the
code of an executable, but cannot be used in the code of a shared
library (i.e. libgfortran.dylib)**

In such cases, the  function _NSGetEnviron should be called to get
the address of 'environ’.

tested on a number of Darwin platforms old and new, and on
x86_64/powerpc64-linux,

OK for master?
… and backports to open branches?
thanks
Iain

** we’ve been “getting away with it” because of the application of
dynamic_lookup in a rather broad manner, which I’m about to get rid
of.

—————

libgfortran/ChangeLog:

	* intrinsics/execute_command_line.c (environ): Use
	_NSGetEnviron to get the environment pointer on Darwin.



---
  libgfortran/intrinsics/execute_command_line.c | 5 +++++
  1 file changed, 5 insertions(+)

  #include <signal.h>

Comments

Thomas Koenig Nov. 21, 2020, 8:12 a.m. UTC | #1
Hi Iain,

> tested on a number of Darwin platforms old and new, and on
> x86_64/powerpc64-linux,
> 
> OK for master?
> … and backports to open branches?

One question...

> +# ifdef __APPLE__
> +#  include <crt_externs.h>
> +#  define environ (*_NSGetEnviron ())

Is it guaranteed that crt_externs.h is present if __APPLE__
is defined?

If that's the case, OK for master and backports.

Best regards

	Thomas
Iain Sandoe Nov. 21, 2020, 9:46 a.m. UTC | #2
Hi Thomas,

Thomas Koenig via Fortran <fortran@gcc.gnu.org> wrote:
>>
>> tested on a number of Darwin platforms old and new, and on
>> x86_64/powerpc64-linux,
>> OK for master?
>> … and backports to open branches?
>
> One question...
>
>> +# ifdef __APPLE__
>> +#  include <crt_externs.h>
>> +#  define environ (*_NSGetEnviron ())
>
> Is it guaranteed that crt_externs.h is present if __APPLE__
> is defined?

__APPLE__ is defined by GCC when targetting macOS (actually by other  
compilers too,
although that’s probably not important for building libgfortran).

The header is present on all macOS systems supported by GCC (and at least  
two revisions
prior to the earliest that currently builds GCC open branches).

Of course, a user with -D on the command line can define it .. but then  
will get a build
error - which is fine __APPLE__ is in the implementation namespace, the  
user gets what
they deserve ;)

We could be paranoid and write #if defined (__APPLE__) &&  
__has_include(<crt_externs.h>)
but that seems over the top here.

> If that's the case, OK for master and backports.

applied to master,
will deal with the backports along with other Darwin ones.
thanks
Iain
diff mbox series

Patch

diff --git a/libgfortran/intrinsics/execute_command_line.c  
b/libgfortran/intrinsics/execute_command_line.c
index 71d61a766ad..6d7b8fc658e 100644
--- a/libgfortran/intrinsics/execute_command_line.c
+++ b/libgfortran/intrinsics/execute_command_line.c
@@ -34,7 +34,12 @@  see the files COPYING3 and COPYING.RUNTIME  
respectively.  If not, see
  #endif
  #ifdef HAVE_POSIX_SPAWN
  #include <spawn.h>
+# ifdef __APPLE__
+#  include <crt_externs.h>
+#  define environ (*_NSGetEnviron ())
+# else
  extern char **environ;
+# endif
  #endif
  #if defined(HAVE_POSIX_SPAWN) || defined(HAVE_FORK)