[libc-commits] [libc] 4c182df - [libc] Fix suseconds_t definition and utimes_test (#134326)

[via libc-commits]

Author: Michael Jones
Date: 2025-04-04T12:53:46-07:00
New Revision: 4c182df633bcd7fd7f0634b2cf6bcab91c3674ec

URL: https://github.com/llvm/llvm-project/commit/4c182df633bcd7fd7f0634b2cf6bcab91c3674ec
DIFF: https://github.com/llvm/llvm-project/commit/4c182df633bcd7fd7f0634b2cf6bcab91c3674ec.diff

LOG: [libc] Fix suseconds_t definition and utimes_test (#134326)

The main issue was that the kernel expected `suseconds_t` to be 64 bits
but ours was 32. This caused inconsistent failures since all valid
`suseconds_t` values are less than 1000000 (1 million), and some
configurations caused `struct timeval` to be padded to 128 bits.

Also: forgot to use TEST_FILE instead of FILE_PATH in some places.

Added: 
    

Modified: 
    libc/include/llvm-libc-types/suseconds_t.h
    libc/test/src/sys/time/CMakeLists.txt
    libc/test/src/sys/time/utimes_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/include/llvm-libc-types/suseconds_t.h b/libc/include/llvm-libc-types/suseconds_t.h
index 32ecc9f537d00..8e926e8401f5c 100644
--- a/libc/include/llvm-libc-types/suseconds_t.h
+++ b/libc/include/llvm-libc-types/suseconds_t.h
@@ -9,6 +9,11 @@
 #ifndef LLVM_LIBC_TYPES_SUSECONDS_T_H
 #define LLVM_LIBC_TYPES_SUSECONDS_T_H
 
-typedef __INT32_TYPE__ suseconds_t;
+// Per posix: suseconds_t shall be a signed integer type capable of storing
+// values at least in the range [-1, 1000000]. [...] the widths of [other
+// types...] and suseconds_t are no greater than the width of type long.
+
+// The kernel expects 64 bit suseconds_t at least on x86_64.
+typedef long suseconds_t;
 
 #endif // LLVM_LIBC_TYPES_SUSECONDS_T_H

diff  --git a/libc/test/src/sys/time/CMakeLists.txt b/libc/test/src/sys/time/CMakeLists.txt
index c092d33e43d85..72a65eec00937 100644
--- a/libc/test/src/sys/time/CMakeLists.txt
+++ b/libc/test/src/sys/time/CMakeLists.txt
@@ -8,6 +8,7 @@ add_libc_unittest(
     utimes_test.cpp
   DEPENDS
     libc.hdr.fcntl_macros
+    libc.hdr.sys_stat_macros
     libc.src.errno.errno
     libc.src.fcntl.open
     libc.src.sys.time.utimes
@@ -16,4 +17,5 @@ add_libc_unittest(
     libc.src.unistd.write
     libc.src.stdio.remove
     libc.src.sys.stat.stat
+    libc.test.UnitTest.ErrnoCheckingTest
 )

diff  --git a/libc/test/src/sys/time/utimes_test.cpp b/libc/test/src/sys/time/utimes_test.cpp
index 69607ba928e1e..66e69a1b2a700 100644
--- a/libc/test/src/sys/time/utimes_test.cpp
+++ b/libc/test/src/sys/time/utimes_test.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "hdr/fcntl_macros.h"
+#include "hdr/sys_stat_macros.h"
 #include "hdr/types/struct_timeval.h"
 #include "src/errno/libc_errno.h"
 #include "src/fcntl/open.h"
@@ -14,17 +15,22 @@
 #include "src/sys/stat/stat.h"
 #include "src/sys/time/utimes.h"
 #include "src/unistd/close.h"
+
+#include "test/UnitTest/ErrnoCheckingTest.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
+using LlvmLibcUtimesTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;
+
 // SUCCESS: Takes a file and successfully updates
 // its last access and modified times.
-TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
+TEST_F(LlvmLibcUtimesTest, ChangeTimesSpecific) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 
   constexpr const char *FILE_PATH = "utimes_pass.test";
   auto TEST_FILE = libc_make_test_file_path(FILE_PATH);
-  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
+  ASSERT_ERRNO_SUCCESS();
   ASSERT_GT(fd, 0);
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
 
@@ -36,11 +42,11 @@ TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
   times[1].tv_usec = 23456;
 
   // ensure utimes succeeds
-  ASSERT_THAT(LIBC_NAMESPACE::utimes(FILE_PATH, times), Succeeds(0));
+  ASSERT_THAT(LIBC_NAMESPACE::utimes(TEST_FILE, times), Succeeds(0));
 
   // verify the times values against stat of the TEST_FILE
   struct stat statbuf;
-  ASSERT_EQ(LIBC_NAMESPACE::stat(FILE_PATH, &statbuf), 0);
+  ASSERT_EQ(LIBC_NAMESPACE::stat(TEST_FILE, &statbuf), 0);
 
   // seconds
   ASSERT_EQ(statbuf.st_atim.tv_sec, times[0].tv_sec);
@@ -57,13 +63,13 @@ TEST(LlvmLibcUtimesTest, ChangeTimesSpecific) {
 
 // FAILURE: Invalid values in the timeval struct
 // to check that utimes rejects it.
-TEST(LlvmLibcUtimesTest, InvalidMicroseconds) {
+TEST_F(LlvmLibcUtimesTest, InvalidMicroseconds) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 
   constexpr const char *FILE_PATH = "utimes_fail.test";
   auto TEST_FILE = libc_make_test_file_path(FILE_PATH);
-  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
   ASSERT_GT(fd, 0);
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
 
@@ -76,7 +82,7 @@ TEST(LlvmLibcUtimesTest, InvalidMicroseconds) {
   times[1].tv_usec = 1000000; // invalid
 
   // ensure utimes fails
-  ASSERT_THAT(LIBC_NAMESPACE::utimes(FILE_PATH, times), Fails(EINVAL));
+  ASSERT_THAT(LIBC_NAMESPACE::utimes(TEST_FILE, times), Fails(EINVAL));
 
   // check for failure on
   // the other possible bad values
@@ -87,6 +93,6 @@ TEST(LlvmLibcUtimesTest, InvalidMicroseconds) {
   times[1].tv_usec = 1000;
 
   // ensure utimes fails once more
-  ASSERT_THAT(LIBC_NAMESPACE::utimes(FILE_PATH, times), Fails(EINVAL));
+  ASSERT_THAT(LIBC_NAMESPACE::utimes(TEST_FILE, times), Fails(EINVAL));
   ASSERT_THAT(LIBC_NAMESPACE::remove(TEST_FILE), Succeeds(0));
 }