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
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
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
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
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
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
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
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
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
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
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
This seems to be all that's tested by system/extras/tests/bionic that isn't
already better tested here.
Change-Id: Id0aa985cefd4047a6007ba9804f541069d9e92ed
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
In order to run tsan unit tests, we need to support pthread spin APIs.
Bug: 18623621
Bug: 25392375
Change-Id: Icbb4a74e72e467824b3715982a01600031868e29
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
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)
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>
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
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
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
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
Previous implementation of rwlock contains four atomic variables, which
is hard to maintain and change. So I make following changes in this CL:
1. Add pending flags in rwlock.state, so we don't need to synchronize
between different atomic variables. Using compare_and_swap operations
on rwlock.state is enough for all state change.
2. Add pending_lock to protect readers/writers waiting and wake up
operations. As waiting/wakeup is not performance critical, using a
lock is easier to maintain.
3. Add writer preference option.
4. Add unit tests for rwlock.
Bug: 19109156
Change-Id: Idcaa58d695ea401d64445610b465ac5cff23ec7c
