classification
Title: test_str.py crashes
Type: crash Stage:
Components: Tests Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: loewis Nosy List: donmez, gregory.p.smith, gvanrossum, loewis, theller
Priority: normal Keywords:

Created on 2007-12-13 10:09 by donmez, last changed 2008-01-09 17:46 by gvanrossum. This issue is now closed.

Files
File name Uploaded Description Edit
backtrace.txt donmez, 2007-12-13 10:12
valgrind.txt donmez, 2007-12-13 10:21
valgrind-supp.txt donmez, 2007-12-13 18:08
fwrapv.patch donmez, 2007-12-13 19:55
wrap.patch donmez, 2007-12-13 20:26
wrap.patch donmez, 2007-12-13 20:28
Messages (35)
msg58525 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 10:09
Checkout Python 2.5 from release25-maint branch, revision 59479

Compiled with gcc 4.3.0 20071212 , make test crashes with the following
output

[... snip ...]
test_socket_ssl
test_socket_ssl skipped -- Use of the `network' resource not enabled
test_socketserver
test_socketserver skipped -- Use of the `network' resource not enabled
test_softspace
test_sort
test_sqlite
test_startfile
test_startfile skipped -- cannot import name startfile
test_str
make: *** [test] Segmentation fault
msg58526 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 10:12
gdb backtrace, segfaulting test is Lib/test/test_str.py
msg58529 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 10:21
Valgrind output, shows lots of invalid reads.
msg58540 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 17:57
What hardware and OS?  32 or 64 bit?  What optimization level?  Debug
build or not?

NB. Unless you used /Misc/valgrind-python.supp, the valgrind output is
useless.
msg58546 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 18:08
Linux 2.6.18, x86, 32bit .

Executed valgrind with 

valgrind  --suppressions=./Misc/valgrind-python.supp -v ./python
./Lib/test/test_str.py

attached as valgrind-supp.txt it still shows lots of invalid reads.

Optimization level is -g -O3 which seems to be default as I didn't
specify CFLAGS.
msg58549 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 18:22
--enable-pydebug fixes the crash it might be that some uninitialized
variable doesn't take affect unless optimized as valgrind output shows
many of this.
msg58550 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 18:23
Looks like expandtabs() has a problem.  Can you boil it down to a single
call?
msg58551 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 18:23
BTW is this a released version of GCC?  If not, you might want to file
the bug with the GCC project...
msg58555 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 18:38
This is a soon to be released GCC though I won't deny it has
regressions, but note that extra optimizations already uncovered bugs in
other software.

And unless I can get a minimal C testcase, GCC bug will be worthless.

Exact crashling call is string_tests.py line 255 :

self.checkraises(OverflowError,
                             '\ta\n\tb', 'expandtabs', sys.maxint)

Commenting out this fixes the crash.
msg58558 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 19:01
> This is a soon to be released GCC though I won't deny it has
> regressions, but note that extra optimizations already uncovered bugs in
> other software.

And the GCC authors always win these cases, C standard in hand.

> And unless I can get a minimal C testcase, GCC bug will be worthless.
>
> Exact crashling call is string_tests.py line 255 :
>
> self.checkraises(OverflowError,
>                              '\ta\n\tb', 'expandtabs', sys.maxint)
>
> Commenting out this fixes the crash.

If you want for me to debug this myself it'll be ages. it looks like
the crashing call is

'\ta\n\tb'.expandtabs(2147483647)

Can you confirm that this crashes? If it does, you should be able to
use gdb to step through expandtabs() and hopefully analyze the
problem.
msg58560 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 19:06
Actually, looking at the sample code and the string_expandtabs()
implementation it's clear what happened: the test for overflow on line
3318 or 3331 or 3339 must have been optimized out by GCC.

This is very inconvenient because lots of buffer overflow protection
uses similar code; this means that code that has been audited and fixed
in the past will again be vulnerable after compilation by GCC 4.3.

I'm going to ask Martin von Loewis to give an opinion on this.

Thanks for bringing this up!
msg58561 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:07
Indeed you are correct,

>>> '\ta\n\tb'.expandtabs(2147483647)

Program received signal SIGSEGV, Segmentation fault.
string_expandtabs (self=0xb7ba7c60, args=0xb7ba7dec) at
Objects/stringobject.c:3358
3358                        *q++ = ' ';
(gdb) bt
#0  string_expandtabs (self=0xb7ba7c60, args=0xb7ba7dec) at
Objects/stringobject.c:3358
#1  0xb7e1b6dd in PyCFunction_Call (func=0xb7ba72ec, arg=0xb7ba7dec,
kw=0x0) at Objects/methodobject.c:73
#2  0xb7e6d05b in PyEval_EvalFrameEx (f=0x80cc40c, throwflag=0) at
Python/ceval.c:3569
#3  0xb7e6ec35 in PyEval_EvalCodeEx (co=0xb7b987b8, globals=0xb7bceacc,
locals=0xb7bceacc, args=0x0, argcount=0, kws=0x0, kwcount=0, defs=0x0,
defcount=0,
    closure=0x0) at Python/ceval.c:2832
