[libc] implement cached process/thread identity #95965

SchrodingerZhu · 2024-06-18T18:13:26Z

No description provided.

llvmbot · 2024-06-18T18:14:15Z

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

Patch is 21.25 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/95965.diff

24 Files Affected:

(modified) libc/config/linux/aarch64/entrypoints.txt (+1)
(modified) libc/config/linux/riscv/entrypoints.txt (+1)
(modified) libc/config/linux/x86_64/entrypoints.txt (+1)
(modified) libc/docs/dev/undefined_behavior.rst (+21)
(modified) libc/spec/posix.td (+5-10)
(modified) libc/src/__support/OSUtil/CMakeLists.txt (+9)
(modified) libc/src/__support/OSUtil/linux/CMakeLists.txt (+13)
(added) libc/src/__support/OSUtil/linux/pid.cpp (+20)
(added) libc/src/__support/OSUtil/pid.h (+32)
(modified) libc/src/__support/threads/CMakeLists.txt (+19)
(modified) libc/src/__support/threads/linux/CMakeLists.txt (+1)
(modified) libc/src/__support/threads/linux/rwlock.h (+4-5)
(modified) libc/src/__support/threads/linux/thread.cpp (+2)
(modified) libc/src/__support/threads/thread.h (+12-1)
(added) libc/src/__support/threads/tid.h (+34)
(modified) libc/src/unistd/CMakeLists.txt (+10)
(modified) libc/src/unistd/getpid.h (+2-2)
(added) libc/src/unistd/gettid.cpp (+17)
(added) libc/src/unistd/gettid.h (+21)
(modified) libc/src/unistd/linux/CMakeLists.txt (+2-2)
(modified) libc/src/unistd/linux/fork.cpp (+18-10)
(modified) libc/src/unistd/linux/getpid.cpp (+2-8)
(modified) libc/test/src/unistd/CMakeLists.txt (+10)
(added) libc/test/src/unistd/gettid_test.cpp (+15)

diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index dfed6acbdf257..a2643bda8277f 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -290,6 +290,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
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index e12d6b3957e51..ae0c64045da88 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -295,6 +295,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
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index cfe35167ca32e..0380d4a3734e8 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -308,6 +308,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
diff --git a/libc/docs/dev/undefined_behavior.rst b/libc/docs/dev/undefined_behavior.rst
index c97a539ca8da4..9f31aeff9ab97 100644
--- a/libc/docs/dev/undefined_behavior.rst
+++ b/libc/docs/dev/undefined_behavior.rst
@@ -89,3 +89,24 @@ The C23 standard states that if the value of the ``rnd`` argument of the
 the value of a math rounding direction macro, the direction of rounding is
 unspecified. LLVM's libc chooses to use the ``FP_INT_TONEAREST`` rounding
 direction in this case.
