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

android: test_faulthandler fails #71121

Closed
xdegaye mannequin opened this issue May 3, 2016 · 24 comments
Closed

android: test_faulthandler fails #71121

xdegaye mannequin opened this issue May 3, 2016 · 24 comments
Assignees
Labels
3.7 (EOL) end of life tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@xdegaye
Copy link
Mannequin

xdegaye mannequin commented May 3, 2016

BPO 26934
Nosy @terryjreedy, @vstinner, @xdegaye, @serhiy-storchaka, @moreati, @yan12125
Files
  • test_output.txt: test results
  • buggy_raise.patch
  • buggy_raise_2.patch
  • buggy_raise_3.patch
  • buggy_raise_4.patch
  • buggy_raise_5.patch: 'requires_raise' decorator without argument
  • 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/xdegaye'
    closed_at = <Date 2016-11-14.07:40:21.818>
    created_at = <Date 2016-05-03.14:58:24.657>
    labels = ['3.7', 'type-bug', 'tests']
    title = 'android: test_faulthandler fails'
    updated_at = <Date 2016-11-14.07:40:21.818>
    user = 'https://github.com/xdegaye'

    bugs.python.org fields:

    activity = <Date 2016-11-14.07:40:21.818>
    actor = 'xdegaye'
    assignee = 'xdegaye'
    closed = True
    closed_date = <Date 2016-11-14.07:40:21.818>
    closer = 'xdegaye'
    components = ['Tests']
    creation = <Date 2016-05-03.14:58:24.657>
    creator = 'xdegaye'
    dependencies = []
    files = ['42699', '45371', '45379', '45380', '45404', '45459']
    hgrepos = []
    issue_num = 26934
    keywords = ['patch']
    message_count = 24.0
    messages = ['264735', '265038', '265044', '265046', '265057', '265556', '265557', '265586', '265611', '280143', '280152', '280194', '280197', '280347', '280379', '280382', '280387', '280650', '280652', '280678', '280710', '280712', '280713', '280715']
    nosy_count = 8.0
    nosy_names = ['terry.reedy', 'vstinner', 'SilentGhost', 'xdegaye', 'python-dev', 'serhiy.storchaka', 'Alex.Willmer', 'yan12125']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue26934'
    versions = ['Python 3.6', 'Python 3.7']

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 3, 2016

    test_faulthandler fails on an android emulator running an x86 system image at API level 21.

    See the attached test_output.txt file.

    @xdegaye xdegaye mannequin added stdlib Python modules in the Lib dir build The build process and cross-build type-bug An unexpected behavior, bug, or error labels May 3, 2016
    @terryjreedy
    Copy link
    Member

    The 8 failures are all because exitcode is 0 when it should not be.

        self.assertNotEqual(exitcode, 0)
    AssertionError: 0 == 0

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 7, 2016

    Reproducing the code that is executed on the first failure (test_disable) gives:

    On linux:
    $ python
    Python 3.6.0a0 (default:47fa003aa9f1, Feb 24 2016, 13:09:02) 
    [GCC 5.3.0] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import faulthandler
    >>> faulthandler.enable()
    >>> faulthandler.disable()
    True
    >>> faulthandler._sigsegv()
    Segmentation fault (core dumped)
    
    
    On the android emulator:
    root@generic_x86:/data/local/tmp # python
    Python 3.6.0a0 (default:811ccdee6f87+, May  7 2016, 09:02:21) 
    [GCC 4.9 20140827 (prerelease)] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import faulthandler
    >>> faulthandler.enable()
    >>> faulthandler.disable()
    True
    >>> faulthandler._sigsegv()
    >>>

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 7, 2016

    The gdb backtrace of running interactively 'faulthandler._sigsegv()'. After the SIGSEGV and the last gdb 'continue', python is still alive with a '>>>' prompt and ready for the next user input:

    (gdb) b faulthandler_sigsegv
    Breakpoint 1 at 0xb752159f: file /home/xavier/src/packages/android/cpython/Modules/faulthandler.c, line 964.
    (gdb) c
    Continuing.

    Breakpoint 1, faulthandler_sigsegv (self=self@entry=<module at remote 0xb6d85cf4>,
    args=args@entry=()) at /home/xavier/src/packages/android/cpython/Modules/faulthandler.c:964
    964 {
    (gdb) next
    965 int release_gil = 0;
    (gdb)
    966 if (!PyArg_ParseTuple(args, "|i:_sigsegv", &release_gil))
    (gdb)
    969 if (release_gil) {
    (gdb)
    974 faulthandler_raise_sigsegv();
    (gdb) step
    faulthandler_raise_sigsegv ()
    at /home/xavier/src/packages/android/cpython/Modules/faulthandler.c:941
    941 {
    (gdb) next
    942 faulthandler_suppress_crash_report();
    (gdb)
    958 raise(SIGSEGV);
    (gdb)

    Program received signal SIGSEGV, Segmentation fault.
    0xb76ec126 in ?? ()
    (gdb) c
    Continuing.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented May 7, 2016

    This is due to something mysterious in Android's bionic & kernel. For the following C program:

    #include <stdlib.h>
    #include <signal.h>
    #include <stdio.h>
    
    int main()
    {
        sigaction(SIGSEGV, NULL, NULL);
        raise(SIGSEGV);
        printf("Good evening!\n");
        return 0;
    }

    Compiled with: /opt/android-ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-gcc test_sigsegv.c --sysroot /opt/android-ndk/platforms/android-21/arch-arm -fPIE -Wl,-pie
    (Replace /opt/android-ndk to the path of NDK)

    On ASUS ZE500KL:
    shell@ASUS_Z00E_2:/data/local/tmp $ ./a.out
    Good evening!

    On Linux: (Compiled with gcc test_sigsegv.c)
    $ ./a.out
    [2] 23434 segmentation fault (core dumped) ./a.out

    Seems the sigaction() call in Python occurs in Modules/signalmodule.c.

    @vstinner
    Copy link
    Member

    This is due to something mysterious in Android's bionic & kernel. For the following C program:
    (...)

    Please try without "sigaction(SIGSEGV, NULL, NULL);". Does the program still quit?

    @vstinner
    Copy link
    Member

    Does faulthandler._read_null() crash Python? If yes, I should patch test_faulthandler to replace all _sigsegv() with faulthandler._read_null().

    I was too lazy to do that because currently the unit tests checks the exact error message, whereas _read_null() gives different error messages depending on the platform.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented May 15, 2016

    Please try without "sigaction(SIGSEGV, NULL, NULL);". Does the program still quit?

    Nope.

    int main()
    {
        raise(SIGSEGV);
    }

    Still exits normally with 0.

    Does faulthandler._read_null() crash Python?

    Yes.

    int main()
    {
        int *x = 0;
        int y = *x;
        return 0;
    }

    Is killed and exits with 139 as expected.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 15, 2016

    root@generic_x86:/data/local/tmp # python
    Python 3.6.0a0 (default:eee959fee5f5+, May 14 2016, 13:43:41) 
    [GCC 4.9 20150123 (prerelease)] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import faulthandler
    >>> faulthandler._read_null() 
    Segmentation fault

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Nov 6, 2016

    The problem: raise() does not cause the program to exit in a Python interactive session and a spawned Python process exits with 0. Android fixed this problem at API level 24 (Android 6.0 aka Marshmallow).
    The attached patch fixes this (tested at API levels 21 et 24).

    @xdegaye xdegaye mannequin added tests Tests in the Lib/test dir 3.7 (EOL) end of life and removed stdlib Python modules in the Lib dir build The build process and cross-build labels Nov 6, 2016
    @xdegaye xdegaye mannequin self-assigned this Nov 6, 2016
    @vstinner
    Copy link
    Member

    vstinner commented Nov 6, 2016

    Can you try to write a decorator to not suplicate the same long skipIf()?

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Nov 7, 2016

    Thanks for the suggestion Victor. The decorator may be used in other tests as well, and referring to the concern raised by Chi Hsuan in msg280147 and my answer, it makes it more clear that the build time _android_api_level should not be used in the standard library itself.
    New patch.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Nov 7, 2016

    New patch following Victor code review.

    @vstinner
    Copy link
    Member

    vstinner commented Nov 8, 2016

    buggy_raise_3.patch LGTM but I don't think that it's worth it to add @requires_raise to support directly. I suggest to only add it to test_faulthandler.py.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Nov 9, 2016

    New patch.

    @vstinner
    Copy link
    Member

    vstinner commented Nov 9, 2016

    buggy_raise_4.patch LGTM. Thanks for your patience :-)

    Maybe later you can also modify existing tests to use the new
    @requires_android_level?

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Nov 9, 2016

    Thanks for your patience :-)

    No problem, your suggestions and code reviews are always very much welcome :)
    I will push this patch with the other Android patches after 3.6 is released. The patch at bpo-26936 will also use @requires_android_level.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Nov 12, 2016

    New patch using a plain 'requires_raise' decorator without argument.

    @serhiy-storchaka
    Copy link
    Member

    LGTM.

    @vstinner
    Copy link
    Member

    Oh, you didn't push your patch yet? Go ahead.

    buggy_raise_5.patch LGTM and is better than perfect :-)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 13, 2016

    New changeset 66aac9676a28 by Xavier de Gaye in branch 'default':
    Issue bpo-26934: Fix test_faulthandler on Android where raise() exits with 0,
    https://hg.python.org/cpython/rev/66aac9676a28

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Nov 13, 2016

    New changeset f37ac1a003f3 in branch 3.6.
    New changeset 892f13827219 in branch default.

    @xdegaye xdegaye mannequin closed this as completed Nov 13, 2016
    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Nov 13, 2016

    Buildbots fail since _ANDROID_API_LEVEL could end up being None

    @SilentGhost SilentGhost mannequin reopened this Nov 13, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 13, 2016

    New changeset 2639afcedaad by Xavier de Gaye in branch '3.6':
    Issue bpo-26934: Handle _ANDROID_API_LEVEL is None on Windows
    https://hg.python.org/cpython/rev/2639afcedaad

    New changeset 4012e28539c4 by Xavier de Gaye in branch 'default':
    Issue bpo-26934: Merge 3.6
    https://hg.python.org/cpython/rev/4012e28539c4

    @xdegaye xdegaye mannequin closed this as completed Nov 14, 2016
    @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
    3.7 (EOL) end of life tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants