[RFA] save/restore environ in libiberty pex_unix_exec_child

[RFA] save/restore environ in libiberty pex_unix_exec_child

[Doug Evans]

Hi.

I ran into this while using pex_run_in_environment in gdb.

Ok to check in?

2009-12-07  Doug Evans  <dje@google.com>

	* pex-unix.c (pex_unix_exec_child): Save/restore environ.

==== //depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/libiberty/pex-unix.c#1 - /usr/local/google/home/dje/ctools/gdb/7.0.x-prod/depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/libiberty/pex-unix.c ====
--- /tmp/g4-67641/cache/depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/libiberty/pex-unix.c#1	2009-12-06 12:30:36.000000000 -0800
+++ /usr/local/google/home/dje/ctools/gdb/7.0.x-prod/depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/libiberty/pex-unix.c	2009-12-06 12:27:46.000000000 -0800
@@ -397,6 +397,10 @@
   volatile int sleep_interval;
   volatile int retries;
 
+  /* We vfork and then set environ in the child before calling execvp.
+     This clobbers the parent's environ so we need to restore it.  */
+  char **save_environ = environ;
+
   sleep_interval = 1;
   pid = -1;
   for (retries = 0; retries < 4; ++retries)
@@ -468,6 +472,7 @@
 
     default:
       /* Parent process.  */
+      environ = save_environ;
       if (in != STDIN_FILE_NO)
 	{
 	  if (close (in) < 0)

Re: [RFA] save/restore environ in libiberty pex_unix_exec_child

[DJ Delorie]

If the child assignment truly clobbers the parent, won't the parent's
assignment clobber the child?

If it's portable enough, using exevpe/execve instead of execvp/execv
would be preferred.

Re: [RFA] save/restore environ in libiberty pex_unix_exec_child

[Doug Evans]

On Mon, Dec 7, 2009 at 12:09 PM, DJ Delorie <dj@redhat.com> wrote:
>
> If the child assignment truly clobbers the parent, won't the parent's
> assignment clobber the child?
>
> If it's portable enough, using exevpe/execve instead of execvp/execv
> would be preferred.

The context is a child running under vfork.  While the linux man page
says one cannot assume, for example, that the parent is blocked until
the child execs/exits, it would be odd if in implementations where the
parent is not blocked that assignments in the parent clobbered the
child.  s/odd/broken/ even. :-)

[from man vfork]
CONFORMING TO
       4.3BSD,  POSIX.1-2001.  The requirements put on vfork() by the
standards are weaker than those put on fork(2), so an implementation
where the two are
       synonymous is compliant.  In particular, the programmer cannot
rely on the parent remaining blocked until a call of execve(2) or
_exit(2) and  cannot
       rely on any specific behavior with respect to shared memory.

I don't disagree re. execve, *if* it's portable enough.  I wasn't
prepared to investigate the full gamut of portability issues for this.
 Some thought may have gone into what's there now (maybe, maybe not,
but I err'd on the side of being cautious).  My patch seems like a
straightforward fix to what's there now.

Re: [RFA] save/restore environ in libiberty pex_unix_exec_child

[Tom Tromey]

>>>>> "DJ" == DJ Delorie <dj@redhat.com> writes:

DJ> If the child assignment truly clobbers the parent, won't the parent's
DJ> assignment clobber the child?

During a vfork, the parent is suspended until the child execs or dies.
So, this assignment won't affect the child.

Tom

Re: [RFA] save/restore environ in libiberty pex_unix_exec_child

[DJ Delorie]

Hmm... my Linux doesn't even include execvpe().

Ok to check it in then, but *please* include a comment documenting the
assumption that the parent won't run until the child is done.

Re: [RFA] save/restore environ in libiberty pex_unix_exec_child

[Doug Evans]

On Mon, Dec 7, 2009 at 12:55 PM, DJ Delorie <dj@redhat.com> wrote:
>
> Hmm... my Linux doesn't even include execvpe().
>
> Ok to check it in then, but *please* include a comment documenting the
> assumption that the parent won't run until the child is done.
>

Thanks.

I'm always for more comments explaining why things are the way they are.