Skip to content

Conversation

SchrodingerZhu
Copy link
Contributor

@SchrodingerZhu SchrodingerZhu commented Nov 21, 2024

clock_gettime being a header library discards the dependency chain behind it.

@llvmbot llvmbot added the libc label Nov 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 21, 2024

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/117224.diff

3 Files Affected:

  • (modified) libc/src/__support/time/linux/CMakeLists.txt (+3-1)
  • (added) libc/src/__support/time/linux/clock_gettime.cpp (+62)
  • (modified) libc/src/__support/time/linux/clock_gettime.h (+1-41)
diff --git a/libc/src/__support/time/linux/CMakeLists.txt b/libc/src/__support/time/linux/CMakeLists.txt
index f038cb8854b9b8..94ed09e6521524 100644
--- a/libc/src/__support/time/linux/CMakeLists.txt
+++ b/libc/src/__support/time/linux/CMakeLists.txt
@@ -1,7 +1,9 @@
-add_header_library(
+add_object_library(
   clock_gettime
   HDRS
     clock_gettime.h
+  SRCS
+    clock_gettime.cpp
   DEPENDS
     libc.include.sys_syscall
     libc.hdr.types.struct_timespec
diff --git a/libc/src/__support/time/linux/clock_gettime.cpp b/libc/src/__support/time/linux/clock_gettime.cpp
new file mode 100644
index 00000000000000..3a0eca417724ac
--- /dev/null
+++ b/libc/src/__support/time/linux/clock_gettime.cpp
@@ -0,0 +1,62 @@
+//===--- clock_gettime linux 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/time/linux/clock_gettime.h"
+#include "hdr/types/clockid_t.h"
+#include "hdr/types/struct_timespec.h"
+#include "src/__support/OSUtil/linux/vdso.h"
+#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/common.h"
+#include "src/__support/error_or.h"
+#include "src/__support/macros/config.h"
+#include <sys/syscall.h>
+
+#if defined(SYS_clock_gettime64)
+#include <linux/time_types.h>
+#endif
+
+namespace LIBC_NAMESPACE_DECL {
+namespace internal {
+ErrorOr<int> clock_gettime(clockid_t clockid, timespec *ts) {
+  using namespace vdso;
+  int ret;
+#if defined(SYS_clock_gettime)
+  TypedSymbol<VDSOSym::ClockGetTime> clock_gettime;
+  if (LIBC_LIKELY(clock_gettime != nullptr))
+    ret = clock_gettime(clockid, ts);
+  else
+    ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_clock_gettime,
+                                            static_cast<long>(clockid),
+                                            reinterpret_cast<long>(ts));
+#elif defined(SYS_clock_gettime64)
+  static_assert(
+      sizeof(time_t) == sizeof(int64_t),
+      "SYS_clock_gettime64 requires struct timespec with 64-bit members.");
+
+  TypedSymbol<VDSOSym::ClockGetTime64> clock_gettime64;
+  __kernel_timespec ts64{};
+  if (LIBC_LIKELY(clock_gettime64 != nullptr))
+    ret = clock_gettime64(clockid, &ts64);
+  else
+    ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_clock_gettime64,
+                                            static_cast<long>(clockid),
+                                            reinterpret_cast<long>(&ts64));
+  if (ret == 0) {
+    ts->tv_sec = static_cast<decltype(ts->tv_sec)>(ts64.tv_sec);
+    ts->tv_nsec = static_cast<decltype(ts->tv_nsec)>(ts64.tv_nsec);
+  }
+#else
+#error "SYS_clock_gettime and SYS_clock_gettime64 syscalls not available."
+#endif
+  if (ret < 0)
+    return Error(-ret);
+  return ret;
+}
+
+} // namespace internal
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/__support/time/linux/clock_gettime.h b/libc/src/__support/time/linux/clock_gettime.h
index 517cca91391a74..f7f996ce7c1975 100644
--- a/libc/src/__support/time/linux/clock_gettime.h
+++ b/libc/src/__support/time/linux/clock_gettime.h
@@ -11,12 +11,7 @@
 
 #include "hdr/types/clockid_t.h"
 #include "hdr/types/struct_timespec.h"
-#include "src/__support/OSUtil/linux/vdso.h"
-#include "src/__support/OSUtil/syscall.h"
-#include "src/__support/common.h"
 #include "src/__support/error_or.h"
-#include "src/__support/macros/config.h"
-#include <sys/syscall.h>
 
 #if defined(SYS_clock_gettime64)
 #include <linux/time_types.h>
@@ -24,42 +19,7 @@
 
 namespace LIBC_NAMESPACE_DECL {
 namespace internal {
-LIBC_INLINE ErrorOr<int> clock_gettime(clockid_t clockid, timespec *ts) {
-  using namespace vdso;
-  int ret;
-#if defined(SYS_clock_gettime)
-  TypedSymbol<VDSOSym::ClockGetTime> clock_gettime;
-  if (LIBC_LIKELY(clock_gettime != nullptr))
-    ret = clock_gettime(clockid, ts);
-  else
-    ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_clock_gettime,
-                                            static_cast<long>(clockid),
-                                            reinterpret_cast<long>(ts));
-#elif defined(SYS_clock_gettime64)
-  static_assert(
-      sizeof(time_t) == sizeof(int64_t),
-      "SYS_clock_gettime64 requires struct timespec with 64-bit members.");
-
-  TypedSymbol<VDSOSym::ClockGetTime64> clock_gettime64;
-  __kernel_timespec ts64{};
-  if (LIBC_LIKELY(clock_gettime64 != nullptr))
-    ret = clock_gettime64(clockid, &ts64);
-  else
-    ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_clock_gettime64,
-                                            static_cast<long>(clockid),
-                                            reinterpret_cast<long>(&ts64));
-  if (ret == 0) {
-    ts->tv_sec = static_cast<decltype(ts->tv_sec)>(ts64.tv_sec);
-    ts->tv_nsec = static_cast<decltype(ts->tv_nsec)>(ts64.tv_nsec);
-  }
-#else
-#error "SYS_clock_gettime and SYS_clock_gettime64 syscalls not available."
-#endif
-  if (ret < 0)
-    return Error(-ret);
-  return ret;
-}
-
+ErrorOr<int> clock_gettime(clockid_t clockid, timespec *ts);
 } // namespace internal
 } // namespace LIBC_NAMESPACE_DECL
 

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this seems like a good fix to the issue

@SchrodingerZhu SchrodingerZhu merged commit 1b413c8 into llvm:main Nov 21, 2024
7 of 8 checks passed
@SchrodingerZhu SchrodingerZhu deleted the libc/clock_gettime_fix branch November 21, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants