Merge "Read maps data before using and do safe reads." into main
diff --git a/benchmarks/linker_relocation/include/linker_reloc_bench_asm.h b/benchmarks/linker_relocation/include/linker_reloc_bench_asm.h
index 885e47f..ea63e36 100644
--- a/benchmarks/linker_relocation/include/linker_reloc_bench_asm.h
+++ b/benchmarks/linker_relocation/include/linker_reloc_bench_asm.h
@@ -44,9 +44,7 @@
#elif defined(__riscv)
-// No `lga` in clang unless https://reviews.llvm.org/D107278 lands.
-// `la` is equivalent when using PIC (which we do) though.
-#define GOT_RELOC(sym) la a0, sym
+#define GOT_RELOC(sym) lga a0, sym
#define CALL(sym) call sym@plt
#define DATA_WORD(val) .quad val
#define MAIN .globl main; main: li a0, 0; ret
diff --git a/benchmarks/util.h b/benchmarks/util.h
index 99eed5f..347dc35 100644
--- a/benchmarks/util.h
+++ b/benchmarks/util.h
@@ -71,7 +71,7 @@
bool LockToCPU(int cpu_to_lock);
-static __inline __attribute__ ((__always_inline__)) void MakeAllocationResident(
+static inline __attribute__((__always_inline__)) void MakeAllocationResident(
void* ptr, size_t nbytes, int pagesize) {
uint8_t* data = reinterpret_cast<uint8_t*>(ptr);
for (size_t i = 0; i < nbytes; i += pagesize) {
diff --git a/libc/bionic/execinfo.cpp b/libc/bionic/execinfo.cpp
index d129f7c..e53a037 100644
--- a/libc/bionic/execinfo.cpp
+++ b/libc/bionic/execinfo.cpp
@@ -73,6 +73,11 @@
#elif defined(__aarch64__)
// All instructions are 4 bytes long, skip back one instruction.
ip -= 4;
+#elif defined(__riscv)
+ // C instructions are the shortest at 2 bytes long. (Unlike thumb, it's
+ // non-trivial to recognize C instructions when going backwards in the
+ // instruction stream.)
+ ip -= 2;
#elif defined(__i386__) || defined(__x86_64__)
// It's difficult to decode exactly where the previous instruction is,
// so subtract 1 to estimate where the instruction lives.
diff --git a/libc/bionic/posix_timers.cpp b/libc/bionic/posix_timers.cpp
index f522516..ccbbfcf 100644
--- a/libc/bionic/posix_timers.cpp
+++ b/libc/bionic/posix_timers.cpp
@@ -34,6 +34,8 @@
#include <string.h>
#include <time.h>
+#include "private/bionic_lock.h"
+
// System calls.
extern "C" int __rt_sigprocmask(int, const sigset64_t*, sigset64_t*, size_t);
extern "C" int __rt_sigtimedwait(const sigset64_t*, siginfo_t*, const timespec*, size_t);
@@ -60,6 +62,7 @@
int sigev_notify;
// The fields below are only needed for a SIGEV_THREAD timer.
+ Lock startup_handshake_lock;
pthread_t callback_thread;
void (*callback)(sigval_t);
sigval_t callback_argument;
@@ -73,6 +76,18 @@
static void* __timer_thread_start(void* arg) {
PosixTimer* timer = reinterpret_cast<PosixTimer*>(arg);
+ // Check that our parent managed to create the kernel timer and bail if not...
+ timer->startup_handshake_lock.lock();
+ if (timer->kernel_timer_id == -1) {
+ free(timer);
+ return nullptr;
+ }
+
+ // Give ourselves a specific meaningful name now we have a kernel timer.
+ char name[16]; // 16 is the kernel-imposed limit.
+ snprintf(name, sizeof(name), "POSIX timer %d", to_kernel_timer_id(timer));
+ pthread_setname_np(timer->callback_thread, name);
+
sigset64_t sigset = {};
sigaddset64(&sigset, TIMER_SIGNAL);
@@ -109,6 +124,7 @@
return -1;
}
+ timer->kernel_timer_id = -1;
timer->sigev_notify = (evp == nullptr) ? SIGEV_SIGNAL : evp->sigev_notify;
// If not a SIGEV_THREAD timer, the kernel can handle it without our help.
@@ -149,6 +165,10 @@
sigaddset64(&sigset, TIMER_SIGNAL);
sigset64_t old_sigset;
+ // Prevent the child thread from running until the timer has been created.
+ timer->startup_handshake_lock.init(false);
+ timer->startup_handshake_lock.lock();
+
// Use __rt_sigprocmask instead of sigprocmask64 to avoid filtering out TIMER_SIGNAL.
__rt_sigprocmask(SIG_BLOCK, &sigset, &old_sigset, sizeof(sigset));
@@ -162,21 +182,21 @@
return -1;
}
+ // Try to create the kernel timer.
sigevent se = *evp;
se.sigev_signo = TIMER_SIGNAL;
se.sigev_notify = SIGEV_THREAD_ID;
se.sigev_notify_thread_id = pthread_gettid_np(timer->callback_thread);
- if (__timer_create(clock_id, &se, &timer->kernel_timer_id) == -1) {
- __timer_thread_stop(timer);
+ rc = __timer_create(clock_id, &se, &timer->kernel_timer_id);
+
+ // Let the child run (whether we created the kernel timer or not).
+ timer->startup_handshake_lock.unlock();
+ // If __timer_create(2) failed, the child will kill itself and free the
+ // timer struct, so we just need to exit.
+ if (rc == -1) {
return -1;
}
- // Give the thread a specific meaningful name.
- // It can't do this itself because the kernel timer isn't created until after it's running.
- char name[16]; // 16 is the kernel-imposed limit.
- snprintf(name, sizeof(name), "POSIX timer %d", to_kernel_timer_id(timer));
- pthread_setname_np(timer->callback_thread, name);
-
*timer_id = timer;
return 0;
}
diff --git a/libc/bionic/sys_statvfs.cpp b/libc/bionic/sys_statvfs.cpp
index b3a0aca..3a05c3f 100644
--- a/libc/bionic/sys_statvfs.cpp
+++ b/libc/bionic/sys_statvfs.cpp
@@ -17,7 +17,7 @@
#include <sys/statfs.h>
#include <sys/statvfs.h>
-static __inline void __bionic_statfs_to_statvfs(const struct statfs* src, struct statvfs* dst) {
+static inline void __bionic_statfs_to_statvfs(const struct statfs* src, struct statvfs* dst) {
dst->f_bsize = src->f_bsize;
dst->f_frsize = src->f_frsize;
dst->f_blocks = src->f_blocks;
diff --git a/libc/bionic/sys_thread_properties.cpp b/libc/bionic/sys_thread_properties.cpp
index d1a73b7..d7188f5 100644
--- a/libc/bionic/sys_thread_properties.cpp
+++ b/libc/bionic/sys_thread_properties.cpp
@@ -77,35 +77,9 @@
// Find the thread-pointer register for the given thread.
void** tp_reg = nullptr;
-#if defined(__x86_64__)
- {
- ErrnoRestorer errno_restorer;
- errno = 0;
- uintptr_t fs_base = ptrace(PTRACE_PEEKUSER, tid, offsetof(user_regs_struct, fs_base), nullptr);
- if (errno == 0) {
- tp_reg = reinterpret_cast<void**>(fs_base);
- }
- }
-#elif defined(__i386__)
- struct user_regs_struct regs;
- struct iovec pt_iov = {
- .iov_base = ®s,
- .iov_len = sizeof(regs),
- };
-
- if (ptrace(PTRACE_GETREGSET, tid, NT_PRSTATUS, &pt_iov) == 0) {
- struct user_desc u_info;
- u_info.entry_number = regs.xgs >> 3;
- if (ptrace(PTRACE_GET_THREAD_AREA, tid, u_info.entry_number, &u_info) == 0) {
- tp_reg = reinterpret_cast<void**>(u_info.base_addr);
- }
- }
-#elif defined(__aarch64__)
+#if defined(__aarch64__)
uint64_t reg;
- struct iovec pt_iov {
- .iov_base = ®, .iov_len = sizeof(reg),
- };
-
+ struct iovec pt_iov { .iov_base = ®, .iov_len = sizeof(reg) };
if (ptrace(PTRACE_GETREGSET, tid, NT_ARM_TLS, &pt_iov) == 0) {
tp_reg = reinterpret_cast<void**>(reg);
}
@@ -114,6 +88,31 @@
// Reset the tp_reg if ptrace was unsuccessful.
tp_reg = nullptr;
}
+#elif defined(__i386__)
+ struct user_regs_struct regs;
+ struct iovec pt_iov = { .iov_base = ®s, .iov_len = sizeof(regs) };
+ if (ptrace(PTRACE_GETREGSET, tid, NT_PRSTATUS, &pt_iov) == 0) {
+ struct user_desc u_info;
+ u_info.entry_number = regs.xgs >> 3;
+ if (ptrace(PTRACE_GET_THREAD_AREA, tid, u_info.entry_number, &u_info) == 0) {
+ tp_reg = reinterpret_cast<void**>(u_info.base_addr);
+ }
+ }
+#elif defined(__riscv)
+ struct user_regs_struct regs;
+ struct iovec pt_iov = { .iov_base = ®s, .iov_len = sizeof(regs) };
+ if (ptrace(PTRACE_GETREGSET, tid, NT_PRSTATUS, &pt_iov) == 0) {
+ tp_reg = reinterpret_cast<void**>(regs.tp);
+ }
+#elif defined(__x86_64__)
+ {
+ ErrnoRestorer errno_restorer;
+ errno = 0;
+ uintptr_t fs_base = ptrace(PTRACE_PEEKUSER, tid, offsetof(user_regs_struct, fs_base), nullptr);
+ if (errno == 0) {
+ tp_reg = reinterpret_cast<void**>(fs_base);
+ }
+ }
#endif
if (tp_reg == nullptr) {
diff --git a/libc/include/bits/threads_inlines.h b/libc/include/bits/threads_inlines.h
index 5878e0a..459866e 100644
--- a/libc/include/bits/threads_inlines.h
+++ b/libc/include/bits/threads_inlines.h
@@ -124,7 +124,7 @@
};
#pragma clang diagnostic pop
-static inline void* _Nonnull __bionic_thrd_trampoline(void* _Nonnull __arg) {
+static __inline void* _Nonnull __bionic_thrd_trampoline(void* _Nonnull __arg) {
struct __bionic_thrd_data __data =
*__BIONIC_CAST(static_cast, struct __bionic_thrd_data*, __arg);
free(__arg);
diff --git a/libc/include/strings.h b/libc/include/strings.h
index 2f4f764..4b8cc08 100644
--- a/libc/include/strings.h
+++ b/libc/include/strings.h
@@ -61,13 +61,13 @@
/** Deprecated. Use memmove() instead. */
#define bcopy(b1, b2, len) __bionic_bcopy((b1), (b2), (len))
-static __inline__ __always_inline void __bionic_bcopy(const void* _Nonnull b1, void* _Nonnull b2, size_t len) {
+static __inline __always_inline void __bionic_bcopy(const void* _Nonnull b1, void* _Nonnull b2, size_t len) {
__builtin_memmove(b2, b1, len);
}
/** Deprecated. Use memset() instead. */
#define bzero(b, len) __bionic_bzero((b), (len))
-static __inline__ __always_inline void __bionic_bzero(void* _Nonnull b, size_t len) {
+static __inline __always_inline void __bionic_bzero(void* _Nonnull b, size_t len) {
__builtin_memset(b, 0, len);
}
diff --git a/libc/include/sys/cdefs.h b/libc/include/sys/cdefs.h
index 7625d38..5d1718e 100644
--- a/libc/include/sys/cdefs.h
+++ b/libc/include/sys/cdefs.h
@@ -87,9 +87,12 @@
#define __STRING(x) #x
#define ___STRING(x) __STRING(x)
-#if defined(__cplusplus)
-#define __inline inline /* convert to C++ keyword */
-#endif /* !__cplusplus */
+// C++ has `inline` as a keyword, as does C99, but ANSI C (aka C89 aka C90)
+// does not. Everything accepts the `__inline__` extension though. We could
+// just use that directly in our own code, but there's historical precedent
+// for `__inline` meaning it's still used in upstream BSD code (and potentially
+// downstream in vendor or app code).
+#define __inline __inline__
#define __always_inline __attribute__((__always_inline__))
#define __attribute_const__ __attribute__((__const__))
@@ -260,7 +263,7 @@
* them available externally. FORTIFY'ed functions try to be as close to possible as 'invisible';
* having stack protectors detracts from that (b/182948263).
*/
-# define __BIONIC_FORTIFY_INLINE static __inline__ __attribute__((__no_stack_protector__)) \
+# define __BIONIC_FORTIFY_INLINE static __inline __attribute__((__no_stack_protector__)) \
__always_inline __VERSIONER_FORTIFY_INLINE
/*
* We should use __BIONIC_FORTIFY_VARIADIC instead of __BIONIC_FORTIFY_INLINE
@@ -268,9 +271,9 @@
* The __always_inline attribute is useless, misleading, and could trigger
* clang compiler bug to incorrectly inline variadic functions.
*/
-# define __BIONIC_FORTIFY_VARIADIC static __inline__
+# define __BIONIC_FORTIFY_VARIADIC static __inline
/* Error functions don't have bodies, so they can just be static. */
-# define __BIONIC_ERROR_FUNCTION_VISIBILITY static __attribute__((__unused__))
+# define __BIONIC_ERROR_FUNCTION_VISIBILITY static __unused
#else
/* Further increase sharing for some inline functions */
# define __pass_object_size_n(n)
diff --git a/linker/linker_main.cpp b/linker/linker_main.cpp
index 089eceb..e27fd91 100644
--- a/linker/linker_main.cpp
+++ b/linker/linker_main.cpp
@@ -422,20 +422,11 @@
ElfW(Ehdr)* elf_hdr = reinterpret_cast<ElfW(Ehdr)*>(si->base);
- // We haven't supported non-PIE since Lollipop for security reasons.
+ // For security reasons we dropped non-PIE support in API level 21,
+ // and the NDK no longer supports earlier API levels.
if (elf_hdr->e_type != ET_DYN) {
- // We don't use async_safe_fatal here because we don't want a tombstone:
- // even after several years we still find ourselves on app compatibility
- // investigations because some app's trying to launch an executable that
- // hasn't worked in at least three years, and we've "helpfully" dropped a
- // tombstone for them. The tombstone never provided any detail relevant to
- // fixing the problem anyway, and the utility of drawing extra attention
- // to the problem is non-existent at this late date.
- async_safe_format_fd(STDERR_FILENO,
- "\"%s\": error: Android 5.0 and later only support "
- "position-independent executables (-fPIE).\n",
- g_argv[0]);
- _exit(EXIT_FAILURE);
+ __linker_error("error: %s: Android only supports position-independent "
+ "executables (-fPIE)\n", exe_info.path.c_str());
}
// Use LD_LIBRARY_PATH and LD_PRELOAD (but only if we aren't setuid/setgid).
diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp
index fa712a1..b9229ca 100644
--- a/linker/linker_phdr.cpp
+++ b/linker/linker_phdr.cpp
@@ -298,7 +298,6 @@
}
if (header_.e_shentsize != sizeof(ElfW(Shdr))) {
- // Fail if app is targeting Android O or above
if (get_application_target_sdk_version() >= 26) {
DL_ERR_AND_LOG("\"%s\" has unsupported e_shentsize: 0x%x (expected 0x%zx)",
name_.c_str(), header_.e_shentsize, sizeof(ElfW(Shdr)));
@@ -312,12 +311,10 @@
}
if (header_.e_shstrndx == 0) {
- // Fail if app is targeting Android O or above
if (get_application_target_sdk_version() >= 26) {
DL_ERR_AND_LOG("\"%s\" has invalid e_shstrndx", name_.c_str());
return false;
}
-
DL_WARN_documented_change(26,
"invalid-elf-header_section-headers-enforced-for-api-level-26",
"\"%s\" has invalid e_shstrndx", name_.c_str());
diff --git a/tests/time_test.cpp b/tests/time_test.cpp
index ca8e260..baafbf6 100644
--- a/tests/time_test.cpp
+++ b/tests/time_test.cpp
@@ -31,6 +31,8 @@
#include <thread>
#include "SignalUtils.h"
+#include "android-base/file.h"
+#include "android-base/strings.h"
#include "utils.h"
using namespace std::chrono_literals;
@@ -797,21 +799,41 @@
ASSERT_EQ(1, timer_create_NULL_signal_handler_invocation_count);
}
-TEST(time, timer_create_EINVAL) {
- clockid_t invalid_clock = 16;
+static int GetThreadCount() {
+ std::string status;
+ if (android::base::ReadFileToString("/proc/self/status", &status)) {
+ for (const auto& line : android::base::Split(status, "\n")) {
+ int thread_count;
+ if (sscanf(line.c_str(), "Threads: %d", &thread_count) == 1) {
+ return thread_count;
+ }
+ }
+ }
+ return -1;
+}
- // A SIGEV_SIGNAL timer is easy; the kernel does all that.
+TEST(time, timer_create_EINVAL) {
+ const clockid_t kInvalidClock = 16;
+
+ // A SIGEV_SIGNAL timer failure is easy; that's the kernel's problem.
timer_t timer_id;
- ASSERT_EQ(-1, timer_create(invalid_clock, nullptr, &timer_id));
+ ASSERT_EQ(-1, timer_create(kInvalidClock, nullptr, &timer_id));
ASSERT_ERRNO(EINVAL);
- // A SIGEV_THREAD timer is more interesting because we have stuff to clean up.
- sigevent se;
- memset(&se, 0, sizeof(se));
+ // A SIGEV_THREAD timer failure is more interesting because we have a thread
+ // to clean up (https://issuetracker.google.com/340125671).
+ sigevent se = {};
se.sigev_notify = SIGEV_THREAD;
se.sigev_notify_function = NoOpNotifyFunction;
- ASSERT_EQ(-1, timer_create(invalid_clock, &se, &timer_id));
+ ASSERT_EQ(-1, timer_create(kInvalidClock, &se, &timer_id));
ASSERT_ERRNO(EINVAL);
+
+ // timer_create() doesn't guarantee that the thread will be dead _before_
+ // it returns because that would require extra synchronization that's
+ // unnecessary in the normal (successful) case. A timeout here means we
+ // leaked a thread.
+ while (GetThreadCount() > 1) {
+ }
}
TEST(time, timer_create_multiple) {
diff --git a/tests/utils.h b/tests/utils.h
index dcb08f5..3c83b73 100644
--- a/tests/utils.h
+++ b/tests/utils.h
@@ -38,6 +38,7 @@
#endif
#include <atomic>
+#include <iomanip>
#include <string>
#include <regex>
@@ -253,7 +254,7 @@
AssertChildExited(pid, expected_exit_status, &error_msg);
if (expected_output_regex != nullptr) {
if (!std::regex_search(output_, std::regex(expected_output_regex))) {
- FAIL() << "regex " << expected_output_regex << " didn't match " << output_;
+ FAIL() << "regex " << std::quoted(expected_output_regex) << " didn't match " << std::quoted(output_);
}
}
}