calling os.execlp('true') with the wrong number of arguments (missing 2nd arg), the interpreter crashes. fixed in 3.x, this is a backport of the patch to 2.x

Looks ok, but I think it needs a test, and it looks like execve suffers from the same problem.

As far as I can tell, it does not *crash* the interpreter.  Instead, it replaces the python interpreter process with a "true" utility.

$ ./python.exe 
Python 2.7a4+ (trunk:78816M, Mar  9 2010, 18:57:13) 
[GCC 4.2.1 (Apple Inc. build 5646) (dot 1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import os; os.execlp('true')
$ echo $?
0
$ ./python.exe 
Python 2.7a4+ (trunk:78816M, Mar  9 2010, 18:57:13) 
[GCC 4.2.1 (Apple Inc. build 5646) (dot 1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import os; os.execlp('false')
air:trunk sasha$ echo $?
1

I forgot to add that this behavior is documented:

os.execlp = execlp(file, *args)
    execlp(file, *args)
    
    Execute the executable file (which is searched for along $PATH)
    with argument list args, replacing the current process.

The original report, issue1039 has a better problem description:

"In a windows debug build, an assertion is triggered when os.execvpe is
called with an empty argument list:

self.assertRaises(OSError, os.execvpe, 'no such app-', [], None)"

The patch, file8338/os.diff is complete with a test and should be easy to apply to the trunk.

I would be -0 on the change.  On one hand, POSIX seems to require a non-empty argument list, on the other hand at least Linux and MacOS X appear to be happy with os.execlp('true').  Furthemore, the proposed change will change the exception from os.execlp('no such program') from OSError to ValueError, which is not a backward compatible change.  As such it is certainly not appropriate for bug-fix releases.

As far as 3x series are concerned, I think ValueError exception should be documented.

Adding Thomas.

Thomas,

Do you remember why your patch for issue1039 was not backported?

it does crash:

$ python
Python 2.6.5rc2 (r265rc2:78822, Mar 11 2010, 13:01:50) 
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.execlp('true')
Segmentation fault (core dumped)

arg[0] (the command name) must be part of the second parameter.


side notice: the execlpe looks inconsistent (both 2.x and 3.x).

>>> os.execlpe('true')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.6/os.py", line 335, in execlpe
    env = args[-1]
IndexError: tuple index out of range

shouldn't this be a ValueError as well?

On Tue, Mar 16, 2010 at 7:46 PM, Matthias Klose <report@bugs.python.org> wrote:
..
> it does crash:

Strange.  Can you report your Linux version and the stack trace from the crash?

On Ubuntu, 2.6.24-27-generic #1 SMP Wed Jan 27 23:54:28 UTC 2010 i686
GNU/Linux, I see no crash.

Moreover, I've just checked that the following C program works just fine:

#include <unistd.h>

int
main()
{
  execlp("ls", NULL);
}

It does not crash on Gentoo, either:

rdmurray@maestro:~>uname -a
Linux maestro 2.6.31-gentoo-r3 #1 SMP PREEMPT Thu Oct 22 20:13:19 EDT 2009 i686 Intel(R) Core(TM)2 CPU 6600 @ 2.40GHz GenuineIntel GNU/Linux

rdmurray@maestro:~/python/release26-maint>./python
Python 2.6.5rc2 (release26-maint:79008, Mar 16 2010, 20:29:24) 
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
[36856 refs]
>>> os.execlp('false')
rdmurray@maestro:~/python/release26-maint>echo $?
1

crash seen on both Debian unstable and recent Ubuntu lucid.

(gdb) run
Starting program: /home/doko/a.out 
process 30155 is executing new program: /bin/ls
[Thread debugging using libthread_db enabled]

Program received signal SIGSEGV, Segmentation fault.
strrchr () at ../sysdeps/i386/strrchr.S:178
178	../sysdeps/i386/strrchr.S: No such file or directory.
	in ../sysdeps/i386/strrchr.S
(gdb) bt
#0  strrchr () at ../sysdeps/i386/strrchr.S:178
#1  0x080524d2 in ?? ()
#2  0x0804fb2c in ?? ()
#3  0x00170bd6 in __libc_start_main (main=0x804fb10, argc=0, 
    ubp_av=0xbffff534, init=0x805bd60, fini=0x805bd50, 
    rtld_fini=0x11e0b0 <_dl_fini>, stack_end=0xbffff52c) at libc-start.c:226
#4  0x08049cf1 in ?? ()

stack trace from lucid; glibc-2.11.1, linux 2.6.32

Crash reproduced under Mandriva Linux 2010.0 (glibc-2.10.1-6.2mnb2). Stack trace is as follows:

(gdb) bt
#0  0x00007fb59f2eba9a in strrchr () from /lib64/libc.so.6
#1  0x00000000004010ae in ?? ()
#2  0x0000000000400ff3 in ?? ()
#3  0x00007fb59f29091d in __libc_start_main () from /lib64/libc.so.6
#4  0x0000000000400d79 in ?? ()
#5  0x00007fff00162fa8 in ?? ()
#6  0x000000000000001c in ?? ()
#7  0x0000000000000000 in ?? ()

Also, please add the missing test when backporting the patch.

> Thomas,
> 
> Do you remember why your patch for issue1039 was not backported?

Probably an oversight only.

I agree that this should be fixed, since we presumably want to be "strictly conforming" to the posix standards, but it looks like this is a regression in either linux or glibc.  From the standard's rational section:

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 IEEE Std 1003.1-2001 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.

On Thu, Mar 18, 2010 at 3:32 PM, R. David Murray <report@bugs.python.org> wrote:
..
> I agree that this should be fixed, since we presumably want to be "strictly conforming" to the posix standards,
> but it looks like this is a regression in either linux or glibc.  From the standard's rational section:
>

Let me add just make a few observations without drawing a conclusion:

1.  This is not a crash of python.  As far as I can tell, it is the
executed program that crashes attempting to dereference argv[0]
without checking argc first.   On the platforms that support execution
with argc=0, this is a bug in the application or in the C library.

2. Currently 3.x and 2.x python throw different exceptions from
os.execlp('xyz') if xyz does not exist.   Maybe os.execlp(one_arg)
should raise OSError instead of ValueError.

3. Another alternative would be to change the signature from
os.execlp(path, ...) to os.execlp(path, name, ...) and make
os.execlp() raise TypeError if name is not supplied.   This is an
attractive possibility because it can be done by a trivial change in
os.py while preserving the ability to exec with argc=0 for those who
know what they are doing.

commited to the trunk, commit to the 2.6 branch pending

committed to the 2.6 branch as well

I believe that backporting this change to 2.6 is inappropriate.  It will more than likely cause perfectly correct code to stop working, and that is not something we like to do in a maintenance release.

I believe that the bug on the debian/ubuntu side is actually that /bin/true crashes if argc is 0.  Someone with a debian or ubuntu system should test this.

In fact, reviewing this issue again after Alexander's post to python-dev, I don't think that this fix in its current form is advisable at all.  It makes Python's execv function less flexible than the underlying system API, and more importantly changes its behavior in a backwards incompatible way.  Python 3 is a different story in this regard, since the change went in before 3.1, and as noted it makes Python "strictly compliant" with the posix standard, which seems like an acceptable reason to decrease flexibility.

Since it does trigger a crash on the windows equivalent API, the check should be conditional on platform.  And it should generate a py3k warning on other platforms.

I'm open to an argument that the API can be changed in 2.7, although I don't think we normally do that without a deprecation first.  But I think there is no question that this change in its current form needs to be backed out of 2.6.

> this change in its current form needs to be backed out of 2.6

done.

I'll check for uses of execlp and execlpe.

how should the divergency of execlp (raises ValueError), and execlpe (raises IndexError) be handled?

Hello

Could you please write the revision number when you speak about a commit? Text like “fixed in r4253” will become an helpful link.

Thanks

reverted in r79190 on the 2.6 branch

Please see related issue8191.

If this new feature stays in 3.x, shouldn't 2.7 have a -3 warning?

Also, I would consider adding os.execlp(path) -> os.execlp(path, os.path.basename(path)) transformation to 2to3.

I noticed that the change is still present in 2.7a.  For what it's worth, I agree with David:

"""
Since it does trigger a crash on the windows equivalent API, the check should be conditional on platform.  And it should generate a py3k warning on other platforms.
"""

I am changing classification to "feature request" and adding "Windows" to the list of components because if I understand David correctly, the title of the issue is valid only on Windows platform.  (On other platforms os.execlp(path) may cause a crash of the program found at path, but not a crash of python interpreter itself.)  I also remove 2.6 from the list of versions since after r79190 no further action is contemplated for 2.6 branch.

OK, this went in to 2.7 without the OS conditional, and there has been no great hue and cry, so I guess it was safe enough :)

As for the difference in error message between execlp and execlpe, I think that's fine.  The execlpe index error message is accurate: execlpe requires an 'env' arg, and it was missing.