+
+Cached ``getpid/gettid``
+------------------------
+Since version ``2.25``, glibc removes its cache mechanism for ``getpid/gettid`` 
+(See the history section in https://man7.org/linux/man-pages/man2/getpid.2.html).
+LLVM's libc still implements the cache as it is useful for fast deadlock detection.
+The cache mechanism is also implemented in MUSL and bionic.
+
+Unwrapped ``SYS_clone/SYS_fork/SYS_vfork``
+------------------------------------------
+It is highly discouraged to use unwrapped ``SYS_clone/SYS_fork/SYS_vfork``. 
+First, calling such syscalls without provided libc wrappers ignores 
+all the ``pthread_atfork`` entries as libc can no longer detect the ``fork``. 
+Second, libc relies on the ``fork/clone`` wrappers to correctly maintain cache for
+process id and thread id, and other important process-specific states such as the list 
+of robust mutexes. Third, even if the user is to call ``exec*`` functions immediately, 
+there can still be other unexpected issues. For instance, there can be signal handlers 
+inherited from parent process triggered inside the instruction window between ``fork`` 
+and ``exec*``. As libc failed to maintain its internal states correctly, even though the
+functions used inside the signal handlers are marked as ``async-signal-safe`` (such as
+``getpid``), they will still return wrong values or lead to other even worse situations.
diff --git a/libc/spec/posix.td b/libc/spec/posix.td
index d14047548e104..c3996a213f60b 100644
--- a/libc/spec/posix.td
+++ b/libc/spec/posix.td
@@ -512,6 +512,11 @@ def POSIX : StandardSpec<"POSIX"> {
           RetValSpec<PidT>,
           [ArgSpec<VoidType>]
         >,
+        FunctionSpec<
+          "gettid",
+          RetValSpec<PidT>,
+          [ArgSpec<VoidType>]
+        >,
         FunctionSpec<
           "getuid",
           RetValSpec<UidT>,
@@ -567,16 +572,6 @@ def POSIX : StandardSpec<"POSIX"> {
           RetValSpec<IntType>,
           [ArgSpec<ConstCharPtr>]
         >,
-        FunctionSpec<
-          "getpid",
-          RetValSpec<IntType>,
-          [ArgSpec<VoidType>]
-        >,
-        FunctionSpec<
-          "getppid",
-          RetValSpec<IntType>,
-          [ArgSpec<VoidType>]
-        >,
         FunctionSpec<
           "link",
           RetValSpec<IntType>,
diff --git a/libc/src/__support/OSUtil/CMakeLists.txt b/libc/src/__support/OSUtil/CMakeLists.txt
index 94d1042ccbb4a..2db72f40c6f18 100644
--- a/libc/src/__support/OSUtil/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/CMakeLists.txt
@@ -15,3 +15,12 @@ add_object_library(
   DEPENDS
     ${target_os_util}
 )
+
+if(TARGET libc.src.__support.OSUtil.${LIBC_TARGET_OS}.pid)
+  add_object_library(
+    pid
+    ALIAS
+    DEPENDS
+      .${LIBC_TARGET_OS}.pid
+  )
+endif()
diff --git a/libc/src/__support/OSUtil/linux/CMakeLists.txt b/libc/src/__support/OSUtil/linux/CMakeLists.txt
index 78b117fd19439..ddc51aadd0d02 100644
--- a/libc/src/__support/OSUtil/linux/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/CMakeLists.txt
@@ -22,3 +22,16 @@ add_object_library(
     libc.hdr.types.struct_flock64
     libc.hdr.types.struct_f_owner_ex
 )
+
+add_object_library(
+  pid
+  SRCS
+    pid.cpp
+  HDRS
+    ../pid.h
+  DEPENDS
+    libc.src.__support.OSUtil.osutil
+    libc.src.__support.common
+    libc.hdr.types.pid_t
+    libc.include.sys_syscall
+)
diff --git a/libc/src/__support/OSUtil/linux/pid.cpp b/libc/src/__support/OSUtil/linux/pid.cpp
new file mode 100644
index 0000000000000..47ef00565cbe9
--- /dev/null
+++ b/libc/src/__support/OSUtil/linux/pid.cpp
@@ -0,0 +1,20 @@
+//===------------ pid_t utilities implementation ----------------*- 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/__support/OSUtil/pid.h"
+#include "src/__support/OSUtil/syscall.h"
+#include <sys/syscall.h>
+
+namespace LIBC_NAMESPACE {
+
+pid_t ProcessIdentity::cache = -1;
+pid_t ProcessIdentity::get_uncached() {
+  return syscall_impl<pid_t>(SYS_getpid);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/__support/OSUtil/pid.h b/libc/src/__support/OSUtil/pid.h
new file mode 100644
index 0000000000000..5fda6726308e1
--- /dev/null
+++ b/libc/src/__support/OSUtil/pid.h
@@ -0,0 +1,32 @@
+//===------------ pid_t utilities -------------------------------*- 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_OSUTIL_PID_H
+#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_PID_H
+#include "hdr/types/pid_t.h"
+#include "src/__support/macros/attributes.h"
+#include "src/__support/macros/optimization.h"
+namespace LIBC_NAMESPACE {
+
+class ProcessIdentity {
+  static pid_t cache;
+  static pid_t get_uncached();
+
+public:
+  LIBC_INLINE static void invalidate_cache() { cache = -1; }
+  LIBC_INLINE static void refresh_cache() { cache = get_uncached(); }
+  LIBC_INLINE static pid_t get() {
+    if (LIBC_UNLIKELY(cache < 0))
+      return get_uncached();
+    return cache;
+  }
+};
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_PID_H
diff --git a/libc/src/__support/threads/CMakeLists.txt b/libc/src/__support/threads/CMakeLists.txt
index 9ea0b59befe7a..ef8769ff9498e 100644
--- a/libc/src/__support/threads/CMakeLists.txt
+++ b/libc/src/__support/threads/CMakeLists.txt
@@ -45,6 +45,7 @@ add_header_library(
     libc.src.__support.CPP.optional
     libc.src.__support.CPP.string_view
     libc.src.__support.CPP.stringstream
+    libc.hdr.types.pid_t
 )
 
 if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.thread)
@@ -80,3 +81,21 @@ if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.CndVar)
     .${LIBC_TARGET_OS}.CndVar
   )
 endif()
+
+set(tid_dep)
+if (LLVM_LIBC_FULL_BUILD)
+  list(APPEND tid_dep libc.src.__support.thread)
+else()
+  list(APPEND tid_dep libc.src.__support.OSUtil.osutil)
+  list(APPEND tid_dep libc.include.sys_syscall)
+endif()
+
+add_header_library(
+  tid
+  HDRS
+    tid.h
+  DEPENDS
+    libc.src.__support.common
+    libc.hdr.types.pid_t
+    ${tid_dep}
+)
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index 95e509b7a825d..832617afb6068 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -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.tid
   COMPILE_OPTIONS
     -DLIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT=${LIBC_CONF_RWLOCK_DEFAULT_SPIN_COUNT}
     ${monotonicity_flags}
diff --git a/libc/src/__support/threads/linux/rwlock.h b/libc/src/__support/threads/linux/rwlock.h
index 201fe92c37fc0..1faeeec1bb025 100644
--- a/libc/src/__support/threads/linux/rwlock.h
+++ b/libc/src/__support/threads/linux/rwlock.h
@@ -22,6 +22,7 @@
 #include "src/__support/threads/linux/futex_word.h"
 #include "src/__support/threads/linux/raw_mutex.h"
 #include "src/__support/threads/sleep.h"
+#include "src/__support/threads/tid.h"
 
 #ifndef LIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT
 #define LIBC_COPT_RWLOCK_DEFAULT_SPIN_COUNT 100
@@ -335,8 +336,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) {
@@ -358,7 +357,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(gettid_inline(), cpp::MemoryOrder::RELAXED);
           return LockResult::Success;
         }
         // Notice that old is updated by the compare_exchange_weak_with
@@ -393,7 +392,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) == gettid_inline())
       return LockResult::Deadlock;
 
 #if LIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY
