Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_str.py crashes #45949

Closed
donmez mannequin opened this issue Dec 13, 2007 · 35 comments
Closed

test_str.py crashes #45949

donmez mannequin opened this issue Dec 13, 2007 · 35 comments
Assignees
Labels
tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@donmez
Copy link
Mannequin

donmez mannequin commented Dec 13, 2007

BPO 1608
Nosy @gvanrossum, @loewis, @theller, @gpshead
Files
  • backtrace.txt
  • valgrind.txt
  • valgrind-supp.txt
  • fwrapv.patch
  • wrap.patch
  • wrap.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/loewis'
    closed_at = <Date 2007-12-13.22:38:59.753>
    created_at = <Date 2007-12-13.10:09:56.859>
    labels = ['tests', 'type-crash']
    title = 'test_str.py crashes'
    updated_at = <Date 2008-01-09.17:46:04.525>
    user = 'https://bugs.python.org/donmez'

    bugs.python.org fields:

    activity = <Date 2008-01-09.17:46:04.525>
    actor = 'gvanrossum'
    assignee = 'loewis'
    closed = True
    closed_date = <Date 2007-12-13.22:38:59.753>
    closer = 'gvanrossum'
    components = ['Tests']
    creation = <Date 2007-12-13.10:09:56.859>
    creator = 'donmez'
    dependencies = []
    files = ['8936', '8937', '8940', '8942', '8944', '8945']
    hgrepos = []
    issue_num = 1608
    keywords = []
    message_count = 35.0
    messages = ['58525', '58526', '58529', '58540', '58546', '58549', '58550', '58551', '58555', '58558', '58560', '58561', '58562', '58563', '58564', '58566', '58568', '58570', '58571', '58572', '58573', '58574', '58575', '58576', '58577', '58578', '58579', '58583', '58584', '58586', '58597', '58603', '58604', '58616', '59613']
    nosy_count = 5.0
    nosy_names = ['gvanrossum', 'loewis', 'theller', 'gregory.p.smith', 'donmez']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue1608'
    versions = ['Python 2.5']

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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

    @donmez donmez mannequin added tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 13, 2007
    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    gdb backtrace, segfaulting test is Lib/test/test_str.py

    @donmez donmez mannequin changed the title Regression tests crashes with gcc 4.3 test_str.py crashes Dec 13, 2007
    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    Valgrind output, shows lots of invalid reads.

    @gvanrossum
    Copy link
    Member

    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.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    --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.

    @gvanrossum
    Copy link
    Member

    Looks like expandtabs() has a problem. Can you boil it down to a single
    call?

    @gvanrossum
    Copy link
    Member

    BTW is this a released version of GCC? If not, you might want to file
    the bug with the GCC project...

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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.

    @gvanrossum
    Copy link
    Member

    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.

    @gvanrossum
    Copy link
    Member

    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!

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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.

    @gvanrossum
    Copy link
    Member

    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?

    1 similar comment
    @gvanrossum
    Copy link
    Member

    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?

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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.

    @gvanrossum
    Copy link
    Member

    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.)

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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);
    }

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    Reported as a gcc bug, http://gcc.gnu.org/PR34454

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    Ok so this is a code bug according to GCC developers see comment 1 & 2
    at http://gcc.gnu.org/PR34454 .

    @gvanrossum
    Copy link
    Member

    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.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    -fwrapv fixes the issue, thanks!

    @gvanrossum
    Copy link
    Member

    Can you suggest a patch that adds this permanently, whenever it is supported?

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    After applying patch you need to run autoconf to update configure file
    and svn commit afterwards.

    Regards,
    ismail

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    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.

    @gvanrossum
    Copy link
    Member

    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.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    Attached patch exactly checks if compiler supports -fwrapv otherwise
    doesn't use it. Is this ok?

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Dec 13, 2007

    Last patch had a grammar error in comment, fix that.

    @gvanrossum
    Copy link
    Member

    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.

    @gvanrossum
    Copy link
    Member

    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.)

    @gpshead
    Copy link
    Member

    gpshead commented Dec 14, 2007

    """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.

    @gvanrossum
    Copy link
    Member

    """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.

    @theller
    Copy link

    theller commented Dec 14, 2007

    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.

    @gvanrossum
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    tests Tests in the Lib/test dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants