[libc] Fix matching on struct tm

[ddcc]

Summary

Ensure that struct tm pointer is not nullptr, and verify that all (not any) fields match

[ddcc]

Fix incorrect testcase for gmtime

[ddcc]

Fix incorrect testcase for gmtime_r

[ddcc]

More gmtime test fixes

[ddcc]

More gmtime_r test fixes

[ddcc]

Last wrong testcase for gmtime

[jeffbailey]

Ate these test changes enough to catch this? It seems a bit magical with the numbers. Should there be an explicit test that tests matching more than one field?

[rtenneti]

Thanks for the above fix (facepalm). In the following CL, jeff and I reworked the test code to make it more readable.

In https://reviews.llvm.org/D130405, https://reviews.llvm.org/differential/changeset/?ref=3574155

[ddcc]

In D137371#3912177, @jeffbailey wrote:

Ate these test changes enough to catch this? It seems a bit magical with the numbers. Should there be an explicit test that tests matching more than one field?

I'm not sure I understand the question. There were three issues with the tests that this commit fixes:

1. The tests don't check that the pointer tm_data is not NULL before dereferencing it
2. The test matcher only checked that at least one field of struct tm matches, and not that all fields match.
3. Multiple testcases contained incorrect values for the tm_yday field.

I've been running the llvm-libc tests against a different libc, which is what led me to identify these testcase issues. After these above fixes, the tests against that libc no longer fail. I haven't manually tested these changes against the llvm-libc implementation itself, but from looking at the CI build output they seem fine.

As for the formatting changes, that looks good to me.

libc/test/src/time/TmMatcher.h

class StructTmMatcher : public __llvm_libc::testing::Matcher<::tm> {
   ::tm expected;
   ::tm actual;
 
 public:
   StructTmMatcher(::tm expectedValue) : expected(expectedValue) {}
 
