diff mbox

[libiberty] Fix PR63758 by using the _NSGetEnviron() API on Darwin.

Message ID 37D7634D-114B-4D35-91E4-5E5237FA487D@codesourcery.com
State New
Headers show

Commit Message

Iain Sandoe Sept. 7, 2015, 3:23 p.m. UTC
Hi,

This is mostly Roland's patch with one extra case added by me plus I moved the new header to include/ as suggested in c#7 of the PR since there are other users for it in the compiler.

==

On Darwin platforms, when referenced from the main executable, it's permitted to access *_environ directly.  When the environment is required from a shared library then the _NSGetEnviron() API should be used.  Since libiberty is used from shared libraries (such as libcc1) this mechanism should be applied here.

OK for trunk and active branches?
Iain

include/

	Roland McGrath  <roland@gnu.org>

	PR other/63758
	* environ.h: New file.

libiberty/

	Roland McGrath  <roland@gnu.org>
	Iain Sandoe  <iain@codesourcery.com>

	PR other/63758
	* pex-unix.c: Obtain the environment interface from settings in environ.h
	rather than in-line code.  Update copyright date.
	* setenv.c: Likewise.
	* xmalloc.c: Likewise.
From 9dec1e4a7a62a1f556dce6e35133e4c503898a74 Mon Sep 17 00:00:00 2001
From: Iain Sandoe <iain@codesourcery.com>
Date: Mon, 7 Sep 2015 10:22:07 +0100
Subject: [PATCH] [libiberty] Provide support for indirect use of _environ by
 Darwin.

When referenced from the main executable, it's permitted to access
*_environ directly.  When environ is required from a shared library then
the NSGetEnviron() API should be used.  Since libiberty is used from
shared libraries (such as libcc1) this mechanism should be applied here.
---
 include/environ.h    | 33 +++++++++++++++++++++++++++++++++
 libiberty/pex-unix.c |  5 ++---
 libiberty/setenv.c   | 10 +++-------
 libiberty/xmalloc.c  |  5 +++--
 4 files changed, 41 insertions(+), 12 deletions(-)
 create mode 100644 include/environ.h

Comments

Mike Stump Sept. 7, 2015, 4:43 p.m. UTC | #1
On Sep 7, 2015, at 8:23 AM, Iain Sandoe <iain@codesourcery.com> wrote:
> On Darwin platforms, when referenced from the main executable, it's permitted to access *_environ directly.  

> OK for trunk and active branches?

Darwin bits Ok.
Ian Lance Taylor Sept. 8, 2015, 2 p.m. UTC | #2
On Mon, Sep 7, 2015 at 8:23 AM, Iain Sandoe <iain@codesourcery.com> wrote:
>
> include/
>
>         Roland McGrath  <roland@gnu.org>
>
>         PR other/63758
>         * environ.h: New file.
>
> libiberty/
>
>         Roland McGrath  <roland@gnu.org>
>         Iain Sandoe  <iain@codesourcery.com>
>
>         PR other/63758
>         * pex-unix.c: Obtain the environment interface from settings in environ.h
>         rather than in-line code.  Update copyright date.
>         * setenv.c: Likewise.
>         * xmalloc.c: Likewise.


This seems likely to break cross-compilers to Darwin that do not have
the system libraries available.  I guess I don't care about that if
you don't.

This is OK for mainline.  Thanks.

Ian
Iain Sandoe Sept. 8, 2015, 2:20 p.m. UTC | #3
On 8 Sep 2015, at 15:00, Ian Lance Taylor wrote:

> On Mon, Sep 7, 2015 at 8:23 AM, Iain Sandoe <iain@codesourcery.com> wrote:
>> 
>> include/
>> 
>>        Roland McGrath  <roland@gnu.org>
>> 
>>        PR other/63758
>>        * environ.h: New file.
>> 
>> libiberty/
>> 
>>        Roland McGrath  <roland@gnu.org>
>>        Iain Sandoe  <iain@codesourcery.com>
>> 
>>        PR other/63758
>>        * pex-unix.c: Obtain the environment interface from settings in environ.h
>>        rather than in-line code.  Update copyright date.
>>        * setenv.c: Likewise.
>>        * xmalloc.c: Likewise.
> 
> 
> This seems likely to break cross-compilers to Darwin that do not have
> the system libraries available.  I guess I don't care about that if
> you don't.

I do care about it, but I'm not visualising the case...

AFAICS, when built as a host component for a cross to Darwin from non-Darwin, environ would be declared as **environ as usual.

If an implementation includes a compiler targetting Darwin that defines __APPLE__ but doesn't provide _NSGetEnviron in its libc, then isn't it broken anyway?

