Skip to content

[libc] add simplified tid cache #101620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libc/config/linux/aarch64/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.geteuid
libc.src.unistd.getpid
libc.src.unistd.getppid
libc.src.unistd.gettid
libc.src.unistd.getuid
libc.src.unistd.isatty
libc.src.unistd.link
Expand Down
1 change: 1 addition & 0 deletions libc/config/linux/riscv/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.geteuid
libc.src.unistd.getpid
libc.src.unistd.getppid
libc.src.unistd.gettid
libc.src.unistd.getuid
libc.src.unistd.isatty
libc.src.unistd.link
Expand Down
1 change: 1 addition & 0 deletions libc/config/linux/x86_64/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.geteuid
libc.src.unistd.getpid
libc.src.unistd.getppid
libc.src.unistd.gettid
libc.src.unistd.getuid
libc.src.unistd.isatty
libc.src.unistd.link
Expand Down
6 changes: 6 additions & 0 deletions libc/newhdrgen/yaml/unistd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -307,3 +307,9 @@ functions:
- type: const void *__restrict
- type: void *
- type: ssize_t
- name: gettid
standards:
- Linux
return_type: pid_t
arguments:
- type: void
5 changes: 5 additions & 0 deletions libc/spec/posix.td
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,11 @@ def POSIX : StandardSpec<"POSIX"> {
RetValSpec<PidT>,
[ArgSpec<VoidType>]
>,
FunctionSpec<
"gettid",
RetValSpec<PidT>,
[ArgSpec<VoidType>]
>,
FunctionSpec<
"getuid",
RetValSpec<UidT>,
Expand Down
16 changes: 16 additions & 0 deletions libc/src/__support/threads/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,19 @@ if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.CndVar)
.${LIBC_TARGET_OS}.CndVar
)
endif()

if (LLVM_LIBC_FULL_BUILD)
set(identifier_dependency_on_thread libc.src.__support.threads.thread)
endif()

add_header_library(
identifier
HDRS
identifier.h
DEPENDS
libc.src.__support.OSUtil.osutil
libc.src.__support.common
libc.include.sys_syscall
libc.hdr.types.pid_t
${identifier_dependency_on_thread}
)
49 changes: 49 additions & 0 deletions libc/src/__support/threads/identifier.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//===--- Thread Identifier Header --------------------------------*- C++-*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_LIBC_SRC___SUPPORT_THREADS_IDENTIFIER_H
#define LLVM_LIBC_SRC___SUPPORT_THREADS_IDENTIFIER_H

#ifdef LIBC_FULL_BUILD
#include "src/__support/threads/thread.h"
#endif // LIBC_FULL_BUILD

#include "hdr/types/pid_t.h"
#include "src/__support/OSUtil/syscall.h"
#include <sys/syscall.h>

