Commit graph

133 commits

Author SHA1 Message Date
Elliott Hughes
575129886c Pre-size the vector in the pthread_create ENOMEM test.
Just to avoid the possibility of an unfortunate mmap occurring in order to
grow the vector.

Bug: N/A
Test: ran tests
Change-Id: I850740461d761916a0377272664159d010da7b84
2017-10-03 12:20:07 -07:00
Elliott Hughes
53dc9dd701 Improve pthread_create failure handling.
Return EAGAIN rather than aborting if we fail to set up the TLS for a new
thread.

Add a test that uses all the VMAs so we can properly test these edge cases.

Add an explicit test for pthread_attr_setdetachstate, which we use in the
previous test, but other than that has no tests.

Remove support for ro.logd.timestamp/persist.logd.timestamp, which doesn't
seem to be used, and which prevents us from logging failures in cases where
mmap fails (because we need to mmap in the system property implementation).

Bug: http://b/65608572
Test: ran tests
Change-Id: I9009f06546e1c2cc55eff996d08b55eff3482343
2017-09-19 14:02:50 -07:00
Elliott Hughes
d6c678ca90 Support larger guard regions.
This also fixes a long-standing bug where the guard region would be taken
out of the stack itself, rather than being -- as POSIX demands -- additional
space after the stack. Historically a 128KiB stack with a 256KiB guard would
have given you an immediate crash.

Bug: http://b/38413813
Test: builds, boots
Change-Id: Idd12a3899be1d92fea3d3e0fa6882ca2216bd79c
2017-09-18 16:09:43 -07:00
Yabin Cui
ab4cddc329 Fix pthread_barrier_smoke test (part 2).
Bug:http://b/37652807
Test: run bionic-unit-tests.
Change-Id: Iaea553177956c5d08f754210273637f69d888c20
2017-05-02 16:18:13 -07:00
Yabin Cui
d5c04c5ebb Fix pthread_barrier_smoke test.
Bug: http://b/37652807
Test: run bionic-unit-tests.
Change-Id: Id7245223bc2a284efed6e710892b58947ef5d555
2017-05-02 12:57:39 -07:00
Tom Cherry
b8ab61804c Move scopeguard into android::base
Test: boot bullhead, bionic unit tests
Change-Id: I223249684867655ecb53713b10da41d3014f96ae
2017-04-05 16:37:07 -07:00
Josh Gao
61db9ac8da Split up the stack space tests into their own noinline functions.
Prevent the compiler from being too smart and allocating a stack buffer
at the beginning of a function.

Bug: http://b/36206043
Test: 32/64-bit dynamic tests pass, static ones still don't
Change-Id: I90c575be43a9dd6c4fefc0d8b514f1ae0405b994
2017-03-15 19:53:17 -07:00
Josh Gao
415daa8cca Increase signal stack size on 32-bit to 16kB.
snprintf to a buffer of length PATH_MAX consumes about 7kB of stack.

Bug: http://b/35858739
Test: bionic-unit-tests --gtest_filter="*big_enough*"
Change-Id: I34a7f42c1fd2582ca0d0a9b7e7a5290bc1cc19b1
2017-03-08 16:43:59 -08:00
Elliott Hughes
6ce686c48b Downgrade the special case of pthread_t(0) to a warning.
So far this is the only issue we've hit in vendor code, and we've hit
it several times already. Rather than try to fix bullhead (the current
problem), let's just admit that the special case of 0 is a lot less
worrying.

Also fix the test expectations to correspond to the new abort message.

Bug: http://b/35455349 (crashes on 0)
Bug: http://b/35622944 (tests)
Test: ran tests
Change-Id: Iec57011fa699a954ebeaec151db2193e36d1ef35
2017-02-21 14:21:43 -08:00
Elliott Hughes
11859d467c Be more strict about using invalid pthread_ts.
Another release, another attempt to remove the global thread list.

But this time, let's admit that it's not going away. We can switch to using
a read/write lock for the global thread list, and to aborting rather than
quietly returning ESRCH if we're given an invalid pthread_t.

This change affects 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.

(This patch replaces such users with calls to pthread_gettid_np, which
at least makes the TOCTOU window smaller.)

We can't check thread->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.

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 using
      pthread_detach(pthread_self()) from the new thread's start routine
      rather than doing the 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 directly.

      That's still not completely safe because if you're too late, the tid
      may have been reused, but then your code is inherently unsafe anyway.

Bug: http://b/19636317
Test: ran tests
Change-Id: I0372c4428e8a7f1c3af5c9334f5d9c25f2c73f21
2017-02-13 17:59:29 -08:00
Elliott Hughes
bcb152903a Revert "Stop checking the global thread list in several trivial cases."
This reverts commit f5a4992b71.

Breaks OMX_ImgEnc in cameraserver (http://b/35088254).

Change-Id: I6dcf12706a184b0b8b72451584567a42dfa1bb4f
2017-02-07 21:05:30 +00:00
Elliott Hughes
f5a4992b71 Stop checking the global thread list in several trivial cases.
Since removing the global thread is hard, let's take the different
groups of functions individually.

The existing code was racy anyway, because the thread might still be
on the list but have exited (leaving tid == 0).

Bug: http://b/19636317
Test: ran tests
Change-Id: Icc0986ff124d5f9b8a653edf718c549d1563973b
2017-02-06 14:09:53 -08:00
Elliott Hughes
7484c21c4c Revert "Remove the global thread list."
This reverts commit b0e8c565a6.

Breaks swiftshader (http:/b/34883464).

Change-Id: I7b21193ba8a78f07d7ac65e41d0fe8516940a83b
2017-02-02 02:41:38 +00:00
Elliott Hughes
b0e8c565a6 Remove the global thread list.
Another release, another attempt to fix this bug.

This change affects pthread_detach, pthread_getcpuclockid,
pthread_getschedparam/pthread_setschedparam, pthread_join, and pthread_kill:
instead of returning ESRCH when passed an invalid pthread_t, they'll now SEGV.

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

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 using pthread_detach(pthread_self())
  from the new thread's start routine rather than doing the 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 directly. That's still not completely safe
  because if you're too late, the tid may have been reused, but then your
  code is inherently unsafe anyway.

If we find too much code is still broken, we can come back and disable
the global thread list lookups for anything targeting >= O and then have
another go at really removing this in P...

Bug: http://b/19636317
Test: N6P boots, bionic tests pass
Change-Id: Ia92641212f509344b99ee2a9bfab5383147fcba6
2017-01-07 14:16:46 -08:00
Chih-Hung Hsieh
9af13d24b1 Do not return with stack address in global variable.
Clang static analyzer gives warning when address of
local variable 'attr' is saved in a global variable.
This change passes required values down to signal handler
instead of saving local variable address in a signal handler.

Change-Id: I7955939487a5afdf7b1f47eb74a92eb5aa76cfc9
2016-06-02 15:02:25 -07:00
Dan Albert
baa2a973bd Use clang's nullability instead of nonnull.
http://clang.llvm.org/docs/AttributeReference.html#nonnull

_Nonnull is similar to the nonnull attribute in that it will instruct
compilers to warn the user if it can prove that a null argument is
being passed. Unlike the nonnull attribute, this annotation indicated
that a value *should not* be null, not that it *cannot* be null, or
even that the behavior is undefined. The important distinction is that
the optimizer will perform surprising optimizations like the
following:

    void foo(void*) __attribute__(nonnull, 1);

    int bar(int* p) {
      foo(p);

      // The following null check will be elided because nonnull
      // attribute means that, since we call foo with p, p can be
      // assumed to not be null. Thus this will crash if we are called
      // with a null pointer.
      if (src != NULL) {
        return *p;
      }
      return 0;
    }

    int main() {
      return bar(NULL);
    }

Note that by doing this we are no longer attaching any sort of
attribute for GCC (GCC doesn't support attaching nonnull directly to a
parameter, only to the function and naming the arguments
positionally). This means we won't be getting a warning for this case
from GCC any more. People that listen to warnings tend to use clang
anyway, and we're quickly moving toward that as the default, so this
seems to be an acceptable tradeoff.

Change-Id: Ie05fe7cec2f19a082c1defb303f82bcf9241b88d
2016-05-05 17:11:54 -07:00
Chih-Hung Hsieh
62e3a078aa Fix google-explicit-constructor warnings.
Bug: 28341362
Change-Id: I84effbdfa1b9b39328a909b7f70fe17e7ee316c8
2016-05-03 12:08:05 -07:00
Elliott Hughes
4d098ca912 Add a test for pthread_setname_np on another thread in a PR_SET_DUMPABLE 0 process.
Bug: http://b/28051133
Change-Id: I9a578333815afa6bdfc1e3c3eea430a15957304f
2016-04-11 12:43:05 -07:00
Elliott Hughes
725b2a96a7 Add pthread_getname_np.
Also guard both these GNU extensions with _GNU_SOURCE.

Also improve the tests to test each case on both the current thread and
another thread, since the code paths are totally different.

Bug: http://b/27810459
Change-Id: I72b05bca5c5b6ca8ba4585b8edfb716a1c252f92
2016-03-23 17:40:25 -07:00
Yabin Cui
81d2797e33 Fix pthread.pthread_barrier_smoke test.
pthread_barrier_smoke test uses WaitUntilThreadSleep() to wait until
BarrierTestHelper threads sleep in pthread_barrier_wait(). But this
is flaky as there a two futex_wait places in pthread_barrier_wait.
This patch modifies this test to avoid using WaitUntilThreadSleep().

Bug: 27780937
Change-Id: I4c36b82cce9345d5088f8854b289dc5bf7a08e8c
2016-03-22 13:45:55 -07:00
Yabin Cui
61e4d461e5 Adjust test to let it pass on libhoudini.
When using libhoudini to run arm code on x86 platforms, we can't
assume the main thread allocates local variables at the stack
declared by kernel.

Change-Id: Id9457f47fc338a3103fdee25a7a6e622915e7090
2016-03-10 14:28:43 -08:00
Elliott Hughes
33697a0c43 Factor out the waiting for children in bionic tests.
Change-Id: I4a1e51b6920b33dc892d447f5bd6d10f1cb2704a
2016-01-26 13:13:52 -08:00
Elliott Hughes
d31d4c1cc6 Add a few missing pthread tests.
This seems to be all that's tested by system/extras/tests/bionic that isn't
already better tested here.

Change-Id: Id0aa985cefd4047a6007ba9804f541069d9e92ed
2015-12-14 17:35:10 -08:00
Yabin Cui
aec13988da Merge "Fix pthread_test according to tsan report." 2015-11-30 21:38:59 +00:00
Yabin Cui
17554356cc Merge "Change _stdio_handles_locking into _caller_handles_locking." 2015-11-23 18:57:26 +00:00
Yabin Cui
74ed96d597 Merge "Use FUTEX_WAIT_BITSET to avoid converting timeouts." 2015-11-21 01:50:29 +00:00
Yabin Cui
76144aaa63 Change _stdio_handles_locking into _caller_handles_locking.
It is reported by tsan that funlockfile() can unlock an unlocked mutex.
It happens when printf() is called before fopen() or other stdio stuff.
As FLOCKFILE(fp) is called before __sinit(), _stdio_handles_locking is false,
and _FLOCK(fp) will not be locked. But then cantwrite(fp) in __vfprintf()
calls__sinit(), which makes _stdio_handles_locking become true, and
FUNLOCKFILE(fp) unlocks _FLOCK(fp).

Change _stdio_handles_locking into _caller_handles_locking,
so __sinit() won't change its value. Add test due to my previous fault.

Bug: 25392375
Change-Id: I483e3c3cdb28da65e62f1fd9615bf58c5403b4dd
2015-11-20 17:44:26 -08:00
Yabin Cui
c9a659c57b Use FUTEX_WAIT_BITSET to avoid converting timeouts.
Add unittests for pthread APIs with timeout parameter.

Bug: 17569991

Change-Id: I6b3b9b2feae03680654cd64c3112ce7644632c87
2015-11-19 13:42:03 -08:00
Yabin Cui
fe3a83a934 Implement pthread spin.
In order to run tsan unit tests, we need to support pthread spin APIs.

Bug: 18623621
Bug: 25392375
Change-Id: Icbb4a74e72e467824b3715982a01600031868e29
2015-11-18 17:51:21 -08:00
Yabin Cui
a36158a77d Fix pthread_test according to tsan report.
1. Fix leak threads and data races related to spin_flag.
2. Increase stack size to run under tsan.

This doesn't pass all pthread tests, as some tests are used
to run intentionally in race situations.

Bug: 25392375
Change-Id: Icfba3e141e7170abd890809586e89b99adc8bd02
2015-11-16 21:15:58 -08:00
Yabin Cui
b804b9d67b Merge "Implement pthread barrier." 2015-11-17 00:22:54 +00:00
Yabin Cui
e7c2fffa16 Implement pthread barrier.
Bug: 24341262
Change-Id: I5472549e5d7545c1c3f0bef78235f545557b9630
2015-11-16 14:02:26 -08:00
Elliott Hughes
f208361b2b Clean up pthread_gettid_np test.
Change-Id: I0fad26c7824981bfa3ad3a8a0b28a1984062dcd1
2015-11-11 13:32:28 -08:00
Junjie Hu
de1246202a Fix potential race condition on CTS TC pthread_gettid_np
Root cause:
If start_routine thread exits before pthread_gettid_np is invokded, the "tid" field
will be cleared so that pthread_gettid_np will get "0" (which is cleared by kernel, 
due to the flag "CLONE_CHILD_CLEARTID" is set while calling clone system call inside
pthread_create).

Proposed patch:
Use a mutex to guarantee pthread_gettid_np will be invoked and returned before the
start_routine exits

Signed-off-by: Junjie Hu <junjie.hu@mediatek.com>

Change-Id: I22411f1b0f7446d76a0373cef4ccec858fac7018
(cherry picked from commit 4f80102935)
2015-11-11 21:21:21 +00:00
Elliott Hughes
0b2acdfcc9 Use const auto& in for loops.
Change-Id: Ic437c59797ee4e7dc38291da35c72d827bc89c8d
2015-10-02 18:25:19 -07:00
Elliott Hughes
15dfd63aba Fix another duplicate maps parser.
Change-Id: Icb69f59ffbd0d5de7f727142260fae152d36a904
2015-09-22 16:40:14 -07:00
Elliott Hughes
0dec228921 Clean up /proc/<pid>/maps sscanfs.
sscanf will swallow whitespace for us.

Change-Id: I59931cbad00f0144fd33ed4749ac0aaad15e6de6
2015-09-22 15:45:50 -07:00
Yabin Cui
33ac04a215 Increase alternative signal stack size on 64-bit devices.
Bug: 23041777
Bug: 24187462
Change-Id: I7d84c0cc775a74753a3e8e101169c0fb5dbf7437
2015-09-22 11:18:26 -07:00
Mor-sarid, Nitzan
569333293a Fix the way to get main thread stack start address.
For previous way to get the stack using the [stack] string from
/proc/self/task/<pid>/maps is not enough. On x86/x86_64, if an
alternative signal stack is used while a task switch happens,
the [stack] indicator may no longer be correct.

Instead, stack_start from /proc/self/stat which is always inside
the main stack, is used to find the main stack in /proc/self/maps.

Change-Id: Ieb010e71518b57560d541cd3b3563e5aa9660750
Signed-off-by: Nitzan Mor-sarid <nitzan.mor-sarid@intel.com>
Signed-off-by: Mingwei Shi <mingwei.shi@intel.com>
2015-09-16 11:45:13 -07:00
Christopher Ferris
60907c7f4e Allow NULL in pthread_mutex_lock/unlock.
The pthread_mutex_lock and pthread_mutex_unlock were allowed to
fail silently on L 32 bit devices when passed a NULL. We changed
this to a crash on 32 bit devices, but there are still games that make
these calls and are not likely to be updated. Therefore, once again
allow NULL to be passed in on 32 bit devices.

Bug: 19995172
(cherry picked from commit 511cfd9dc8)

Change-Id: I159a99a941cff94297ef3fffda7075f8ef1ae252
2015-06-10 10:50:43 -07:00
Yabin Cui
b0c6f2dba2 Fix pthread_attr_getstack__main_thread failure on glibc.
Move test of bionic specific implementation into bionic ifdef.

Bug: 19805726
Change-Id: Idf369b16e7f41f060c75b0aaf34e05cf3c161aa9
2015-05-20 14:41:15 -07:00
Yabin Cui
f9eeea6d65 Merge "Remove pthread_detach_no_leak test." 2015-05-08 01:36:30 +00:00
Yabin Cui
2957cc5f10 Remove pthread_detach_no_leak test.
This test has lost its purpose as we are using mmap/munmap for pthread_internal_t. And it is a flaky test.

Bug: 20860440
Change-Id: I7cbb6bc3fd8a2ca430415beab5ee27a856ce4ea7
2015-05-07 16:53:25 -07:00
Dmitriy Ivanov
5624a6a1e5 Move pthread_atfork_dlclose test out of static lib
Bug: http://b/20858755
Change-Id: I0d84e8b43dc33902d75af18db6b7c8e0b619d718
2015-05-06 14:15:28 -07:00
Elliott Hughes
9a2744df30 Merge "Fix POSIX timer thread naming." 2015-04-25 18:01:18 +00:00
Elliott Hughes
d1aea30b2a Fix POSIX timer thread naming.
Spencer Low points out that we never actually set a name because the constant
part of the string was longer than the kernel's maximum, and the kernel
rejects long names rather than truncate.

Shorten the fixed part of the string while still keeping it meaningful. 9999
POSIX timers should be enough for any process...

Bug: https://code.google.com/p/android/issues/detail?id=170089
Change-Id: Ic05f07584c1eac160743519091a540ebbf8d7eb1
2015-04-25 10:05:24 -07:00
Dmitriy Ivanov
ea295f68f1 Unregister pthread_atfork handlers on dlclose()
Bug: http://b/20339788
Change-Id: I874c87faa377645fa9e0752f4fc166d81fd9ef7e
2015-04-24 17:57:37 -07:00
Dimitry Ivanov
094f58fb2a Revert "Unregister pthread_atfork handlers on dlclose()"
The visibility control in pthread_atfork.h is incorrect.
 It breaks 64bit libc.so by hiding pthread_atfork.

 This reverts commit 6df122f852.

Change-Id: I21e4b344d500c6f6de0ccb7420b916c4e233dd34
2015-04-24 03:46:57 +00:00
Dmitriy Ivanov
6df122f852 Unregister pthread_atfork handlers on dlclose()
Change-Id: I326fdf6bb06bed12743f08980b5c69d849c015b8
2015-04-22 19:19:37 -07:00
Yabin Cui
8f3f04184a Merge "Prevent using static-allocated pthread keys before creation." 2015-04-14 20:35:08 +00:00