Iain
Ian Lance Taylor Sept. 8, 2015, 2:27 p.m. UTC | #4
On Tue, Sep 8, 2015 at 7:20 AM, Iain Sandoe <iain@codesourcery.com> wrote:
>
>> This seems likely to break cross-compilers to Darwin that do not have
>> the system libraries available.  I guess I don't care about that if
>> you don't.
>
> I do care about it, but I'm not visualising the case...
>
> AFAICS, when built as a host component for a cross to Darwin from non-Darwin, environ would be declared as **environ as usual.
>
> If an implementation includes a compiler targetting Darwin that defines __APPLE__ but doesn't provide _NSGetEnviron in its libc, then isn't it broken anyway?

I'm talking about the case of building a cross-compiler where the
system libraries are not available.  This is sometimes done as a first
step toward building a full cross-compiler.

Ian
Iain Sandoe Oct. 18, 2015, 10:42 a.m. UTC | #5
On 8 Sep 2015, at 15:27, Ian Lance Taylor wrote:

> On Tue, Sep 8, 2015 at 7:20 AM, Iain Sandoe <iain@codesourcery.com> wrote:
>> 
>>> This seems likely to break cross-compilers to Darwin that do not have
>>> the system libraries available.  I guess I don't care about that if
>>> you don't.
>> 
>> I do care about it, but I'm not visualising the case...
>> 
>> AFAICS, when built as a host component for a cross to Darwin from non-Darwin, environ would be declared as **environ as usual.
>> 
>> If an implementation includes a compiler targeting Darwin that defines __APPLE__ but doesn't provide _NSGetEnviron in its libc, then isn't it broken anyway?
> 
> I'm talking about the case of building a cross-compiler where the
> system libraries are not available.  This is sometimes done as a first
> step toward building a full cross-compiler.

I've applied the patch since it solves an immediate problem (and has been requested).

Right now, the only case that I can think of when there's a Darwin-hosted statically-linked user-space executable is in bringing up the system itself, in which case one has to build non-standard crts and a statically-linkable libc.  Last time I did this was on 10.5 with the darwinbuild stuff, not sure it's even feasible on modern Darwin which is built with a different compiler.

It's possible that making the Darwin case conditional on ! defined(__STATIC__) might be sufficient to guard that, but I need to think of some way to test it.

Iain
Mike Stump Oct. 19, 2015, 6:53 p.m. UTC | #6
On Oct 18, 2015, at 3:42 AM, Iain Sandoe <iain@codesourcery.com> wrote:
>>>> This seems likely to break cross-compilers to Darwin that do not have
>>>> the system libraries available.  I guess I don't care about that if
>>>> you don't.
>>> 
>>> I do care about it, but I'm not visualising the case...
>>> 
>>> AFAICS, when built as a host component for a cross to Darwin from non-Darwin, environ would be declared as **environ as usual.
>>> 
>>> If an implementation includes a compiler targeting Darwin that defines __APPLE__ but doesn't provide _NSGetEnviron in its libc, then isn't it broken anyway?
>> 
>> I'm talking about the case of building a cross-compiler where the
>> system libraries are not available.  This is sometimes done as a first
>> step toward building a full cross-compiler.
> 
> I've applied the patch since it solves an immediate problem (and has been requested).
> 
> Right now, the only case that I can think of when there's a Darwin-hosted statically-linked user-space executable is in bringing up the system itself, in which case one has to build non-standard crts and a statically-linkable libc.  Last time I did this was on 10.5 with the darwinbuild stuff, not sure it's even feasible on modern Darwin which is built with a different compiler.
> 
> It's possible that making the Darwin case conditional on ! defined(__STATIC__) might be sufficient to guard that, but I need to think of some way to test it.

So, I see two different things here.  One is a build of the darwin open source kernel.  I’ve never done that, though, I knew people that did.  I don’t play in this space, so I don’t know how much of a rat hole it is, or if it is even possible anymore.  Really, it should just be a drop a new gcc into the official open source darwin build infrastructure and hit build.  If it didn’t just build before, then it might be a time sink to make it work, I just don’t know.

The other is, it is theoretically nice to be able to build up an entire gcc tool chain for a mac, starting from a linux box.  I usually don’t do this, but, I do a subset, which is a cc1 with no headers and no link or assembly support that fails to build, but works far enough to get past cc1.  This isn’t handy for users, but for a developer, I like to do this from time to time.

I don’t see the case that Ian is concerned about.  Either, they have Apple’s library, and it does include this routine, or, someone is making a replacement OS, and then will need to now provide that routine, if they did not before.  Partial builds without a library are fine, but without a library, you can’t link anything (other than -r) anyway, so I’m not sure it matters that it would fail to link, as it failed before anyway (for example, printf would not be found either).