#4  0xb7e6ee53 in PyEval_EvalCode (co=0xb7b987b8, globals=0xb7bceacc,
locals=0xb7bceacc) at Python/ceval.c:494
#5  0xb7e8ea8d in PyRun_InteractiveOneFlags (fp=0xb7d48420,
filename=0xb7ec589c "<stdin>", flags=0xbff3c6a8) at Python/pythonrun.c:1273
#6  0xb7e8ecc6 in PyRun_InteractiveLoopFlags (fp=0xb7d48420,
filename=0xb7ec589c "<stdin>", flags=0xbff3c6a8) at Python/pythonrun.c:723
#7  0xb7e8f427 in PyRun_AnyFileExFlags (fp=0xb7d48420,
filename=0xb7ec589c "<stdin>", closeit=0, flags=0xbff3c6a8) at
Python/pythonrun.c:692
#8  0xb7e9a347 in Py_Main (argc=0, argv=0xbff3c774) at Modules/main.c:523
#9  0x080485a2 in main (argc=538976288, argv=0x20202020) at
./Modules/python.c:23


Though I am not exactly sure how to proceed from here.
msg58562 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 19:08
Martin, can you look into this?  It seems GCC 4.3 disables buffer
overflow protection checks.  The best short-term solution may be to
disable that particular kind of optimization.  How?
msg58563 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 19:08
Martin, can you look into this?  It seems GCC 4.3 disables buffer
overflow protection checks.  The best short-term solution may be to
disable that particular kind of optimization.  How?
msg58564 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:10
Guido,

if you can give me a sample testcase I can bug GCC developers, this
doesn't look good from GCC side at all. Btw from my limited C knowledge
marking variables would volatile would prevent optimizations of them.
msg58566 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 19:16
> if you can give me a sample testcase I can bug GCC developers, this
> doesn't look good from GCC side at all. Btw from my limited C knowledge
> marking variables would volatile would prevent optimizations of them.

The example would be something like

void foo(ssize_t x)
{
  if (x >= 0) {
    if (x+x < 0) printf("Overflow\n");
  }
}

main()
{
  foo(2147483647);
}

