"exec PROGRAM LIST" does not work with empty list #16075

p5pRT · 2017-07-10T12:13:10Z

Migrated from rt.perl.org#131730 (status was 'resolved')

Searchable as RT131730$

p5pRT · 2017-07-10T12:13:10Z

From @pali

When exec is called with empty list, either via explicit () or implicit,
from empty array variable, then it always fail and set $! to empty
string. See examples:

$ perl -e 'exec {"binary"} () or die "exec failed: \"$!\"\n";'
exec failed: ""

$ perl -e 'my @args; exec {"binary"} @args or die "exec failed: \"$!\"\n";'
exec failed: ""

Here is patch which fixes it:

Inline Patch

diff --git a/doio.c b/doio.c
index 6f4cd84f8c..32f6e3416f 100644
--- a/doio.c
+++ b/doio.c
@@ -1583,7 +1583,7 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
 #if defined(__SYMBIAN32__) || defined(__LIBCATAMOUNT__)
     Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system");
 #else
-    if (sp > mark) {
+    if (sp >= mark) {
 	const char **a;
 	const char *tmps = NULL;
 	Newx(PL_Argv, sp - mark + 1, const char*);

With above patch it is working fine:

perl -e 'exec {"true"} () or die "exec failed: \"$!\"\n";'; echo $?
0

perl -e 'exec {"false"} () or die "exec failed: \"$!\"\n";'; echo $?
1

p5pRT · 2017-07-10T13:58:10Z

From @ilmari

(via RT) <perlbug-followup@perl.org> writes:

When exec is called with empty list, either via explicit () or implicit,
from empty array variable, then it always fail and set $! to empty
string. See examples:

$ perl -e 'exec {"binary"} () or die "exec failed: \"$!\"\n";'
exec failed: ""

$ perl -e 'my @args; exec {"binary"} @args or die "exec failed: \"$!\"\n";'
exec failed: ""

Here is patch which fixes it:

diff --git a/doio.c b/doio.c
index 6f4cd84f8c..32f6e3416f 100644
--- a/doio.c
+++ b/doio.c
@@ -1583,7 +1583,7 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
#if defined(__SYMBIAN32__) || defined(__LIBCATAMOUNT__)
Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system");
#else
- if (sp > mark) {
+ if (sp >= mark) {
const char **a;
const char *tmps = NULL;
Newx(PL_Argv, sp - mark + 1, const char*);

With above patch it is working fine:

perl -e 'exec {"true"} () or die "exec failed: \"$!\"\n";'; echo $?
0

perl -e 'exec {"false"} () or die "exec failed: \"$!\"\n";'; echo $?
1

This causes calling execvp() with an empty argv array, which is not
strictly POSIX-conforming (it "should point to a filename"), and might
break programs that expect to be able to access argv[0] without checking
argc, but I agree perl should still allow it.

However, with this patch "exec {''} ()" and "exec()" segfault a bit
further down, trying to dereference PL_Argv[0], which is the terminating
NULL pointer.

exec() crashes on line 1601 while exec {''} () crashes on line 1608:

if (really)
tmps = SvPV_nolen_const(really);
1601: if ((!really && *PL_Argv[0] != '/') ||
[...]
if (really && *tmps) {
PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv));
} else {
1608: PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv));
}

It gets to that path insted of the "really" branch, because tmps is an
empty string so, *tmps is the terminating NUL byte (this behaviour of
ignoring the return value of the block if it's empty is not documented,
but has been there since perl 3.0).

I propose explicitly returning ENOENT (which is what execvp() returns if
the file name is an empty string) in both of the above failing cases,
i.e. something like this:

Inline Patch

diff --git a/doio.c b/doio.c
index 6f4cd84f8c..cbaf7e6a33 100644
--- a/doio.c
+++ b/doio.c
@@ -1583,7 +1583,7 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
 #if defined(__SYMBIAN32__) || defined(__LIBCATAMOUNT__)
     Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system");
 #else
-    if (sp > mark) {
+    if (sp >= mark) {
 	const char **a;
 	const char *tmps = NULL;
 	Newx(PL_Argv, sp - mark + 1, const char*);
@@ -1598,17 +1598,19 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
 	*a = NULL;
 	if (really)
 	    tmps = SvPV_nolen_const(really);
-	if ((!really && *PL_Argv[0] != '/') ||
+	if ((!really && PL_Argv[0] && *PL_Argv[0] != '/') ||
 	    (really && *tmps != '/'))		/* will execvp use PATH? */
 	    TAINT_ENV();		/* testing IFS here is overkill, probably */
 	PERL_FPU_PRE_EXEC
 	if (really && *tmps) {
             PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv));
-	} else {
+	} else if (PL_Argv[0]) {
             PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv));
-	}
+	} else {
+            SETERRNO(ENOENT,LIB_INVARG);
+	}
 	PERL_FPU_POST_EXEC
-	S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0]), fd, do_report);
+	S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0] ? PL_Argv[0] : ""), fd, do_report);
     }
     do_execfree();
 #endif

- ilmari
-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.               - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.                      - Calle Dybedahl

p5pRT · 2017-07-10T13:58:10Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2017-07-13T08:53:40Z

From @pali

On Monday 10 July 2017 06:58:10 Dagfinn Ilmari Mannsåker via RT wrote:

(via RT) <perlbug-followup@perl.org> writes:

When exec is called with empty list, either via explicit () or implicit,
from empty array variable, then it always fail and set $! to empty
string. See examples:

$ perl -e 'exec {"binary"} () or die "exec failed: \"$!\"\n";'
exec failed: ""

$ perl -e 'my @args; exec {"binary"} @args or die "exec failed: \"$!\"\n";'
exec failed: ""

Here is patch which fixes it:

diff --git a/doio.c b/doio.c
index 6f4cd84f8c..32f6e3416f 100644
--- a/doio.c
+++ b/doio.c
@@ -1583,7 +1583,7 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
#if defined(__SYMBIAN32__) || defined(__LIBCATAMOUNT__)
Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system");
#else
- if (sp > mark) {
+ if (sp >= mark) {
const char **a;
const char *tmps = NULL;
Newx(PL_Argv, sp - mark + 1, const char*);

With above patch it is working fine:

perl -e 'exec {"true"} () or die "exec failed: \"$!\"\n";'; echo $?
0

perl -e 'exec {"false"} () or die "exec failed: \"$!\"\n";'; echo $?
1

This causes calling execvp() with an empty argv array, which is not
strictly POSIX-conforming (it "should point to a filename"),

I cannot currently find POSIX requirement that first arg cannot be NULL.

and might
break programs that expect to be able to access argv[0] without checking
argc, but I agree perl should still allow it.

But, in ISO C89, ISO C99 and also ISO C11, main's argc can be zero. In
all those ISO C standards is written:

* The value of argc shall be nonnegative.
* argv[argc] shall be a null pointer.

However, with this patch "exec {''} ()" and "exec()" segfault a bit
further down, trying to dereference PL_Argv[0], which is the terminating
NULL pointer.

Ouh, I have not caught this.

exec() crashes on line 1601 while exec {''} () crashes on line 1608:
    if $really$
         tmps = SvPV\_nolen\_const$really$;
1601: if ((!really && *PL_Argv[0] != '/') ||
[...]
if (really && *tmps) {
PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv));
} else {
1608: PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv));
}

It gets to that path insted of the "really" branch, because tmps is an
empty string so, *tmps is the terminating NUL byte (this behaviour of
ignoring the return value of the block if it's empty is not documented,
but has been there since perl 3.0).

I propose explicitly returning ENOENT (which is what execvp() returns if
the file name is an empty string) in both of the above failing cases,
i.e. something like this:

Looks good, thanks for update!

diff --git a/doio.c b/doio.c
index 6f4cd84f8c..cbaf7e6a33 100644
--- a/doio.c
+++ b/doio.c
@@ -1583,7 +1583,7 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
#if defined(__SYMBIAN32__) || defined(__LIBCATAMOUNT__)
Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system");
#else
- if (sp > mark) {
+ if (sp >= mark) {
const char **a;
const char *tmps = NULL;
Newx(PL_Argv, sp - mark + 1, const char*);
@@ -1598,17 +1598,19 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
*a = NULL;
if (really)
tmps = SvPV_nolen_const(really);
- if ((!really && *PL_Argv[0] != '/') ||
+ if ((!really && PL_Argv[0] && *PL_Argv[0] != '/') ||
(really && *tmps != '/')) /* will execvp use PATH? */
TAINT_ENV(); /* testing IFS here is overkill, probably */
PERL_FPU_PRE_EXEC
if (really && *tmps) {
PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv));
- } else {
+ } else if (PL_Argv[0]) {
PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv));
- }
+ } else {
+ SETERRNO(ENOENT,LIB_INVARG);
+ }
PERL_FPU_POST_EXEC
- S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0]), fd, do_report);
+ S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0] ? PL_Argv[0] : ""), fd, do_report);
}
do_execfree();
#endif

- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

p5pRT · 2017-07-13T17:14:41Z

From @ilmari

pali@cpan.org writes:

On Monday 10 July 2017 06:58:10 Dagfinn Ilmari Mannsåker via RT wrote:

This causes calling execvp() with an empty argv array, which is not
strictly POSIX-conforming (it "should point to a filename"),

I cannot currently find POSIX requirement that first arg cannot be NULL.

http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html says:

| The value in argv[0] should point to a filename string that is
| associated with the process being started by one of the exec
| functions.

It's only a "should", not a "must", so the requirement only applies to
Strictly Conforming POSIX Applications, which we can't require perl
scripts to be.

and might
break programs that expect to be able to access argv[0] without checking
argc, but I agree perl should still allow it.

But, in ISO C89, ISO C99 and also ISO C11, main's argc can be zero. In
all those ISO C standards is written:

* The value of argc shall be nonnegative.
* argv[argc] shall be a null pointer.

The RATIONALE section of the POSIX exec() man page explains the history:

| Early proposals required that the value of argc passed to main() be "one
| or greater". This was driven by the same requirement in drafts of the
| ISO C standard. In fact, historical implementations have passed a value
| of zero when no arguments are supplied to the caller of the exec
| functions. This requirement was removed from the ISO C standard and
| subsequently removed from this volume of POSIX.1-2008 as well. The
| wording, in particular the use of the word should, requires a Strictly
| Conforming POSIX Application to pass at least one argument to the exec
| function, thus guaranteeing that argc be one or greater when invoked by
| such an application. In fact, this is good practice, since many existing
| applications reference argv[0] without first checking the value of argc.

[…]

I propose explicitly returning ENOENT (which is what execvp() returns if
the file name is an empty string) in both of the above failing cases,
i.e. something like this:

Looks good, thanks for update!

I'm going on holiday for a week today, but unless anyone objects (or
beats me to it) I'll write some tests and push this when I get back.

diff --git a/doio.c b/doio.c
index 6f4cd84f8c..cbaf7e6a33 100644
--- a/doio.c
+++ b/doio.c
@@ -1583,7 +1583,7 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
#if defined(__SYMBIAN32__) || defined(__LIBCATAMOUNT__)
Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system");
#else
- if (sp > mark) {
+ if (sp >= mark) {
const char **a;
const char *tmps = NULL;
Newx(PL_Argv, sp - mark + 1, const char*);
@@ -1598,17 +1598,19 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp,
*a = NULL;
if (really)
tmps = SvPV_nolen_const(really);
- if ((!really && *PL_Argv[0] != '/') ||
+ if ((!really && PL_Argv[0] && *PL_Argv[0] != '/') ||
(really && *tmps != '/')) /* will execvp use PATH? */
TAINT_ENV(); /* testing IFS here is overkill, probably */
PERL_FPU_PRE_EXEC
if (really && *tmps) {
PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv));
- } else {
+ } else if (PL_Argv[0]) {
PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv));
- }
+ } else {
+ SETERRNO(ENOENT,LIB_INVARG);
+ }
PERL_FPU_POST_EXEC
- S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0]), fd, do_report);
+ S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0] ? PL_Argv[0] : ""), fd, do_report);
}
do_execfree();
#endif

- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law

p5pRT · 2017-08-09T12:55:22Z

From @pali

On Thursday 13 July 2017 10:14:41 Dagfinn Ilmari Mannsåker via RT wrote:

I'm going on holiday for a week today, but unless anyone objects (or
beats me to it) I'll write some tests and push this when I get back.

Hi! If you are back, can you process this patch?

p5pRT · 2017-10-02T17:44:15Z

From @pali

Hi! I would like to remind patch in this ticket.

p5pRT · 2017-10-18T17:59:12Z

From @ilmari

pali@cpan.org writes:

On Thursday 13 July 2017 10:14:41 Dagfinn Ilmari Mannsåker via RT wrote:

I'm going on holiday for a week today, but unless anyone objects (or
beats me to it) I'll write some tests and push this when I get back.

Hi! If you are back, can you process this patch?

Sorry for the delay. I've now merged the patch, and it'll be in the
upcoming 5.27.5 development release.

- ilmari
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law

p5pRT · 2017-10-21T02:29:28Z

From @jkeenan

Since we haven't observed any test failures since Ilmari committed the patch, I'm marking this ticket Resolved.
--
James E Keenan (jkeenan@cpan.org)

p5pRT · 2017-10-21T02:29:29Z

@jkeenan - Status changed from 'open' to 'resolved'

p5pRT closed this as completed Oct 21, 2017

p5pRT added the Severity Low label Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"exec PROGRAM LIST" does not work with empty list #16075

"exec PROGRAM LIST" does not work with empty list #16075

p5pRT commented Jul 10, 2017

p5pRT commented Jul 10, 2017

p5pRT commented Jul 10, 2017

p5pRT commented Jul 10, 2017

p5pRT commented Jul 13, 2017

p5pRT commented Jul 13, 2017

p5pRT commented Aug 9, 2017

p5pRT commented Oct 2, 2017

p5pRT commented Oct 18, 2017

p5pRT commented Oct 21, 2017

p5pRT commented Oct 21, 2017

"exec PROGRAM LIST" does not work with empty list #16075

"exec PROGRAM LIST" does not work with empty list #16075

Comments

p5pRT commented Jul 10, 2017

p5pRT commented Jul 10, 2017

From @pali

p5pRT commented Jul 10, 2017

From @ilmari

p5pRT commented Jul 10, 2017

p5pRT commented Jul 13, 2017

From @pali

p5pRT commented Jul 13, 2017

From @ilmari

p5pRT commented Aug 9, 2017

From @pali

p5pRT commented Oct 2, 2017

From @pali

p5pRT commented Oct 18, 2017

From @ilmari

p5pRT commented Oct 21, 2017

From @jkeenan

p5pRT commented Oct 21, 2017