   bool match(::tm actualValue) {
     actual = actualValue;
-    return (actual.tm_sec == expected.tm_sec ||
-            actual.tm_min == expected.tm_min ||
-            actual.tm_hour == expected.tm_hour ||
-            actual.tm_mday == expected.tm_mday ||
-            actual.tm_mon == expected.tm_mon ||
-            actual.tm_year == expected.tm_year ||
-            actual.tm_wday == expected.tm_wday ||
-            actual.tm_yday == expected.tm_yday ||
+    return (actual.tm_sec == expected.tm_sec &&
+            actual.tm_min == expected.tm_min &&
+            actual.tm_hour == expected.tm_hour &&
+            actual.tm_mday == expected.tm_mday &&
+            actual.tm_mon == expected.tm_mon &&
+            actual.tm_year == expected.tm_year &&
+            actual.tm_wday == expected.tm_wday &&
+            actual.tm_yday == expected.tm_yday &&
             actual.tm_isdst == expected.tm_isdst);
   }
 
   void describeValue(const char *label, ::tm value,
                      __llvm_libc::testutils::StreamWrapper &stream) {
     stream << label;
     stream << " sec: " << value.tm_sec;
     stream << " min: " << value.tm_min;

libc/test/src/time/gmtime_r_test.cpp

 // from gmtime. Added couple of validation tests.
 TEST(LlvmLibcGmTimeR, EndOf32BitEpochYear) {
   // Test for maximum value of a signed 32-bit integer.
   // Test implementation can encode time for Tue 19 January 2038 03:14:07 UTC.
   time_t seconds = 0x7FFFFFFF;
   struct tm tm_data;
   struct tm *tm_data_ptr;
   tm_data_ptr = __llvm_libc::gmtime_r(&seconds, &tm_data);
+  ASSERT_NE(tm_data_ptr, nullptr);
   EXPECT_TM_EQ((tm{7,  // sec
                    14, // min
                    3,  // hr
                    19, // day
                    0,  // tm_mon starts with 0 for Jan
                    2038 - TimeConstants::TIME_YEAR_BASE, // year
                    2,                                    // wday
-                   7,                                    // yday
+                   18,                                   // yday
                    0}),
                *tm_data_ptr);
   EXPECT_TM_EQ(*tm_data_ptr, tm_data);
 }
 
 TEST(LlvmLibcGmTimeR, Max64BitYear) {
   if (sizeof(time_t) == 4)
     return;
   // Test for Tue Jan 1 12:50:50 in 2,147,483,647th year.
   time_t seconds = 67767976202043050;
   struct tm tm_data;
   struct tm *tm_data_ptr;
   tm_data_ptr = __llvm_libc::gmtime_r(&seconds, &tm_data);
+  ASSERT_NE(tm_data_ptr, nullptr);
   EXPECT_TM_EQ((tm{50, // sec
                    50, // min
                    12, // hr
                    1,  // day
                    0,  // tm_mon starts with 0 for Jan
                    2147483647 - TimeConstants::TIME_YEAR_BASE, // year
                    2,                                          // wday
-                   50,                                         // yday
+                   0,                                          // yday
                    0}),
                *tm_data_ptr);
   EXPECT_TM_EQ(*tm_data_ptr, tm_data);
 }

libc/test/src/time/gmtime_test.cpp

 }
 
 TEST(LlvmLibcGmTime, InvalidSeconds) {
   time_t seconds = 0;
   struct tm *tm_data = nullptr;
   // -1 second from 1970-01-01 00:00:00 returns 1969-12-31 23:59:59.
   seconds = -1;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{59,     // sec
                    59,     // min
                    23,     // hr
                    31,     // day
                    12 - 1, // tm_mon starts with 0 for Jan
                    1969 - TimeConstants::TIME_YEAR_BASE, // year
                    3,                                    // wday
                    364,                                  // yday
                    0}),
                *tm_data);
   // 60 seconds from 1970-01-01 00:00:00 returns 1970-01-01 00:01:00.
   seconds = 60;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    1, // min
                    0, // hr
                    1, // day
                    0, // tm_mon starts with 0 for Jan
                    1970 - TimeConstants::TIME_YEAR_BASE, // year
                    4,                                    // wday
                    0,                                    // yday
                    0}),
                *tm_data);
 }
 
 TEST(LlvmLibcGmTime, InvalidMinutes) {
   time_t seconds = 0;
   struct tm *tm_data = nullptr;
   // -1 minute from 1970-01-01 00:00:00 returns 1969-12-31 23:59:00.
   seconds = -TimeConstants::SECONDS_PER_MIN;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0,  // sec
                    59, // min
                    23, // hr
                    31, // day
                    11, // tm_mon starts with 0 for Jan
                    1969 - TimeConstants::TIME_YEAR_BASE, // year
                    3,                                    // wday
-                   0,                                    // yday
+                   364,                                  // yday
                    0}),
                *tm_data);
   // 60 minutes from 1970-01-01 00:00:00 returns 1970-01-01 01:00:00.
   seconds = 60 * TimeConstants::SECONDS_PER_MIN;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    0, // min
                    1, // hr
                    1, // day
                    0, // tm_mon starts with 0 for Jan
                    1970 - TimeConstants::TIME_YEAR_BASE, // year
                    4,                                    // wday
                    0,                                    // yday
                    0}),
                *tm_data);
 }
 
 TEST(LlvmLibcGmTime, InvalidHours) {
   time_t seconds = 0;
   struct tm *tm_data = nullptr;
   // -1 hour from 1970-01-01 00:00:00 returns 1969-12-31 23:00:00.
   seconds = -TimeConstants::SECONDS_PER_HOUR;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0,  // sec
                    0,  // min
                    23, // hr
                    31, // day
                    11, // tm_mon starts with 0 for Jan
                    1969 - TimeConstants::TIME_YEAR_BASE, // year
                    3,                                    // wday
-                   0,                                    // yday
+                   364,                                  // yday
                    0}),
                *tm_data);
   // 24 hours from 1970-01-01 00:00:00 returns 1970-01-02 00:00:00.
   seconds = 24 * TimeConstants::SECONDS_PER_HOUR;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    0, // min
                    0, // hr
                    2, // day
                    0, // tm_mon starts with 0 for Jan
                    1970 - TimeConstants::TIME_YEAR_BASE, // year
                    5,                                    // wday
-                   0,                                    // yday
+                   1,                                    // yday
                    0}),
                *tm_data);
 }
 
 TEST(LlvmLibcGmTime, InvalidYear) {
   // -1 year from 1970-01-01 00:00:00 returns 1969-01-01 00:00:00.
   time_t seconds =
       -TimeConstants::DAYS_PER_NON_LEAP_YEAR * TimeConstants::SECONDS_PER_DAY;
   struct tm *tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    0, // min
                    0, // hr
                    1, // day
                    0, // tm_mon starts with 0 for Jan
                    1969 - TimeConstants::TIME_YEAR_BASE, // year
                    3,                                    // wday
                    0,                                    // yday
                    0}),
                *tm_data);
 }
 
 TEST(LlvmLibcGmTime, InvalidMonths) {
   time_t seconds = 0;
   struct tm *tm_data = nullptr;
   // -1 month from 1970-01-01 00:00:00 returns 1969-12-01 00:00:00.
   seconds = -31 * TimeConstants::SECONDS_PER_DAY;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0,      // sec
                    0,      // min
                    0,      // hr
                    1,      // day
                    12 - 1, // tm_mon starts with 0 for Jan
                    1969 - TimeConstants::TIME_YEAR_BASE, // year
                    1,                                    // wday
-                   0,                                    // yday
+                   334,                                  // yday
                    0}),
                *tm_data);
   // 1970-13-01 00:00:00 returns 1971-01-01 00:00:00.
   seconds =
       TimeConstants::DAYS_PER_NON_LEAP_YEAR * TimeConstants::SECONDS_PER_DAY;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    0, // min
                    0, // hr
                    1, // day
                    0, // tm_mon starts with 0 for Jan
                    1971 - TimeConstants::TIME_YEAR_BASE, // year
                    5,                                    // wday
                    0,                                    // yday
                    0}),
                *tm_data);
 }
 
 TEST(LlvmLibcGmTime, InvalidDays) {
   time_t seconds = 0;
   struct tm *tm_data = nullptr;
   // -1 day from 1970-01-01 00:00:00 returns 1969-12-31 00:00:00.
   seconds = -1 * TimeConstants::SECONDS_PER_DAY;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0,  // sec
                    0,  // min
                    0,  // hr
                    31, // day
                    11, // tm_mon starts with 0 for Jan
                    1969 - TimeConstants::TIME_YEAR_BASE, // year
                    3,                                    // wday
-                   0,                                    // yday
+                   364,                                  // yday
                    0}),
                *tm_data);
 
   // 1970-01-32 00:00:00 returns 1970-02-01 00:00:00.
   seconds = 31 * TimeConstants::SECONDS_PER_DAY;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    0, // min
                    0, // hr
                    1, // day
-                   0, // tm_mon starts with 0 for Jan
+                   1, // tm_mon starts with 0 for Jan
                    1970 - TimeConstants::TIME_YEAR_BASE, // year
                    0,                                    // wday
-                   0,                                    // yday
+                   31,                                   // yday
                    0}),
                *tm_data);
 
   // 1970-02-29 00:00:00 returns 1970-03-01 00:00:00.
   seconds = 59 * TimeConstants::SECONDS_PER_DAY;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    0, // min
                    0, // hr
                    1, // day
                    2, // tm_mon starts with 0 for Jan
                    1970 - TimeConstants::TIME_YEAR_BASE, // year
                    0,                                    // wday
-                   0,                                    // yday
+                   59,                                   // yday
                    0}),
                *tm_data);
 
   // 1972-02-30 00:00:00 returns 1972-03-01 00:00:00.
   seconds = ((2 * TimeConstants::DAYS_PER_NON_LEAP_YEAR) + 60) *
             TimeConstants::SECONDS_PER_DAY;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{0, // sec
                    0, // min
                    0, // hr
                    1, // day
                    2, // tm_mon starts with 0 for Jan
                    1972 - TimeConstants::TIME_YEAR_BASE, // year
                    3,                                    // wday
-                   0,                                    // yday
+                   60,                                   // yday
                    0}),
                *tm_data);
 }
 
 TEST(LlvmLibcGmTime, EndOf32BitEpochYear) {
   // Test for maximum value of a signed 32-bit integer.
   // Test implementation can encode time for Tue 19 January 2038 03:14:07 UTC.
   time_t seconds = 0x7FFFFFFF;
   struct tm *tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{7,  // sec
                    14, // min
                    3,  // hr
                    19, // day
                    0,  // tm_mon starts with 0 for Jan
                    2038 - TimeConstants::TIME_YEAR_BASE, // year
                    2,                                    // wday
-                   7,                                    // yday
+                   18,                                   // yday
                    0}),
                *tm_data);
 }
 
 TEST(LlvmLibcGmTime, Max64BitYear) {
   if (sizeof(time_t) == 4)
     return;
   // Mon Jan 1 12:50:50 2170 (200 years from 1970),
   time_t seconds = 6311479850;
   struct tm *tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{50, // sec
                    50, // min
                    12, // hr
                    1,  // day
                    0,  // tm_mon starts with 0 for Jan
                    2170 - TimeConstants::TIME_YEAR_BASE, // year
                    1,                                    // wday
-                   50,                                   // yday
+                   0,                                    // yday
                    0}),
                *tm_data);
 
   // Test for Tue Jan 1 12:50:50 in 2,147,483,647th year.
   seconds = 67767976202043050;
   tm_data = __llvm_libc::gmtime(&seconds);
+  ASSERT_NE(tm_data, nullptr);
   EXPECT_TM_EQ((tm{50, // sec
                    50, // min
                    12, // hr
                    1,  // day
                    0,  // tm_mon starts with 0 for Jan
                    2147483647 - TimeConstants::TIME_YEAR_BASE, // year
                    2,                                          // wday
-                   50,                                         // yday
+                   0,                                          // yday
                    0}),
                *tm_data);
 }