From 2237b6b8ecd2ec45648b3dae07b98baa9ae27601 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 13 Dec 2017 15:18:15 -0800 Subject: [PATCH] Mention the POSIX header tests in docs/status.md. Also add a couple of comments in the tests for ease of understanding when grepping for `__BIONIC__`. Bug: N/A Test: N/A Change-Id: I7833a3ffbcc3badf9cec95f268d11a6d8a5ff9aa --- docs/status.md | 182 ++++++++++++++--------------- tests/headers/posix/dirent_h.c | 4 +- tests/headers/posix/netinet_in_h.c | 2 +- 3 files changed, 94 insertions(+), 94 deletions(-) diff --git a/docs/status.md b/docs/status.md index 0aaa0b3f5..d4ca91318 100644 --- a/docs/status.md +++ b/docs/status.md @@ -1,71 +1,39 @@ -Android bionic status -===================== +# Android bionic status -# Target API level behavioral differences +## Bionic function availability -Most bionic bug fixes and improvements have been made without checks for -the app's `targetSdkVersion`. As of O there were exactly two exceptions, -but there are likely to be more in future because of Project Treble. +### POSIX -Invalid `pthread_t` handling (targetSdkVersion >= O) ----------------------------------------------------- +You can see the current status with respect to POSIX in the form of tests: +https://android.googlesource.com/platform/bionic/+/master/tests/headers/posix/ -As part of a long-term goal to remove the global thread list, -and in an attempt to flush out racy code, we changed how an invalid -`pthread_t` is handled. For `pthread_detach`, `pthread_getcpuclockid`, -`pthread_getschedparam`/`pthread_setschedparam`, `pthread_join`, and -`pthread_kill`, instead of returning ESRCH when passed an invalid -`pthread_t`, if you're targeting O or above, they'll abort with the -message "attempt to use invalid pthread\_t". +Some POSIX functionality is not supported by the Linux kernel, and +is guarded with tests for `__linux__`. Other functionality is not +supported by bionic or glibc, and guarded with tests for `__BIONIC__` +and `__GLIBC__`. In other cases historical accidents mean 32-bit +bionic diverged but 64-bit bionic matches POSIX; these are guarded with +`__LP64__`. -Note that this doesn't change behavior as much as you might think: the -old lookup only held the global thread list lock for the duration of -the lookup, so there was still a race between that and the dereference -in the caller, given that callers actually need the tid to pass to some -syscall or other, and sometimes update fields in the `pthread_internal_t` -struct too. +Most bionic-only diversions should be accompanied by an explanatory comment. -We can't check a thread's tid against 0 to see whether a `pthread_t` -is still valid because a dead thread gets its thread struct unmapped -along with its stack, so the dereference isn't safe. +Missing functions are either obsolete or explicitly disallowed by SELinux: + * `a64l`/`l64a` + * `confstr` + * `crypt`/`encrypt`/`setkey` + * `gethostid` + * `shm_open`/`shm_unlink` + * `sockatmark` -To fix your code, taking the affected functions one by one: +Missing functionality: + * `` + * `` + * Thread cancellation + * Robust mutexes - * `pthread_getcpuclockid` and `pthread_getschedparam`/`pthread_setschedparam` - should be fine. Unsafe calls to those seem highly unlikely. +Run `./libc/tools/check-symbols-glibc.py` in bionic/ for the current +list of POSIX functions implemented by glibc but not by bionic. - * Unsafe `pthread_detach` callers probably want to switch to - `pthread_attr_setdetachstate` instead, or use - `pthread_detach(pthread_self());` from the new thread's start routine - rather than calling detach in the parent. - - * `pthread_join` calls should be safe anyway, because a joinable thread - won't actually exit and unmap until it's joined. If you're joining an - unjoinable thread, the fix is to stop marking it detached. If you're - joining an already-joined thread, you need to rethink your design! - - * Unsafe `pthread_kill` calls aren't portably fixable. (And are obviously - inherently non-portable as-is.) The best alternative on Android is to - use `pthread_gettid_np` at some point that you know the thread to be - alive, and then call `kill`/`tgkill` with signal 0 (which checks - whether a process exists rather than actually sending a - signal). That's still not completely safe because if you're too late - the tid may have been reused, but your code is inherently unsafe without - a redesign anyway. - -Interruptable `sem_wait` (targetSdkVersion >= N) ------------------------------------------------- - -POSIX says that `sem_wait` can be interrupted by delivery of a -signal. This wasn't historically true in Android, and when we fixed this -bug we found that existing code relied on the old behavior. To preserve -compatibility, `sem_wait` can only return EINTR on Android if the app -targets N or later. - -# Bionic function availability - -libc ----- +### libc Current libc symbols: https://android.googlesource.com/platform/bionic/+/master/libc/libc.map.txt @@ -124,40 +92,11 @@ libc function count over time: G 803, H 825, I 826, J 846, J-MR1 873, J-MR2 881, K 896, L 1116, M 1181, N 1226, O 1278 ``` -ndk-r17$ for i in `ls -1v platforms/android-*/arch-arm/usr/lib/libc.so` ; do echo $i; nm $i | grep -vw [AbdNnt] | grep -vw B | wc -l ; done +ndk-r17$ for i in `ls -1v platforms/android-*/arch-arm/usr/lib/libc.so` ; do \ + echo $i; nm $i | grep -vw [AbdNnt] | grep -vw B | wc -l ; done ``` -Run `./libc/tools/check-symbols-glibc.py` in bionic/ for the current -list of POSIX functions implemented by glibc but not by bionic. Currently -(2017-10): -``` -aio_cancel -aio_error -aio_fsync -aio_read -aio_return -aio_suspend -aio_write -lio_listio -pthread_cancel -pthread_mutex_consistent -pthread_mutex_getprioceiling -pthread_mutex_setprioceiling -pthread_mutexattr_getprioceiling -pthread_mutexattr_getprotocol -pthread_mutexattr_getrobust -pthread_mutexattr_setprioceiling -pthread_mutexattr_setprotocol -pthread_mutexattr_setrobust -pthread_setcancelstate -pthread_setcanceltype -pthread_testcancel -wordexp -wordfree -``` - -libm ----- +### libm Current libm symbols: https://android.googlesource.com/platform/bionic/+/master/libm/libm.map.txt @@ -167,3 +106,64 @@ Current libm symbols: https://android.googlesource.com/platform/bionic/+/master/ libm function count over time: G 158, J-MR2 164, L 220, M 265, O 284 + + + +## Target API level behavioral differences + +Most bionic bug fixes and improvements have been made without checks for +the app's `targetSdkVersion`. As of O there were exactly two exceptions, +but there are likely to be more in future because of Project Treble. + +### Invalid `pthread_t` handling (targetSdkVersion >= O) + +As part of a long-term goal to remove the global thread list, +and in an attempt to flush out racy code, we changed how an invalid +`pthread_t` is handled. For `pthread_detach`, `pthread_getcpuclockid`, +`pthread_getschedparam`/`pthread_setschedparam`, `pthread_join`, and +`pthread_kill`, instead of returning ESRCH when passed an invalid +`pthread_t`, if you're targeting O or above, they'll abort with the +message "attempt to use invalid pthread\_t". + +Note that this doesn't change behavior as much as you might think: the +old lookup only held the global thread list lock for the duration of +the lookup, so there was still a race between that and the dereference +in the caller, given that callers actually need the tid to pass to some +syscall or other, and sometimes update fields in the `pthread_internal_t` +struct too. + +We can't check a thread's tid against 0 to see whether a `pthread_t` +is still valid because a dead thread gets its thread struct unmapped +along with its stack, so the dereference isn't safe. + +To fix your code, taking the affected functions one by one: + + * `pthread_getcpuclockid` and `pthread_getschedparam`/`pthread_setschedparam` + should be fine. Unsafe calls to those seem highly unlikely. + + * Unsafe `pthread_detach` callers probably want to switch to + `pthread_attr_setdetachstate` instead, or use + `pthread_detach(pthread_self());` from the new thread's start routine + rather than calling detach in the parent. + + * `pthread_join` calls should be safe anyway, because a joinable thread + won't actually exit and unmap until it's joined. If you're joining an + unjoinable thread, the fix is to stop marking it detached. If you're + joining an already-joined thread, you need to rethink your design! + + * Unsafe `pthread_kill` calls aren't portably fixable. (And are obviously + inherently non-portable as-is.) The best alternative on Android is to + use `pthread_gettid_np` at some point that you know the thread to be + alive, and then call `kill`/`tgkill` with signal 0 (which checks + whether a process exists rather than actually sending a + signal). That's still not completely safe because if you're too late + the tid may have been reused, but your code is inherently unsafe without + a redesign anyway. + +### Interruptable `sem_wait` (targetSdkVersion >= N) + +POSIX says that `sem_wait` can be interrupted by delivery of a +signal. This wasn't historically true in Android, and when we fixed this +bug we found that existing code relied on the old behavior. To preserve +compatibility, `sem_wait` can only return EINTR on Android if the app +targets N or later. diff --git a/tests/headers/posix/dirent_h.c b/tests/headers/posix/dirent_h.c index 6c61c8d91..4ce0f18d8 100644 --- a/tests/headers/posix/dirent_h.c +++ b/tests/headers/posix/dirent_h.c @@ -34,8 +34,8 @@ static void dirent_h() { INCOMPLETE_TYPE(DIR); TYPE(struct dirent); -#if defined(__BIONIC__) && !defined(__LP64__) - STRUCT_MEMBER(struct dirent, uint64_t, d_ino); // Historical ABI accident. +#if defined(__BIONIC__) && !defined(__LP64__) // Historical ABI accident. + STRUCT_MEMBER(struct dirent, uint64_t, d_ino); #else STRUCT_MEMBER(struct dirent, ino_t, d_ino); #endif diff --git a/tests/headers/posix/netinet_in_h.c b/tests/headers/posix/netinet_in_h.c index 6e0be1c64..cd896851b 100644 --- a/tests/headers/posix/netinet_in_h.c +++ b/tests/headers/posix/netinet_in_h.c @@ -62,7 +62,7 @@ static void netinet_in_h() { TYPE(struct ipv6_mreq); STRUCT_MEMBER(struct ipv6_mreq, struct in6_addr, ipv6mr_multiaddr); -#if defined(__BIONIC__) +#if defined(__BIONIC__) // Currently comes from uapi header. STRUCT_MEMBER(struct ipv6_mreq, int, ipv6mr_interface); #else STRUCT_MEMBER(struct ipv6_mreq, unsigned, ipv6mr_interface);