namespace LIBC_NAMESPACE_DECL {
namespace internal {

LIBC_INLINE pid_t *get_tid_cache() {
#ifdef LIBC_FULL_BUILD
return &self.attrib->tid;
#else
// in non-full build mode, we do not control the fork routine. Therefore,
// we do not cache tid at all.
return nullptr;
#endif
}

LIBC_INLINE pid_t gettid() {
pid_t *cache = get_tid_cache();
if (LIBC_UNLIKELY(!cache || *cache <= 0))
return syscall_impl<pid_t>(SYS_gettid);
return *cache;
}

LIBC_INLINE void force_set_tid(pid_t tid) {
pid_t *cache = get_tid_cache();
if (cache)
*cache = tid;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_IDENTIFIER_H
1 change: 1 addition & 0 deletions libc/src/__support/threads/linux/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ add_header_library(
libc.src.__support.common
libc.src.__support.OSUtil.osutil
libc.src.__support.CPP.limits
libc.src.__support.threads.identifier
COMPILE_OPTIONS
-DLIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT=${LIBC_CONF_RWLOCK_DEFAULT_SPIN_COUNT}
${monotonicity_flags}
Expand Down
9 changes: 4 additions & 5 deletions libc/src/__support/threads/linux/rwlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/optimization.h"
#include "src/__support/threads/identifier.h"
#include "src/__support/threads/linux/futex_utils.h"
#include "src/__support/threads/linux/futex_word.h"
#include "src/__support/threads/linux/raw_mutex.h"
Expand Down Expand Up @@ -336,8 +337,6 @@ class RwLock {
LIBC_INLINE Role get_preference() const {
return static_cast<Role>(preference);
}
// TODO: use cached thread id once implemented.
LIBC_INLINE static pid_t gettid() { return syscall_impl<pid_t>(SYS_gettid); }

template <Role role> LIBC_INLINE LockResult try_lock(RwState &old) {
if constexpr (role == Role::Reader) {
Expand All @@ -359,7 +358,7 @@ class RwLock {
if (LIBC_LIKELY(old.compare_exchange_weak_with(
state, old.set_writer_bit(), cpp::MemoryOrder::ACQUIRE,
cpp::MemoryOrder::RELAXED))) {
writer_tid.store(gettid(), cpp::MemoryOrder::RELAXED);
writer_tid.store(internal::gettid(), cpp::MemoryOrder::RELAXED);
return LockResult::Success;
}
// Notice that old is updated by the compare_exchange_weak_with
Expand Down Expand Up @@ -394,7 +393,7 @@ class RwLock {
unsigned spin_count = LIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT) {
// Phase 1: deadlock detection.
// A deadlock happens if this is a RAW/WAW lock in the same thread.
if (writer_tid.load(cpp::MemoryOrder::RELAXED) == gettid())
if (writer_tid.load(cpp::MemoryOrder::RELAXED) == internal::gettid())
return LockResult::Deadlock;

#if LIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY
Expand Down Expand Up @@ -520,7 +519,7 @@ class RwLock {
if (old.has_active_writer()) {
// The lock is held by a writer.
// Check if we are the owner of the lock.
if (writer_tid.load(cpp::MemoryOrder::RELAXED) != gettid())
if (writer_tid.load(cpp::MemoryOrder::RELAXED) != internal::gettid())
return LockResult::PermissionDenied;
// clear writer tid.
writer_tid.store(0, cpp::MemoryOrder::RELAXED);
Expand Down
10 changes: 10 additions & 0 deletions libc/src/unistd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,13 @@ add_entrypoint_external(
add_entrypoint_external(
opterr
)

add_entrypoint_object(
gettid
SRCS
gettid.cpp
HDRS
gettid.h
DEPENDS
libc.src.__support.threads.identifier
)
17 changes: 17 additions & 0 deletions libc/src/unistd/gettid.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//===-- Implementation file for gettid --------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "src/unistd/gettid.h"
#include "src/__support/common.h"
#include "src/__support/threads/identifier.h"

namespace LIBC_NAMESPACE_DECL {

LLVM_LIBC_FUNCTION(pid_t, gettid, ()) { return internal::gettid(); }

} // namespace LIBC_NAMESPACE_DECL
21 changes: 21 additions & 0 deletions libc/src/unistd/gettid.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//===-- Implementation header for gettid ------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_LIBC_SRC_UNISTD_GETTID_H
#define LLVM_LIBC_SRC_UNISTD_GETTID_H

#include "hdr/types/pid_t.h"
#include "src/__support/common.h"

namespace LIBC_NAMESPACE_DECL {

pid_t gettid();

} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC_UNISTD_GETTID_H
1 change: 1 addition & 0 deletions libc/src/unistd/linux/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ add_entrypoint_object(
libc.src.__support.OSUtil.osutil
libc.src.__support.threads.thread
libc.src.errno.errno
libc.src.__support.threads.identifier
)

add_entrypoint_object(
Expand Down
16 changes: 12 additions & 4 deletions libc/src/unistd/linux/fork.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "src/__support/common.h"
#include "src/__support/macros/config.h"
#include "src/__support/threads/fork_callbacks.h"
#include "src/__support/threads/identifier.h"
#include "src/__support/threads/thread.h" // For thread self object

#include "src/errno/libc_errno.h"
Expand All @@ -25,19 +26,25 @@ namespace LIBC_NAMESPACE_DECL {

LLVM_LIBC_FUNCTION(pid_t, fork, (void)) {
invoke_prepare_callbacks();
pid_t parent_tid = internal::gettid();
// Invalidate parent's tid cache before forking. We cannot do this in child
// process because in the post-fork instruction windows, there may be a signal
// handler triggered which may get the wrong tid.
internal::force_set_tid(0);
#ifdef SYS_fork
pid_t ret = LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_fork);
pid_t ret = syscall_impl<pid_t>(SYS_fork);
#elif defined(SYS_clone)
pid_t ret = LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_clone, SIGCHLD, 0);
pid_t ret = syscall_impl<pid_t>(SYS_clone, SIGCHLD, 0);
#else
#error "fork and clone syscalls not available."
#endif

if (ret == 0) {
// Return value is 0 in the child process.
// The child is created with a single thread whose self object will be a
// copy of parent process' thread which called fork. So, we have to fix up
// the child process' self object with the new process' tid.
self.attrib->tid = LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_gettid);
internal::force_set_tid(syscall_impl<pid_t>(SYS_gettid));
invoke_child_callbacks();
return 0;
}
Expand All @@ -47,7 +54,8 @@ LLVM_LIBC_FUNCTION(pid_t, fork, (void)) {
libc_errno = static_cast<int>(-ret);
return -1;
}

// recover parent's tid.
internal::force_set_tid(parent_tid);
invoke_parent_callbacks();
return ret;
}
Expand Down
4 changes: 4 additions & 0 deletions libc/test/integration/src/unistd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ add_integration_test(
libc.src.sys.wait.wait4
libc.src.sys.wait.waitpid
libc.src.unistd.fork
libc.src.unistd.gettid
libc.src.__support.OSUtil.osutil
libc.src.stdlib.exit
libc.include.sys_syscall
)

if((${LIBC_TARGET_OS} STREQUAL "linux") AND (${LIBC_TARGET_ARCHITECTURE_IS_X86}))
Expand Down
21 changes: 21 additions & 0 deletions libc/test/integration/src/unistd/fork_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@
//
//===----------------------------------------------------------------------===//

#include "src/__support/OSUtil/syscall.h"
#include "src/pthread/pthread_atfork.h"
#include "src/signal/raise.h"
#include "src/stdlib/exit.h"
#include "src/sys/wait/wait.h"
#include "src/sys/wait/wait4.h"
#include "src/sys/wait/waitpid.h"
#include "src/unistd/fork.h"
#include "src/unistd/gettid.h"

#include "test/IntegrationTest/test.h"

#include <errno.h>
#include <signal.h>
#include <sys/syscall.h>
#include <sys/wait.h>
#include <unistd.h>

Expand Down Expand Up @@ -140,7 +144,24 @@ void fork_with_atfork_callbacks() {
ASSERT_NE(child, DONE);
}

void gettid_test() {
// fork and verify tid is consistent with the syscall result.
int pid = LIBC_NAMESPACE::fork();
ASSERT_EQ(LIBC_NAMESPACE::gettid(),
LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_gettid));
if (pid == 0)
LIBC_NAMESPACE::exit(0);
// make sure child process exits normally
int status;
pid_t cpid = LIBC_NAMESPACE::waitpid(pid, &status, 0);
ASSERT_TRUE(cpid > 0);
ASSERT_EQ(cpid, pid);
ASSERT_TRUE(WIFEXITED(status));
ASSERT_EQ(WEXITSTATUS(status), 0);
}

TEST_MAIN(int argc, char **argv, char **envp) {
gettid_test();
fork_and_wait_normal_exit();
fork_and_wait4_normal_exit();
fork_and_waitpid_normal_exit();
Expand Down
Loading