This should print "Overflow" but won't if the evil optimization
triggers. (However you may have to tweak the example program so the
compiler can't inline the argument to foo.)
msg58568 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:24
Test always prints overflow here, tested with -O3 but here are
interesting overflow warnings that might give a clue , but I think
Cpickle is not involved here, but anyway:

/home/cartman/python-2.5/Modules/cPickle.c: In function 'Unpickler_noload':
/home/cartman/python-2.5/Modules/cPickle.c:4232: warning: assuming
signed overflow does not occur when assuming that (X - c) > X is always
false
/home/cartman/python-2.5/Modules/cPickle.c:194: warning: assuming signed
overflow does not occur when assuming that (X - c) > X is always false
/home/cartman/python-2.5/Modules/cPickle.c: In function 'load':
/home/cartman/python-2.5/Modules/cPickle.c:4232: warning: assuming
signed overflow does not occur when assuming that (X - c) > X is always
false
msg58570 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:30
Following testcase doesn't print overflow with gcc 4.3 when compiled
with -O3, works with gcc 3.4.6 though.

#include <sys/types.h>
#include <stdio.h>

void foo(ssize_t x)
{
 if (x >= 0) {
   if (x+x < 0) printf("Overflow\n");
}
}

main()
{
  volatile ssize_t x =2147483647;
  foo(x);
}
msg58571 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:33
Reported as a gcc bug, http://gcc.gnu.org/PR34454
msg58572 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:38
Ok so this is a code bug according to GCC developers see comment 1 & 2
at http://gcc.gnu.org/PR34454 .
msg58573 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 19:43
> Ok so this is a code bug according to GCC developers see comment 1 & 2
> at http://gcc.gnu.org/PR34454 .

I told you you can't win this argument with the GCC devs.

We'll have to use -fwrapv or whatever.
msg58574 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:43
-fwrapv fixes the issue, thanks!
msg58575 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 19:48
Can you suggest a patch that adds this permanently, whenever it is supported?
msg58576 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:55
Looks like -fwrapv is there since gcc 2.95.3 attached patch adds -fwrapv
when debugging disabled, also removes gcc 4.x part from README as it no
longer applies.
msg58577 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:56
After applying patch you need to run autoconf to update configure file
and svn commit afterwards.

Regards,
ismail
msg58578 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 19:59
Ok gcc developers say -fwrapv is there since gcc 3.3 so I think its
still fine, if not I will prepare another patch.

Regards.
msg58579 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 20:04
GCC 2.96 is still the golden standard for me, and it doesn't like
-fwrapv. Please try to come up with a better patch. It should be easy
enough to invoke gcc -fwrapv with a dummy program.
msg58583 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 20:26
Attached patch exactly checks if compiler supports -fwrapv otherwise
doesn't use it. Is this ok?
msg58584 - (view) Author: Ismail Donmez (donmez) * Date: 2007-12-13 20:28
Last patch had a grammar error in comment, fix that.
msg58586 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 20:52
Committed revision 59483 (2.5 branch).
Committed revision 59484 (2.6 trunk).

Keeping this open since someone still needs to run autoconf to
regenerate configure for the 2.6 trunk.
msg58597 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-13 22:38
Thomas Heller ran autoconf for the trunk and submitted as r59485.

(Thomas, could you run it in the 2.5 branch as well?  I seem to have
checked in a lot of gratuitous changes by using an older version of
autoconf.)
msg58603 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2007-12-14 00:45
"""code that has been audited and fixed in the past will again be
vulnerable."""

That code wasn't properly audited or fixed if it depended on integer
overflow behavior.

Anyways, I'm glad we have the flag to disable the optimization on gcc in
the meantime.

We should open a bug regarding fixing all of pythons integer overflows.
 gcc is only one compiler.  Other compilers are free to behave in
exactly the same manner.

I've opened http://bugs.python.org/issue1621 to track the larger code fix.
msg58604 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-14 00:53
> """code that has been audited and fixed in the past will again be
> vulnerable."""
>
> That code wasn't properly audited or fixed if it depended on integer
> overflow behavior.

Whatever, this is how overflow checks have been coded all over the code base.
msg58616 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2007-12-14 06:58
Guido van Rossum schrieb:
> (Thomas, could you run it in the 2.5 branch as well?  I seem to have
> checked in a lot of gratuitous changes by using an older version of
> autoconf.)

Done, see rev 59494.
msg59613 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-01-09 17:46
It would actually be better to use -fno-strict-overflow instead of
-fwrapv, if it exists (GCC 4.2 and later).

See also http://bugs.python.org/issue1621 which suggests there aren't
actually many places that need this; gcc -Wstrict-overflow should help
auditing the code.
History
Date User Action Args
2008-01-09 17:46:04gvanrossumsetmessages: + msg59613
2007-12-14 06:58:48thellersetmessages: + msg58616
2007-12-14 00:53:47gvanrossumsetmessages: + msg58604
2007-12-14 00:45:05gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg58603
2007-12-13 22:38:59gvanrossumsetstatus: open -> closed
nosy: + theller
resolution: fixed
messages: + msg58597
2007-12-13 20:52:02gvanrossumsetpriority: critical -> normal
messages: + msg58586
2007-12-13 20:28:07donmezsetfiles: + wrap.patch
messages: + msg58584
2007-12-13 20:26:54donmezsetfiles: + wrap.patch
messages: + msg58583
2007-12-13 20:04:49gvanrossumsetmessages: + msg58579
2007-12-13 19:59:06donmezsetmessages: + msg58578
2007-12-13 19:56:19donmezsetmessages: + msg58577
2007-12-13 19:55:30donmezsetfiles: + fwrapv.patch
messages: + msg58576
2007-12-13 19:48:57gvanrossumsetmessages: + msg58575
2007-12-13 19:43:28donmezsetmessages: + msg58574
2007-12-13 19:43:02gvanrossumsetmessages: + msg58573
2007-12-13 19:38:52donmezsetmessages: + msg58572
2007-12-13 19:33:41donmezsetmessages: + msg58571
2007-12-13 19:30:47donmezsetmessages: + msg58570
2007-12-13 19:24:48donmezsetmessages: + msg58568
2007-12-13 19:16:37gvanrossumsetmessages: + msg58566
2007-12-13 19:10:07donmezsetmessages: + msg58564
2007-12-13 19:08:28gvanrossumsetassignee: loewis
messages: + msg58563
nosy: + loewis
2007-12-13 19:08:09gvanrossumsetmessages: + msg58562
2007-12-13 19:07:39donmezsetmessages: + msg58561
2007-12-13 19:06:59gvanrossumsetpriority: critical
messages: + msg58560
2007-12-13 19:01:27gvanrossumsetmessages: + msg58558
2007-12-13 18:38:17donmezsetmessages: + msg58555
2007-12-13 18:23:42gvanrossumsetmessages: + msg58551
2007-12-13 18:23:09gvanrossumsetmessages: + msg58550
2007-12-13 18:22:44donmezsetmessages: + msg58549
2007-12-13 18:08:10donmezsetfiles: + valgrind-supp.txt
messages: + msg58546
2007-12-13 17:57:52gvanrossumsetnosy: + gvanrossum
messages: + msg58540
2007-12-13 10:21:13donmezsetfiles: + valgrind.txt
messages: + msg58529
2007-12-13 10:17:32donmezsettitle: Regression tests crashes with gcc 4.3 -> test_str.py crashes
2007-12-13 10:12:58donmezsetfiles: + backtrace.txt
messages: + msg58526
2007-12-13 10:09:56donmezcreate