Kernel builds are special, and they are one of the few things that build static, as does the dyld program).  To test either, I’d recommend either, not worrying about it, life is short, or, if you do care enough, you just gotta roll up you sleeves, as they truly are special.
diff mbox

Patch

diff --git a/include/environ.h b/include/environ.h
new file mode 100644
index 0000000..c18902b
--- /dev/null
+++ b/include/environ.h
@@ -0,0 +1,33 @@ 
+/* Declare the environ system variable.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+This file is part of the libiberty library.
+Libiberty is free software; you can redistribute it and/or
+modify it under the terms of the GNU Library General Public
+License as published by the Free Software Foundation; either
+version 2 of the License, or (at your option) any later version.
+
+Libiberty is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+Library General Public License for more details.
+
+You should have received a copy of the GNU Library General Public
+License along with libiberty; see the file COPYING.LIB.  If not,
+write to the Free Software Foundation, Inc., 51 Franklin Street - Fifth Floor,
+Boston, MA 02110-1301, USA.  */
+
+/* On OSX, the environ variable can be used directly in the code of an
+   executable, but cannot be used in the code of a shared library (such as
+   GCC's liblto_plugin, which links in libiberty code).  Instead, the
+   function _NSGetEnviron can be called to get the address of environ.  */
+
+#ifndef HAVE_ENVIRON_DECL
+#  ifdef __APPLE__
+#     include <crt_externs.h>
+#     define environ (*_NSGetEnviron ())
+#  else
+extern char **environ;
+#  endif
+#  define HAVE_ENVIRON_DECL
+#endif
diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c
index 0715115..b48f315 100644
--- a/libiberty/pex-unix.c
+++ b/libiberty/pex-unix.c
@@ -2,7 +2,7 @@ 
    with other subprocesses), and wait for it.  Generic Unix version
    (also used for UWIN and VMS).
    Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004, 2005, 2009,
-   2010 Free Software Foundation, Inc.
+   2010, 2015 Free Software Foundation, Inc.
 
 This file is part of the libiberty library.
 Libiberty is free software; you can redistribute it and/or
@@ -23,6 +23,7 @@  Boston, MA 02110-1301, USA.  */
 #include "config.h"
 #include "libiberty.h"
 #include "pex-common.h"
+#include "environ.h"
 
 #include <stdio.h>
 #include <signal.h>
@@ -390,8 +391,6 @@  pex_child_error (struct pex_obj *obj, const char *executable,
 
 /* Execute a child.  */
 
-extern char **environ;
-
 #if defined(HAVE_SPAWNVE) && defined(HAVE_SPAWNVPE)
 /* Implementation of pex->exec_child using the Cygwin spawn operation.  */
 
diff --git a/libiberty/setenv.c b/libiberty/setenv.c
index 714ca0a..5b51193 100644
--- a/libiberty/setenv.c
+++ b/libiberty/setenv.c
@@ -1,5 +1,5 @@ 
-/* Copyright (C) 1992, 1995, 1996, 1997, 2002, 2011 Free Software Foundation,
-   Inc.
+/* Copyright (C) 1992, 1995, 1996, 1997, 2002, 2011, 2015
+   Free Software Foundation, Inc.
    This file based on setenv.c in the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -62,11 +62,7 @@  extern int errno;
 #endif
 
 #define __environ	environ
-#ifndef HAVE_ENVIRON_DECL
-#ifndef environ
-extern char **environ;
-#endif
-#endif
+#include "environ.h"
 
 #undef setenv
 #undef unsetenv
diff --git a/libiberty/xmalloc.c b/libiberty/xmalloc.c
index 3e97aab..f849aee 100644
--- a/libiberty/xmalloc.c
+++ b/libiberty/xmalloc.c
@@ -1,5 +1,6 @@ 
 /* memory allocation routines with error checking.
-   Copyright 1989, 90, 91, 92, 93, 94 Free Software Foundation, Inc.
+   Copyright 1989, 1990, 1991, 1992, 1993, 1994, 2015
+   Free Software Foundation, Inc.
    
 This file is part of the libiberty library.
 Libiberty is free software; you can redistribute it and/or
@@ -65,6 +66,7 @@  function will be called to print an error message and terminate execution.
 #endif
 #include "ansidecl.h"
 #include "libiberty.h"
+#include "environ.h"
 
 #include <stdio.h>
 
@@ -117,7 +119,6 @@  void
 xmalloc_failed (size_t size)
 {
 #ifdef HAVE_SBRK
-  extern char **environ;
   size_t allocated;
 
   if (first_break != NULL)