@@ -519,7 +518,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) != gettid_inline())
         return LockResult::PermissionDenied;
       // clear writer tid.
       writer_tid.store(0, cpp::MemoryOrder::RELAXED);
diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp
index 1d986ff38cfff..493f2df7f8301 100644
--- a/libc/src/__support/threads/linux/thread.cpp
+++ b/libc/src/__support/threads/linux/thread.cpp
@@ -517,4 +517,6 @@ void thread_exit(ThreadReturnValue retval, ThreadStyle style) {
   __builtin_unreachable();
 }
 
+pid_t Thread::get_uncached_tid() { return syscall_impl<pid_t>(SYS_gettid); }
+
 } // namespace LIBC_NAMESPACE
diff --git a/libc/src/__support/threads/thread.h b/libc/src/__support/threads/thread.h
index acfe33879f878..e53ff9995f678 100644
--- a/libc/src/__support/threads/thread.h
+++ b/libc/src/__support/threads/thread.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_THREADS_THREAD_H
 #define LLVM_LIBC_SRC___SUPPORT_THREADS_THREAD_H
 
+#include "hdr/types/pid_t.h"
 #include "src/__support/CPP/atomic.h"
 #include "src/__support/CPP/optional.h"
 #include "src/__support/CPP/string_view.h"
@@ -99,7 +100,7 @@ struct alignas(STACK_ALIGNMENT) ThreadAttributes {
   uintptr_t tls;                // Address to the thread TLS memory
   uintptr_t tls_size;           // The size of area pointed to by |tls|.
   unsigned char owned_stack; // Indicates if the thread owns this stack memory
-  int tid;
+  pid_t tid;
   ThreadStyle style;
   ThreadReturnValue retval;
   ThreadAtExitCallbackMgr *atexit_callback_mgr;
@@ -224,6 +225,16 @@ struct Thread {
 
   // Return the name of the thread in |name|. Return the error number of error.
   int get_name(cpp::StringStream &name) const;
+
+  static pid_t get_uncached_tid();
+
+  LIBC_INLINE void refresh_tid() { this->attrib->tid = get_uncached_tid(); }
+  LIBC_INLINE void invalidate_tid() { this->attrib->tid = -1; }
+  LIBC_INLINE pid_t get_tid() {
+    if (LIBC_UNLIKELY(this->attrib->tid < 0))
+      return get_uncached_tid();
+    return this->attrib->tid;
+  }
 };
 
 extern LIBC_THREAD_LOCAL Thread self;
diff --git a/libc/src/__support/threads/tid.h b/libc/src/__support/threads/tid.h
new file mode 100644
index 0000000000000..622f91543af7e
--- /dev/null
+++ b/libc/src/__support/threads/tid.h
@@ -0,0 +1,34 @@
+//===--- Tid wrapper --------------------------------------------*- 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_TID_H
+#define LLVM_LIBC_SRC___SUPPORT_THREADS_TID_H
+
+// This header is for internal usage which automatically dispatches full build
+// and overlay build behaviors.
+
+#include "hdr/types/pid_t.h"
+#include "src/__support/common.h"
+#ifdef LIBC_FULL_BUILD
+#include "src/__support/threads/thread.h"
+#else
+#include "src/__support/OSUtil/syscall.h"
+#include <sys/syscall.h>
+#endif // LIBC_FULL_BUILD
+
+namespace LIBC_NAMESPACE {
+LIBC_INLINE pid_t gettid_inline() {
+#ifdef LIBC_FULL_BUILD
+  return self.get_tid();
+#else
+  return syscall_impl<pid_t>(SYS_gettid);
+#endif
+}
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_TID_H
diff --git a/libc/src/unistd/CMakeLists.txt b/libc/src/unistd/CMakeLists.txt
index 77db76518350c..959595478bb7f 100644
--- a/libc/src/unistd/CMakeLists.txt
+++ b/libc/src/unistd/CMakeLists.txt
@@ -318,3 +318,13 @@ add_entrypoint_external(
 add_entrypoint_external(
   opterr
 )
+
+add_entrypoint_object(
+  gettid
+  SRCS
+    gettid.cpp
+  HDRS
+    gettid.h
+  DEPENDS
+    libc.src.__support.threads.tid
+)
diff --git a/libc/src/unistd/getpid.h b/libc/src/unistd/getpid.h
index 5890dbf63681a..d7f589da81913 100644
--- a/libc/src/unistd/getpid.h
+++ b/libc/src/unistd/getpid.h
@@ -9,11 +9,11 @@
 #ifndef LLVM_LIBC_SRC_UNISTD_GETPID_H
 #define LLVM_LIBC_SRC_UNISTD_GETPID_H
 
-#include <unistd.h>
+#include "hdr/types/pid_t.h"
 
 namespace LIBC_NAMESPACE {
 
-pid_t getpid();
+pid_t getpid(void);
 
 } // namespace LIBC_NAMESPACE
 
diff --git a/libc/src/unistd/gettid.cpp b/libc/src/unistd/gettid.cpp
new file mode 100644
index 0000000000000..040a1323dcad1
--- /dev/null
+++ b/libc/src/unistd/gettid.cpp
@@ -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/tid.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(pid_t, gettid, (void)) { return gettid_inline(); }
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/unistd/gettid.h b/libc/src/unistd/gettid.h
new file mode 100644
index 0000000000000..01691b74895f7
--- /dev/null
+++ b/libc/src/unistd/gettid.h
@@ -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 {
+
+pid_t gettid(void);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_UNISTD_GETTID_H
diff --git a/libc/src/unistd/linux/CMakeLists.txt b/libc/src/unistd/linux/CMakeLists.txt
index 7d831f9c29c74..4648870c708de 100644
--- a/libc/src/unistd/linux/CMakeLists.txt
+++ b/libc/src/unistd/linux/CMakeLists.txt
@@ -101,6 +101,7 @@ add_entrypoint_object(
     libc.include.sys_syscall
     libc.src.__support.threads.fork_callbacks
     libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.pid
     libc.src.__support.threads.thread
     libc.src.errno.errno
 )
@@ -190,8 +191,7 @@ add_entrypoint_object(
     ../getpid.h
   DEPENDS
     libc.include.unistd
-    libc.include.sys_syscall
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.pid
 )
 
 add_entrypoint_object(
diff --git a/libc/src/unistd/linux/fork.cpp b/libc/src/unistd/linux/fork.cpp
index 6fa2b990b19a9..98721e2f5e1d8 100644
--- a/libc/src/unistd/linux/fork.cpp
+++ b/libc/src/unistd/linux/fork.cpp
@@ -8,12 +8,13 @@
 
 #include "src/unistd/fork.h"
 
+#include "src/__support/OSUtil/pid.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 #include "src/__support/threads/fork_callbacks.h"
 #include "src/__support/threads/thread.h" // For thread self object
-
 #include "src/errno/libc_errno.h"
+
 #include <signal.h>      // For SIGCHLD
 #include <sys/syscall.h> // For syscall numbers.
 
@@ -24,6 +25,13 @@ namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(pid_t, fork, (void)) {
   invoke_prepare_callbacks();
+
+  // Invalidate tid/pid cache before fork to avoid post fork signal handler from
+  // getting wrong values. gettid() is not async-signal-safe, but let's provide
+  // our best efforts here.
+  self.invalidate_tid();
+  ProcessIdentity::invalidate_cache();
+
 #ifdef SYS_fork
   pid_t ret = LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_fork);
 #elif defined(SYS_clone)
@@ -31,15 +39,6 @@ LLVM_LIBC_FUNCTION(pid_t, fork, (void)) {
 #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);
-    invoke_child_callbacks();
-    return 0;
-  }
 
   if (ret < 0) {
     // Error case, a child process was not created.
@@ -47,6 +46,15 @@ LLVM_LIBC_FUNCTION(pid_t, fork, (void)) {
     return -1;
   }
 
+  // Refresh tid/pid cache after fork
+  self.refresh_tid();
+  ProcessIdentity::refresh_cache();
+
+  if (ret == 0) {
+    invoke_child_callbacks();
+    return 0;
+  }
+
   invoke_parent_callbacks();
   return ret;
 }
diff --git a/libc/src/unistd/linux/getpid.cpp b/libc/src/unistd/linux/getpid.cpp
index 85730738ca2ef..7446f2208d596 100644
--- a/libc/src/unistd/linux/getpid.cpp
+++ b/libc/src/unistd/linux/getpid.cpp
@@ -7,16 +7,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/unistd/getpid.h"
-
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/OSUtil/pid.h"
 #include "src/__support/common.h"
-
-#include <sys/syscall.h> // For syscall numbers.
-
 namespace LIBC_NAMESPACE {
 
-LLVM_LIBC_FUNCTION(pid_t, getpid, ()) {
-  return LIBC_NAMESPACE::syscall_impl<pid_t>(SYS_getpid);
-}
+LLVM_LIBC_FUNCTION(pid_t, getpid, (void)) { return ProcessIdentity::get(); }
 
 } // namespace LIBC_NAMESPACE
diff --git...
[truncated]

michaelrj-google · 2024-06-18T21:52:07Z

libc/spec/posix.td

@@ -567,16 +572,6 @@ def POSIX : StandardSpec<"POSIX"> {
          RetValSpec<IntType>,
          [ArgSpec<ConstCharPtr>]
        >,
-        FunctionSpec<


why were these deleted?

They were repeated somehow with different types. There are already definitions using PidT in the above. Were they repeated on purpose?

libc/docs/dev/undefined_behavior.rst

libc/src/__support/OSUtil/pid.h

libc/src/unistd/linux/fork.cpp

libc/src/__support/OSUtil/pid.h

nickdesaulniers · 2024-06-20T20:56:49Z

libc/src/__support/threads/linux/thread.cpp

@@ -517,4 +517,6 @@ void thread_exit(ThreadReturnValue retval, ThreadStyle style) {
  __builtin_unreachable();
 }

+pid_t Thread::get_uncached_tid() { return syscall_impl<pid_t>(SYS_gettid); }


Can we clean up the number of places that we call SYS_gettid directly? Looks like gettid_inline does this, too.

Co-authored-by: Nick Desaulniers (paternity leave) <[email protected]>

michaelrj-google

LGTM

QuarticCat

LGTM

SchrodingerZhu · 2024-07-16T04:09:46Z

This PR somehow gets 50k+ commits diverged from main. I think it is a git bug but it makes me really hard to reconcile the changes.

SchrodingerZhu · 2024-07-16T04:44:11Z

superseded by #98989

migrated from #95965 due to corrupted git history

SchrodingerZhu · 2024-07-18T20:28:37Z

@QuarticCat maybe this is a git hash conflict problem?

Summary: migrated from #95965 due to corrupted git history Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251445

SchrodingerZhu requested review from nickdesaulniers and QuarticCat June 18, 2024 18:13

llvmbot added the libc label Jun 18, 2024

michaelrj-google reviewed Jun 18, 2024

View reviewed changes

SchrodingerZhu commented Jun 18, 2024

View reviewed changes

libc/src/__support/OSUtil/pid.h Outdated Show resolved Hide resolved

SchrodingerZhu requested a review from michaelrj-google June 19, 2024 19:33

QuarticCat reviewed Jun 20, 2024

View reviewed changes

libc/src/unistd/linux/fork.cpp Outdated Show resolved Hide resolved

nickdesaulniers reviewed Jun 20, 2024

View reviewed changes

Yifan Zhu and others added 5 commits June 30, 2024 20:48

[libc] implement cached process/thread identity

bdf5f91

initialize pid cache in startup

5d332d6

add TLS-based fork-inflight guard

774b813

add options and docs

84588f4

add integration test

038db32

SchrodingerZhu force-pushed the identity branch from bcaf87f to 038db32 Compare July 1, 2024 03:54

Update libc/src/__support/OSUtil/pid.h

f3ad266

Co-authored-by: Nick Desaulniers (paternity leave) <[email protected]>

michaelrj-google approved these changes Jul 8, 2024

View reviewed changes

address CR

f9aeea8

QuarticCat approved these changes Jul 16, 2024

View reviewed changes

SchrodingerZhu mentioned this pull request Jul 16, 2024

[libc] implement cached process/thread identity #98989

Merged

SchrodingerZhu closed this Jul 16, 2024

SchrodingerZhu deleted the identity branch July 16, 2024 04:44

SchrodingerZhu added a commit that referenced this pull request Jul 18, 2024

[libc] implement cached process/thread identity (#98989)

5c9fc3c

migrated from #95965 due to corrupted git history

SchrodingerZhu restored the identity branch July 18, 2024 20:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc] implement cached process/thread identity #95965

[libc] implement cached process/thread identity #95965

Uh oh!

SchrodingerZhu commented Jun 18, 2024

Uh oh!

llvmbot commented Jun 18, 2024

Uh oh!

michaelrj-google Jun 18, 2024

Uh oh!

SchrodingerZhu Jun 18, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nickdesaulniers Jun 20, 2024

Uh oh!

michaelrj-google left a comment

Uh oh!

QuarticCat left a comment

Uh oh!

SchrodingerZhu commented Jul 16, 2024

Uh oh!

SchrodingerZhu commented Jul 16, 2024

Uh oh!

SchrodingerZhu commented Jul 18, 2024

Uh oh!

Uh oh!

[libc] implement cached process/thread identity #95965

[libc] implement cached process/thread identity #95965

Uh oh!

Conversation

SchrodingerZhu commented Jun 18, 2024

Uh oh!

llvmbot commented Jun 18, 2024

Uh oh!

michaelrj-google Jun 18, 2024

Choose a reason for hiding this comment

Uh oh!

SchrodingerZhu Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nickdesaulniers Jun 20, 2024

Choose a reason for hiding this comment

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

QuarticCat left a comment

Choose a reason for hiding this comment

Uh oh!

SchrodingerZhu commented Jul 16, 2024

Uh oh!

SchrodingerZhu commented Jul 16, 2024

Uh oh!

SchrodingerZhu commented Jul 18, 2024

Uh oh!

Uh oh!

SchrodingerZhu Jun 18, 2024 •

edited

Loading