diff mbox

[2/3] posix: execvpe cleanup

Message ID 1456495001-5298-3-git-send-email-adhemerval.zanella@linaro.org
State New
Headers show

Commit Message

Adhemerval Zanella Netto Feb. 26, 2016, 1:56 p.m. UTC
This patch removes all the dynamic allocation on execvpe code and
instead use direct stack allocation.  This is QoI approach to make
it possible use in scenarios where memory is shared with parent
(vfork or clone with CLONE_VM).

For default process spawn (script file without a shebang), stack
allocation is bounded by NAME_MAX plus PATH_MAX plus 1.  Large
file arguments returns an error (ENAMETOOLONG).  This differs than
current GLIBC pratice in general, but it used to limit stack
allocation for large inputs.  Also, path in PATH environment variable
larger than PATH_MAX are ignored.

The shell direct execution exeception, where execve returns ENOEXEC,
might requires a large stack allocation due large input argument list.

Tested on i686, x86_64, powerpc64le, and aarch64.

	* posix/execvpe.c (__execvpe): Remove dynamic allocation.
	* posix/Makefile (tests): Add tst-execvpe{1,2,3,4,5,6}.
	* posix/tst-execvp1.c (do_test): Use a macro to call execvp.
	* posix/tst-execvp2.c (do_test): Likewise.
	* posix/tst-execvp3.c (do_test): Likewise.
	* posix/tst-execvp4.c (do_test): Likewise.
	* posix/tst-execvpe1.c: New file.
	* posix/tst-execvpe2.c: Likewise.
	* posix/tst-execvpe3.c: Likewise.
	* posix/tst-execvpe4.c: Likewise.
	* posix/tst-execvpe5.c: Likewise.
	* posix/tst-execvpe6.c: Likewise.
---
 posix/Makefile       |   3 +
 posix/execvpe.c      | 244 +++++++++++++++++++++------------------------------
 posix/tst-execvp1.c  |   6 +-
 posix/tst-execvp2.c  |   5 +-
 posix/tst-execvp3.c  |   5 +-
 posix/tst-execvp4.c  |   6 +-
 posix/tst-execvpe1.c |  20 +++++
 posix/tst-execvpe2.c |  20 +++++
 posix/tst-execvpe3.c |  20 +++++
 posix/tst-execvpe4.c |  20 +++++
 posix/tst-execvpe5.c | 157 +++++++++++++++++++++++++++++++++
 posix/tst-execvpe6.c |  82 +++++++++++++++++
 13 files changed, 455 insertions(+), 146 deletions(-)
 create mode 100644 posix/tst-execvpe1.c
 create mode 100644 posix/tst-execvpe2.c
 create mode 100644 posix/tst-execvpe3.c
 create mode 100644 posix/tst-execvpe4.c
 create mode 100644 posix/tst-execvpe5.c
 create mode 100644 posix/tst-execvpe6.c

Comments

Paul Eggert Feb. 26, 2016, 7:38 p.m. UTC | #1
This one has similar problems with int vs ptrdiff_t. Also:

