classification
Title: android: test_faulthandler fails
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: xdegaye Nosy List: Alex.Willmer, Chi Hsuan Yen, SilentGhost, python-dev, serhiy.storchaka, terry.reedy, vstinner, xdegaye
Priority: normal Keywords: patch

Created on 2016-05-03 14:58 by xdegaye, last changed 2016-11-14 07:40 by xdegaye. This issue is now closed.

Files
File name Uploaded Description Edit
test_output.txt xdegaye, 2016-05-03 14:58 test results
buggy_raise.patch xdegaye, 2016-11-06 15:51 review
buggy_raise_2.patch xdegaye, 2016-11-07 09:04 review
buggy_raise_3.patch xdegaye, 2016-11-07 12:09 review
buggy_raise_4.patch xdegaye, 2016-11-09 08:48 review
buggy_raise_5.patch xdegaye, 2016-11-12 11:13 'requires_raise' decorator without argument review
Messages (24)
msg264735 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-05-03 14:58
test_faulthandler fails on an android emulator running an x86 system image at API level 21.

See the attached test_output.txt file.
msg265038 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-05-06 22:14
The 8 failures are all because exitcode is 0 when it should not be.

    self.assertNotEqual(exitcode, 0)
AssertionError: 0 == 0
msg265044 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-05-07 07:09
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()
>>>
msg265046 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-05-07 07:28
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.
msg265057 - (view) Author: Chi Hsuan Yen (Chi Hsuan Yen) * Date: 2016-05-07 10:05
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.
msg265556 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-05-14 23:12
> 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?
msg265557 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-05-14 23:14
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.
msg265586 - (view) Author: Chi Hsuan Yen (Chi Hsuan Yen) * Date: 2016-05-15 06:22
> 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.
msg265611 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-05-15 10:20
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
msg280143 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-11-06 15:51
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).
msg280152 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-06 17:14
Can you try to write a decorator to not suplicate the same long skipIf()?
msg280194 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-11-07 09:04
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.
msg280197 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-11-07 12:09
New patch following Victor code review.
msg280347 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-08 20:57
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.
msg280379 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-11-09 08:48
New patch.
msg280382 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-09 09:01
buggy_raise_4.patch LGTM. Thanks for your patience :-)

Maybe later you can also modify existing tests to use the new
@requires_android_level?
msg280387 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-11-09 09:49
> 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 issue 26936 will also use @requires_android_level.
msg280650 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-11-12 11:13
New patch using a plain 'requires_raise' decorator without argument.
msg280652 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-12 11:23
LGTM.
msg280678 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-12 23:38
Oh, you didn't push your patch yet? Go ahead.

buggy_raise_5.patch LGTM and is better than perfect :-)
msg280710 - (view) Author: Roundup Robot (python-dev) Date: 2016-11-13 20:14
New changeset 66aac9676a28 by Xavier de Gaye in branch 'default':
Issue #26934: Fix test_faulthandler on Android where raise() exits with 0,
https://hg.python.org/cpython/rev/66aac9676a28
msg280712 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2016-11-13 20:18
New changeset f37ac1a003f3 in branch 3.6.
New changeset 892f13827219 in branch default.
msg280713 - (view) Author: SilentGhost (SilentGhost) * Date: 2016-11-13 20:31
Buildbots fail since _ANDROID_API_LEVEL could end up being None
msg280715 - (view) Author: Roundup Robot (python-dev) Date: 2016-11-13 20:57
New changeset 2639afcedaad by Xavier de Gaye in branch '3.6':
Issue #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 #26934: Merge 3.6
https://hg.python.org/cpython/rev/4012e28539c4
History
Date User Action Args
2016-11-14 07:40:21xdegayesetstatus: open -> closed
2016-11-13 20:57:32python-devsetmessages: + msg280715
2016-11-13 20:31:31SilentGhostsetstatus: closed -> open
nosy: + SilentGhost
messages: + msg280713

2016-11-13 20:19:56xdegayesetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2016-11-13 20:18:33xdegayesetmessages: + msg280712
2016-11-13 20:14:38python-devsetnosy: + python-dev
messages: + msg280710
2016-11-12 23:38:14vstinnersetmessages: + msg280678
2016-11-12 11:23:53serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg280652
2016-11-12 11:13:39xdegayesetfiles: + buggy_raise_5.patch

messages: + msg280650
2016-11-09 09:49:15xdegayesetmessages: + msg280387
stage: patch review -> commit review
2016-11-09 09:01:52vstinnersetmessages: + msg280382
2016-11-09 08:48:31xdegayesetfiles: + buggy_raise_4.patch

messages: + msg280379
2016-11-08 20:57:01vstinnersetmessages: + msg280347
2016-11-07 12:09:11xdegayesetfiles: + buggy_raise_3.patch

messages: + msg280197
2016-11-07 09:04:40xdegayesetfiles: + buggy_raise_2.patch

messages: + msg280194
2016-11-06 17:14:53vstinnersetmessages: + msg280152
2016-11-06 15:51:57xdegayesetfiles: + buggy_raise.patch

assignee: xdegaye
components: + Tests, - Library (Lib), Cross-Build
versions: + Python 3.7
keywords: + patch
messages: + msg280143
stage: patch review
2016-05-21 07:06:39xdegayelinkissue26865 dependencies
2016-05-15 10:20:49xdegayesetmessages: + msg265611
2016-05-15 06:22:09Chi Hsuan Yensetmessages: + msg265586
2016-05-14 23:14:24vstinnersetmessages: + msg265557
2016-05-14 23:12:32vstinnersetmessages: + msg265556
2016-05-07 10:05:30Chi Hsuan Yensetnosy: + Chi Hsuan Yen
messages: + msg265057
2016-05-07 07:28:28xdegayesetmessages: + msg265046
2016-05-07 07:09:56xdegayesetmessages: + msg265044
2016-05-06 22:14:13terry.reedysetnosy: + terry.reedy
messages: + msg265038
2016-05-03 14:58:24xdegayecreate