On 02/26/2016 05:56 AM, Adhemerval Zanella wrote:
>   for (const char *p = path; ; p = subp)
>     {
>        if (errno == ENOEXEC)
>         maybe_script_execute (buffer, argv, envp);

This has O(P*C) behavior, where P is the length of the path and C is the 
argument count. How about changing it to have O(P + C) behavior instead, 
by allocating the substitute argv in __execvpe, and reusing it each time 
through the loop? (Admittedly the current code also has this performance 
bug.)

>   /* Construct an argument list for the shell.  */
>   char *new_argv[argc];

This should be "argc +1", not "argc". Shouldn't we have a test case to 
catch bugs like this?

>   new_argv[0] = (char *) _PATH_BSHELL;
>   new_argv[1] = (char *) file;
>   while (argc > 1)
>     {
>       new_argv[argc] = argv[argc - 1];
>       --argc;
>     }

This mishandles the case where argc == 1, which is possible with an 
empty argument vector. The resulting argument vector is not 
null-terminated. (Admittedly the current code also has this bug.) I 
suppose we should have a test case for this.

>   bool got_eacces = false;
>   for (const char *p = path; ; p = subp)
>     {
>       char buffer[path_len + file_len + 1];

Declare 'buffer' just before the loop starts, not in the loop body. That 
will make the code run a bit faster, as the buffer will be allocated and 
deallocated only once. This is OK since (path_len + file_len + 1) does 
not change.

>       /* Set current path considered plus a '/'.  */
>       memcpy (buffer, p, subp - p);
>       buffer[subp - p] = '/';
>       /* And the file to execute.  */
>       memcpy (buffer + (subp - p) + !!(subp > p), file, file_len + 1);

Shouldn't this be using mempcpy? That should simplify the code. I find 
the "!!" ugly and unnecessary; there's nothing wrong with treating a 
boolean as an int, and besides, !! on a boolean still gives you a 
boolean so what's the point?  Something like this, perhaps:

      /* Use the current path entry, plus a '/' if nonempty, plus the 
file to execute.  */
      char *pend = mempcpy (buffer, p, subp - p);
      *pend = '/';
      memcpy (pend + (p < subp), file, file_len + 1);


> +	  /* Record the we got a 'Permission denied' error.  If we end

the -> that

> +# define EXECVP(__file, __argv) execvp (__file, __argv)

No need for the underscores here.
Adhemerval Zanella Netto Feb. 26, 2016, 8:49 p.m. UTC | #2
On 26-02-2016 16:38, Paul Eggert wrote:
> This one has similar problems with int vs ptrdiff_t. Also:
> 
> On 02/26/2016 05:56 AM, Adhemerval Zanella wrote:
>>   for (const char *p = path; ; p = subp)
>>     {
>>        if (errno == ENOEXEC)
>>         maybe_script_execute (buffer, argv, envp);
> 
> This has O(P*C) behavior, where P is the length of the path and C is the argument count. How about changing it to have O(P + C) behavior instead, by allocating the substitute argv in __execvpe, and reusing it each time through the loop? (Admittedly the current code also has this performance bug.)
> 

I do not oppose, although I would like to focus on fixing the usability and
conformance bugs first and set the performance goal as possible future
improvement. I will add a comment about this possible optimization.

>>   /* Construct an argument list for the shell.  */
>>   char *new_argv[argc];
> 
> This should be "argc +1", not "argc". Shouldn't we have a test case to catch bugs like this?

Indeed I will change that.  I add a testcase.

> 
>>   new_argv[0] = (char *) _PATH_BSHELL;
>>   new_argv[1] = (char *) file;
>>   while (argc > 1)
>>     {
>>       new_argv[argc] = argv[argc - 1];
>>       --argc;
>>     }
> 
> This mishandles the case where argc == 1, which is possible with an empty argument vector. The resulting argument vector is not null-terminated. (Admittedly the current code also has this bug.) I suppose we should have a test case for this.
> 

I will fix that.

>>   bool got_eacces = false;
>>   for (const char *p = path; ; p = subp)
>>     {
>>       char buffer[path_len + file_len + 1];
> 
> Declare 'buffer' just before the loop starts, not in the loop body. That will make the code run a bit faster, as the buffer will be allocated and deallocated only once. This is OK since (path_len + file_len + 1) does not change.
> 

I will change that.

>>       /* Set current path considered plus a '/'.  */
>>       memcpy (buffer, p, subp - p);
>>       buffer[subp - p] = '/';
>>       /* And the file to execute.  */
>>       memcpy (buffer + (subp - p) + !!(subp > p), file, file_len + 1);
> 
> Shouldn't this be using mempcpy? That should simplify the code. I find the "!!" ugly and unnecessary; there's nothing wrong with treating a boolean as an int, and besides, !! on a boolean still gives you a boolean so what's the point?  Something like this, perhaps:
> 
>      /* Use the current path entry, plus a '/' if nonempty, plus the file to execute.  */
>      char *pend = mempcpy (buffer, p, subp - p);
>      *pend = '/';
>      memcpy (pend + (p < subp), file, file_len + 1);
> 
> 

I do not have a preference regarding to "!!" and your suggestion seems better.
I will change that.

>> +      /* Record the we got a 'Permission denied' error.  If we end
> 
> the -> that

Ack.

> 
>> +# define EXECVP(__file, __argv) execvp (__file, __argv)
> 
> No need for the underscores here.

OK, I will change that.
Adhemerval Zanella Netto Feb. 27, 2016, 5:04 p.m. UTC | #3
On 26-02-2016 17:49, Adhemerval Zanella wrote:
> 
> 
> On 26-02-2016 16:38, Paul Eggert wrote:
>> This one has similar problems with int vs ptrdiff_t. Also:
>>
>> On 02/26/2016 05:56 AM, Adhemerval Zanella wrote:
>>>   for (const char *p = path; ; p = subp)
>>>     {
>>>        if (errno == ENOEXEC)
>>>         maybe_script_execute (buffer, argv, envp);
>>
>> This has O(P*C) behavior, where P is the length of the path and C is the argument count. How about changing it to have O(P + C) behavior instead, by allocating the substitute argv in __execvpe, and reusing it each time through the loop? (Admittedly the current code also has this performance bug.)
>>
> 
> I do not oppose, although I would like to focus on fixing the usability and
> conformance bugs first and set the performance goal as possible future
> improvement. I will add a comment about this possible optimization.
> 
>>>   /* Construct an argument list for the shell.  */
>>>   char *new_argv[argc];
>>
>> This should be "argc +1", not "argc". Shouldn't we have a test case to catch bugs like this?
> 
> Indeed I will change that.  I add a testcase.
> 
>>
>>>   new_argv[0] = (char *) _PATH_BSHELL;
>>>   new_argv[1] = (char *) file;
>>>   while (argc > 1)
>>>     {
>>>       new_argv[argc] = argv[argc - 1];
>>>       --argc;
>>>     }
>>
>> This mishandles the case where argc == 1, which is possible with an empty argument vector. The resulting argument vector is not null-terminated. (Admittedly the current code also has this bug.) I suppose we should have a test case for this.
>>
> 
> I will fix that.

In fact I think original code is correct. Although execvpe is an GNU extension I
see it should follow the specification of already defined exec* POSIX functions.
And according to POSIX [1], "argv[0] should point to a filename string that 
is associated with the process being started by one of the exec functions" (this
differ with man pages with uses the wording 'by convention' to define first
argument should point to process being stated).

So I see that:

  char *args[] = { NULL }; 
  execvpe (scriptname, args, NULL)

Which triggers the issue is invalid.
Paul Eggert Feb. 27, 2016, 11:17 p.m. UTC | #4
Adhemerval Zanella wrote:
> according to POSIX [1], "argv[0] should point to a filename string that
> is associated with the process being started by one of the exec functions"

Sure, but in this context the word "should" is talking about (as POSIX puts it) 
"a feature or behavior that is recommended programming practice for optimum 
portability."  A conforming application is not required to follow the 
recommendation, and in order to conform to POSIX glibc must support applications 
that do not follow the recommendation but otherwise conform.

My source for the above quote:

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap01.html#tag_01_05_06
Adhemerval Zanella Netto Feb. 29, 2016, 5:38 p.m. UTC | #5
On 27-02-2016 20:17, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> according to POSIX [1], "argv[0] should point to a filename string that
>> is associated with the process being started by one of the exec functions"
> 
> Sure, but in this context the word "should" is talking about (as POSIX puts it) "a feature or behavior that is recommended programming practice for optimum portability."  A conforming application is not required to follow the recommendation, and in order to conform to POSIX glibc must support applications that do not follow the recommendation but otherwise conform.
> 
> My source for the above quote:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap01.html#tag_01_05_06

Fair enough. I also noted uglibc also accepts the empty argument list.
I will change it.
diff mbox

Patch

diff --git a/posix/Makefile b/posix/Makefile
index 55f4f31..a2aabcc 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -82,6 +82,8 @@  tests		:= tstgetopt testfnm runtests runptests	     \
 		   tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
 		   tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
 		   tst-execvp3 tst-execvp4 tst-rfc3484 tst-rfc3484-2 \
+		   tst-execvpe1 tst-execvpe2 tst-execvpe3 tst-execvpe4 \
+		   tst-execvpe5 tst-execvpe6 \
 		   tst-rfc3484-3 \
 		   tst-getaddrinfo3 tst-fnmatch2 tst-cpucount tst-cpuset \
 		   bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \
@@ -229,6 +231,7 @@  tstgetopt-ARGS = -a -b -cfoobar --required foobar --optional=bazbug \
 
 tst-exec-ARGS = -- $(host-test-program-cmd)
 tst-exec-static-ARGS = $(tst-exec-ARGS)
+tst-execvpe5-ARGS = -- $(host-test-program-cmd)
 tst-spawn-ARGS = -- $(host-test-program-cmd)
 tst-spawn-static-ARGS = $(tst-spawn-ARGS)
 tst-dir-ARGS = `pwd` `cd $(common-objdir)/$(subdir); pwd` `cd $(common-objdir); pwd` $(objpfx)tst-dir
diff --git a/posix/execvpe.c b/posix/execvpe.c
index 61697a7..b945ef2 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -15,7 +15,6 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <alloca.h>
 #include <unistd.h>
 #include <stdarg.h>
 #include <stdbool.h>
@@ -23,15 +22,34 @@ 
 #include <string.h>
 #include <errno.h>
 #include <paths.h>
+#include <confstr.h>
+#include <sys/param.h>
 
+#ifndef PATH_MAX
+# ifdef MAXPATHLEN
+#  define PATH_MAX MAXPATHLEN
+# else
+#  define PATH_MAX 1024
+# endif
+#endif
 
 /* The file is accessible but it is not an executable file.  Invoke
    the shell to interpret it as a script.  */
 static void
-internal_function
-scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
+maybe_script_execute (const char *file, char *const argv[], char *const envp[])
 {
+  int argc = 0;
+  while (argv[argc++] != NULL)
+    {
+      if (argc == INT_MAX)
+	{
+	  errno = E2BIG;
+	  return;
+	}
+    }
+
   /* Construct an argument list for the shell.  */
+  char *new_argv[argc];
   new_argv[0] = (char *) _PATH_BSHELL;
   new_argv[1] = (char *) file;
   while (argc > 1)
@@ -39,6 +57,9 @@  scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
       new_argv[argc] = argv[argc - 1];
       --argc;
     }
+
+  /* Execute the shell.  */
+  __execve (new_argv[0], new_argv, envp);
 }
 
 
@@ -47,170 +68,109 @@  scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
 int
 __execvpe (const char *file, char *const argv[], char *const envp[])
 {
+  /* We check the simple case first. */
   if (*file == '\0')
     {
-      /* We check the simple case first. */
       __set_errno (ENOENT);
       return -1;
     }
 
+  /* Don't search when it contains a slash.  */
   if (strchr (file, '/') != NULL)
     {
-      /* Don't search when it contains a slash.  */
       __execve (file, argv, envp);
 
       if (errno == ENOEXEC)
-	{
-	  /* Count the arguments.  */
-	  int argc = 0;
-	  while (argv[argc++])
-	    ;
-	  size_t len = (argc + 1) * sizeof (char *);
-	  char **script_argv;
-	  void *ptr = NULL;
-	  if (__libc_use_alloca (len))
-	    script_argv = alloca (len);
-	  else
-	    script_argv = ptr = malloc (len);
-
-	  if (script_argv != NULL)
-	    {
-	      scripts_argv (file, argv, argc, script_argv);
-	      __execve (script_argv[0], script_argv, envp);
-
-	      free (ptr);
-	    }
-	}
+        maybe_script_execute (file, argv, envp);
+
+      return -1;
     }
-  else
+
+  const char *path = getenv ("PATH");
+  if (!path)
+    path = CS_PATH;
+  /* Although GLIBC does not enforce NAME_MAX, we set it as the maximum
+     size to avoid unbounded stack allocation.  Same applies for
+     PATH_MAX.  */
+  size_t file_len = __strnlen (file, NAME_MAX + 1);
+  size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
+
+  if ((file_len > NAME_MAX)
+      || !__libc_alloca_cutoff (path_len + file_len + 1))
     {
-      size_t pathlen;
-      size_t alloclen = 0;
-      char *path = getenv ("PATH");
-      if (path == NULL)
-	{
-	  pathlen = confstr (_CS_PATH, (char *) NULL, 0);
-	  alloclen = pathlen + 1;
-	}
-      else
-	pathlen = strlen (path);
+      errno = ENAMETOOLONG;
+      return -1;
+    }
 
-      size_t len = strlen (file) + 1;
-      alloclen += pathlen + len + 1;
+  const char *subp;
+  bool got_eacces = false;
+  for (const char *p = path; ; p = subp)
+    {
+      char buffer[path_len + file_len + 1];
 
-      char *name;
-      char *path_malloc = NULL;
-      if (__libc_use_alloca (alloclen))
-	name = alloca (alloclen);
-      else
-	{
-	  path_malloc = name = malloc (alloclen);
-	  if (name == NULL)
-	    return -1;
-	}
+      subp = __strchrnul (p, ':');
 
-      if (path == NULL)
+      /* PATH is larger than PATH_MAX and thus potentially larger than
+	 the stack allocation.  */
+      if (subp - p >= path_len)
 	{
-	  /* There is no `PATH' in the environment.
-	     The default search path is the current directory
-	     followed by the path `confstr' returns for `_CS_PATH'.  */
-	  path = name + pathlen + len + 1;
-	  path[0] = ':';
-	  (void) confstr (_CS_PATH, path + 1, pathlen);
+          /* If there is only one path, bail out.  */
+	  if (*subp == '\0')
+	    break;
+	  /* Otherwise skip to next one.  */
+	  continue;
 	}
 
-      /* Copy the file name at the top.  */
-      name = (char *) memcpy (name + pathlen + 1, file, len);
-      /* And add the slash.  */
-      *--name = '/';
+      /* Set current path considered plus a '/'.  */
+      memcpy (buffer, p, subp - p);
+      buffer[subp - p] = '/';
+      /* And the file to execute.  */
+      memcpy (buffer + (subp - p) + !!(subp > p), file, file_len + 1);
+
+      __execve (buffer, argv, envp);
 
-      char **script_argv = NULL;
-      void *script_argv_malloc = NULL;
-      bool got_eacces = false;
-      char *p = path;
-      do
+      if (errno == ENOEXEC)
+        maybe_script_execute (buffer, argv, envp);
+
+      switch (errno)
 	{
-	  char *startp;
-
-	  path = p;
-	  p = __strchrnul (path, ':');
-
-	  if (p == path)
-	    /* Two adjacent colons, or a colon at the beginning or the end
-	       of `PATH' means to search the current directory.  */
-	    startp = name + 1;
-	  else
-	    startp = (char *) memcpy (name - (p - path), path, p - path);
-
-	  /* Try to execute this name.  If it works, execve will not return. */
-	  __execve (startp, argv, envp);
-
-	  if (errno == ENOEXEC)
-	    {
-	      if (script_argv == NULL)
-		{
-		  /* Count the arguments.  */
-		  int argc = 0;
-		  while (argv[argc++])
-		    ;
-		  size_t arglen = (argc + 1) * sizeof (char *);
-		  if (__libc_use_alloca (alloclen + arglen))
-		    script_argv = alloca (arglen);
-		  else
-		    script_argv = script_argv_malloc = malloc (arglen);
-		  if (script_argv == NULL)
-		    {
-		      /* A possible EACCES error is not as important as
-			 the ENOMEM.  */
-		      got_eacces = false;
-		      break;
-		    }
-		  scripts_argv (startp, argv, argc, script_argv);
-		}
-
-	      __execve (script_argv[0], script_argv, envp);
-	    }
-
-	  switch (errno)
-	    {
-	    case EACCES:
-	      /* Record the we got a `Permission denied' error.  If we end
-		 up finding no executable we can use, we want to diagnose
-		 that we did find one but were denied access.  */
-	      got_eacces = true;
-	    case ENOENT:
-	    case ESTALE:
-	    case ENOTDIR:
-	      /* Those errors indicate the file is missing or not executable
-		 by us, in which case we want to just try the next path
-		 directory.  */
-	    case ENODEV:
-	    case ETIMEDOUT:
-	      /* Some strange filesystems like AFS return even
-		 stranger error numbers.  They cannot reasonably mean
-		 anything else so ignore those, too.  */
-	      break;
-
-	    default:
-	      /* Some other error means we found an executable file, but
-		 something went wrong executing it; return the error to our
-		 caller.  */
-	      return -1;
-	    }
+	  case EACCES:
+	  /* Record the we got a 'Permission denied' error.  If we end
+	     up finding no executable we can use, we want to diagnose
+	     that we did find one but were denied access.  */
+	    got_eacces = true;
+	  case ENOENT:
+	  case ESTALE:
+	  case ENOTDIR:
+	  /* Those errors indicate the file is missing or not executable
+	     by us, in which case we want to just try the next path
+	     directory.  */
+	  case ENODEV:
+	  case ETIMEDOUT:
+	  /* Some strange filesystems like AFS return even
+	     stranger error numbers.  They cannot reasonably mean
+	     anything else so ignore those, too.  */
+	    break;
+
+          default:
+	  /* Some other error means we found an executable file, but
+	     something went wrong executing it; return the error to our
+	     caller.  */
+	    return -1;
 	}
-      while (*p++ != '\0');
-
-      /* We tried every element and none of them worked.  */
-      if (got_eacces)
-	/* At least one failure was due to permissions, so report that
-	   error.  */
-	__set_errno (EACCES);
 
-      free (script_argv_malloc);
-      free (path_malloc);
+      if (*subp++ == '\0')
+	break;
     }
 
-  /* Return the error from the last attempt (probably ENOENT).  */
+  /* We tried every element and none of them worked.  */
+  if (got_eacces)
+    /* At least one failure was due to permissions, so report that
+       error.  */
+    __set_errno (EACCES);
+
   return -1;
+
 }
+
 weak_alias (__execvpe, execvpe)
diff --git a/posix/tst-execvp1.c b/posix/tst-execvp1.c
index ecc673d..fe98ce5 100644
--- a/posix/tst-execvp1.c
+++ b/posix/tst-execvp1.c
@@ -3,6 +3,10 @@ 
 #include <stdlib.h>
 #include <unistd.h>
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
 static int
 do_test (void)
 {
@@ -19,7 +23,7 @@  do_test (void)
 
   char *argv[] = { (char *) "does-not-exist", NULL };
   errno = 0;
-  execvp (argv[0], argv);
+  EXECVP (argv[0], argv);
 
   if (errno != ENOENT)
     {
diff --git a/posix/tst-execvp2.c b/posix/tst-execvp2.c
index 7e0f5d8..9be8733 100644
--- a/posix/tst-execvp2.c
+++ b/posix/tst-execvp2.c
@@ -14,6 +14,9 @@  static int do_test (void);
 #define TEST_FUNCTION do_test ()
 #include "../test-skeleton.c"
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv)  execvp (__file, __argv)
+#endif
 
 static char *copy;
 
@@ -70,7 +73,7 @@  do_test (void)
 
   char *argv[] = { basename (copy), NULL };
   errno = 0;
-  execvp (argv[0], argv);
+  EXECVP (argv[0], argv);
 
   if (errno != EACCES)
     {
diff --git a/posix/tst-execvp3.c b/posix/tst-execvp3.c
index 5ebc879..43f7c34 100644
--- a/posix/tst-execvp3.c
+++ b/posix/tst-execvp3.c
@@ -12,6 +12,9 @@  static int do_test (void);
 
 #include "../test-skeleton.c"
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv)  execvp (__file, __argv)
+#endif
 
 static char *fname;
 
@@ -35,7 +38,7 @@  do_test (void)
     }
 
   char *argv[] = { fname, NULL };
-  execvp (basename (fname), argv);
+  EXECVP (basename (fname), argv);
 
   /* If we come here, the execvp call failed.  */
   return 1;
diff --git a/posix/tst-execvp4.c b/posix/tst-execvp4.c
index 531fab2..116624f 100644
--- a/posix/tst-execvp4.c
+++ b/posix/tst-execvp4.c
@@ -5,6 +5,10 @@ 
 #include <unistd.h>
 #include <sys/stat.h>
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv)  execvp (__file, __argv)
+#endif
+
 static int
 do_test (void)
 {
@@ -27,7 +31,7 @@  do_test (void)
 
   unsetenv ("PATH");
   char *argv[] = { buf + 9, NULL };
-  execvp (argv[0], argv);
+  EXECVP (argv[0], argv);
   return 0;
 }
 
diff --git a/posix/tst-execvpe1.c b/posix/tst-execvpe1.c
new file mode 100644
index 0000000..622eb79
--- /dev/null
+++ b/posix/tst-execvpe1.c
@@ -0,0 +1,20 @@ 
+/* Check ENOENT failure for execvpe.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp1.c>
diff --git a/posix/tst-execvpe2.c b/posix/tst-execvpe2.c
new file mode 100644
index 0000000..cdec09a
--- /dev/null
+++ b/posix/tst-execvpe2.c
@@ -0,0 +1,20 @@ 
+/* Check EACCES for execvpe.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp2.c>
diff --git a/posix/tst-execvpe3.c b/posix/tst-execvpe3.c
new file mode 100644
index 0000000..47cdc14
--- /dev/null
+++ b/posix/tst-execvpe3.c
@@ -0,0 +1,20 @@ 
+/* Check script execution without shebang for execvpe.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp3.c>
diff --git a/posix/tst-execvpe4.c b/posix/tst-execvpe4.c
new file mode 100644
index 0000000..e8883aa
--- /dev/null
+++ b/posix/tst-execvpe4.c
@@ -0,0 +1,20 @@ 
+/* Check unexistent binary for execvpe.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp4.c>
diff --git a/posix/tst-execvpe5.c b/posix/tst-execvpe5.c
new file mode 100644
index 0000000..81085ba
--- /dev/null
+++ b/posix/tst-execvpe5.c
@@ -0,0 +1,157 @@ 
+/* General tests for execpve.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <wait.h>
+
+
+/* Nonzero if the program gets called via `exec'.  */
+static int restart;
+
+
+#define CMDLINE_OPTIONS \
+  { "restart", no_argument, &restart, 1 },
+
+/* Prototype for our test function.  */
+extern void do_prepare (int argc, char *argv[]);
+extern int do_test (int argc, char *argv[]);
+
+#include "../test-skeleton.c"
+
+#define EXECVPE_KEY    "EXECVPE_ENV"
+#define EXECVPE_VALUE  "execvpe_test"
+
+
+static int
+handle_restart (void)
+{
+  /* First check if only one variable is passed on execvpe.  */
+  int env_count = 0;
+  for (char **e = environ; *e != NULL; ++e)
+    if (++env_count == INT_MAX)
+      {
+	printf ("Environment variable number overflow");
+	exit (EXIT_FAILURE);
+      }
+  if (env_count != 1)
+    {
+      printf ("Wrong number of environment variables");
+      exit (EXIT_FAILURE);
+    }
+
+  /* Check if the combinarion os "EXECVPE_ENV=execvpe_test"  */
+  const char *env = getenv (EXECVPE_KEY);
+  if (env == NULL)
+    {
+      printf ("Test environment variable not found");
+      exit (EXIT_FAILURE);
+    }
+
+  if (strncmp (env, EXECVPE_VALUE, sizeof (EXECVPE_VALUE)))
+    {
+      printf ("Test environment variable with wrong value");
+      exit (EXIT_FAILURE);
+    }
+
+  return 0;
+}
+
+
+int
+do_test (int argc, char *argv[])
+{
+  pid_t pid;
+  int status;
+
+  /* We must have
+     - one or four parameters left if called initially
+       + path for ld.so		optional
+       + "--library-path"	optional
+       + the library path	optional
+       + the application name
+  */
+
+  if (restart)
+    {
+      if (argc != 1)
+	{
+	  printf ("Wrong number of arguments (%d)", argc);
+	  exit (EXIT_FAILURE);
+	}
+
+      return handle_restart ();
+    }
+
+  if (argc != 2 && argc != 5)
+    {
+      printf ("Wrong number of arguments (%d)", argc);
+      exit (EXIT_FAILURE);
+    }
+
+  /* We want to test the `execvpe' function.  To do this we restart the
+     program with an additional parameter.  */
+  pid = fork ();
+  if (pid == 0)
+    {
+      /* This is the child.  Construct the command line.  */
+
+      /* We cast here to char* because the test itself does not modify neither
+	 the argument nor the environment list.  */
+      char *envs[] = { (char*)(EXECVPE_KEY "=" EXECVPE_VALUE), NULL };
+      if (argc == 5)
+	{
+	  char *args[] = { argv[1], argv[2], argv[3], argv[4],
+			   (char *) "--direct", (char *) "--restart", NULL };
+	  execvpe (args[0], args, envs);
+	}
+      else
+	{
+	  char *args[] = { argv[1], argv[1],
+			   (char *) "--direct", (char *) "--restart", NULL };
+	  execvpe (args[0], args, envs);
+	}
+
+      printf ("Cannot exec");
+      exit (EXIT_FAILURE);
+    }
+  else if (pid == (pid_t) -1)
+    {
+      printf ("Cannot fork");
+      exit (EXIT_FAILURE);
+    }
+
+  /* Wait for the child.  */
+  if (waitpid (pid, &status, 0) != pid)
+    {
+      printf ("Wrong child");
+      exit (EXIT_FAILURE);
+    }
+
+  if (WTERMSIG (status) != 0)
+    {
+      printf ("Child terminated incorrectly");
+      exit (EXIT_FAILURE);
+    }
+  status = WEXITSTATUS (status);
+
+  return status;
+}
diff --git a/posix/tst-execvpe6.c b/posix/tst-execvpe6.c
new file mode 100644
index 0000000..4551e2c
--- /dev/null
+++ b/posix/tst-execvpe6.c
@@ -0,0 +1,82 @@ 
+/* Check execvpe script argument handling.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/param.h>
+
+static char *fname;
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+
+#include "../test-skeleton.c"
+
+static void
+do_prepare (void)
+{
+  int fd = create_temp_file ("testscript", &fname);
+  dprintf (fd, "echo foo\n");
+  fchmod (fd, 0700);
+  close (fd);
+}
+
+static int
+do_test (void)
+{
+  if  (setenv ("PATH", test_dir, 1) != 0)
+    {
+      puts ("setenv failed");
+      return 1;
+    }
+
+  /* To limit stack allocation for argument construction in case of
+     script without shebang execvpe limits total number of argument
+     to NCARGS.  */
+  const size_t max_args = NCARGS + 1;
+  char **args = malloc ((NCARGS + 1) * sizeof (char*));
+  if (args == NULL)
+    {
+      puts ("malloc failed");
+      return 1;
+    }
+
+  args[0] = fname;
+  for (int i = 1; i < max_args; ++i)
+    args[i] = strdup ("a");
+  args[max_args] = NULL;
+
+  execvpe (basename (fname), args, NULL);
+
+  for (int i = 1; i < max_args; ++i)
+    free (args[i]);
+  free (args);
+
+  if (errno != E2BIG)
+    {
+      printf ("errno = %d (%m), expected E2BIG\n", errno);
+      return 1;
+    }
+
+  return 0;
+}