Merge pull request #4069 from shamb0/string_deduplication_for_localtime_r

localtime_r: deduplicate timezone name allocation

Author: RalfJung

src/tools/miri/src/shims/time.rs

@@ -5,6 +5,8 @@ use std::time::{Duration, SystemTime};
 
 use chrono::{DateTime, Datelike, Offset, Timelike, Utc};
 use chrono_tz::Tz;
+use rustc_abi::Align;
+use rustc_ast::ast::Mutability;
 
 use crate::*;
 
@@ -180,6 +182,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         if !matches!(&*this.tcx.sess.target.os, "solaris" | "illumos") {
             // tm_zone represents the timezone value in the form of: +0730, +08, -0730 or -08.
             // This may not be consistent with libc::localtime_r's result.
+
             let offset_in_seconds = dt.offset().fix().local_minus_utc();
             let tm_gmtoff = offset_in_seconds;
             let mut tm_zone = String::new();
@@ -195,11 +198,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 write!(tm_zone, "{:02}", offset_min).unwrap();
             }
 
-            // FIXME: String de-duplication is needed so that we only allocate this string only once
-            // even when there are multiple calls to this function.
-            let tm_zone_ptr = this
-                .alloc_os_str_as_c_str(&OsString::from(tm_zone), MiriMemoryKind::Machine.into())?;
+            // Add null terminator for C string compatibility.
+            tm_zone.push('\0');
+
+            // Deduplicate and allocate the string.
+            let tm_zone_ptr = this.allocate_bytes(
+                tm_zone.as_bytes(),
+                Align::ONE,
+                MiriMemoryKind::Machine.into(),
+                Mutability::Not,
+            )?;
 
+            // Write the timezone pointer and offset into the result structure.
             this.write_pointer(tm_zone_ptr, &this.project_field_named(&result, "tm_zone")?)?;
             this.write_int_fields_named(&[("tm_gmtoff", tm_gmtoff.into())], &result)?;
         }

src/tools/miri/tests/pass-dep/libc/libc-time.rs

@@ -5,7 +5,21 @@ use std::{env, mem, ptr};
 fn main() {
     test_clocks();
     test_posix_gettimeofday();
-    test_localtime_r();
+    test_localtime_r_gmt();
+    test_localtime_r_pst();
+    test_localtime_r_epoch();
+    #[cfg(any(
+        target_os = "linux",
+        target_os = "macos",
+        target_os = "freebsd",
+        target_os = "android"
+    ))]
+    test_localtime_r_multiple_calls_deduplication();
+    // Architecture-specific tests.
+    #[cfg(target_pointer_width = "32")]
+    test_localtime_r_future_32b();
+    #[cfg(target_pointer_width = "64")]
+    test_localtime_r_future_64b();
 }
 
 /// Tests whether clock support exists at all
@@ -46,14 +60,9 @@ fn test_posix_gettimeofday() {
     assert_eq!(is_error, -1);
 }
 
-fn test_localtime_r() {
-    // Set timezone to GMT.
-    let key = "TZ";
-    env::set_var(key, "GMT");
-
-    const TIME_SINCE_EPOCH: libc::time_t = 1712475836;
-    let custom_time_ptr = &TIME_SINCE_EPOCH;
-    let mut tm = libc::tm {
+/// Helper function to create an empty tm struct.
+fn create_empty_tm() -> libc::tm {
+    libc::tm {
         tm_sec: 0,
         tm_min: 0,
         tm_hour: 0,
@@ -77,7 +86,17 @@ fn test_localtime_r() {
             target_os = "android"
         ))]
         tm_zone: std::ptr::null_mut::<libc::c_char>(),
-    };
+    }
+}
+
+/// Original GMT test
+fn test_localtime_r_gmt() {
+    // Set timezone to GMT.
+    let key = "TZ";
+    env::set_var(key, "GMT");
+    const TIME_SINCE_EPOCH: libc::time_t = 1712475836; // 2024-04-07 07:43:56 GMT
+    let custom_time_ptr = &TIME_SINCE_EPOCH;
+    let mut tm = create_empty_tm();
     let res = unsafe { libc::localtime_r(custom_time_ptr, &mut tm) };
 
     assert_eq!(tm.tm_sec, 56);
@@ -95,20 +114,204 @@ fn test_localtime_r() {
         target_os = "freebsd",
         target_os = "android"
     ))]
-    assert_eq!(tm.tm_gmtoff, 0);
+    {
+        assert_eq!(tm.tm_gmtoff, 0);
+        unsafe {
+            assert_eq!(std::ffi::CStr::from_ptr(tm.tm_zone).to_str().unwrap(), "+00");
+        }
+    }
+
+    // The returned value is the pointer passed in.
+    assert!(ptr::eq(res, &mut tm));
+
+    // Remove timezone setting.
+    env::remove_var(key);
+}
+
+/// PST timezone test (testing different timezone handling).
+fn test_localtime_r_pst() {
+    let key = "TZ";
+    env::set_var(key, "PST8PDT");
+    const TIME_SINCE_EPOCH: libc::time_t = 1712475836; // 2024-04-07 07:43:56 GMT
+    let custom_time_ptr = &TIME_SINCE_EPOCH;
+    let mut tm = create_empty_tm();
+
+    let res = unsafe { libc::localtime_r(custom_time_ptr, &mut tm) };
+
+    assert_eq!(tm.tm_sec, 56);
+    assert_eq!(tm.tm_min, 43);
+    assert_eq!(tm.tm_hour, 0); // 7 - 7 = 0 (PDT offset)
+    assert_eq!(tm.tm_mday, 7);
+    assert_eq!(tm.tm_mon, 3);
+    assert_eq!(tm.tm_year, 124);
+    assert_eq!(tm.tm_wday, 0);
+    assert_eq!(tm.tm_yday, 97);
+    assert_eq!(tm.tm_isdst, -1); // DST information unavailable
+
     #[cfg(any(
         target_os = "linux",
         target_os = "macos",
         target_os = "freebsd",
         target_os = "android"
     ))]
-    unsafe {
-        assert_eq!(std::ffi::CStr::from_ptr(tm.tm_zone).to_str().unwrap(), "+00")
-    };
+    {
+        assert_eq!(tm.tm_gmtoff, -7 * 3600); // -7 hours in seconds
+        unsafe {
+            assert_eq!(std::ffi::CStr::from_ptr(tm.tm_zone).to_str().unwrap(), "-07");
+        }
+    }
 
-    // The returned value is the pointer passed in.
     assert!(ptr::eq(res, &mut tm));
+    env::remove_var(key);
+}
 
-    // Remove timezone setting.
+/// Unix epoch test (edge case testing).
+fn test_localtime_r_epoch() {
+    let key = "TZ";
+    env::set_var(key, "GMT");
+    const TIME_SINCE_EPOCH: libc::time_t = 0; // 1970-01-01 00:00:00
+    let custom_time_ptr = &TIME_SINCE_EPOCH;
+    let mut tm = create_empty_tm();
+
+    let res = unsafe { libc::localtime_r(custom_time_ptr, &mut tm) };
+
+    assert_eq!(tm.tm_sec, 0);
+    assert_eq!(tm.tm_min, 0);
+    assert_eq!(tm.tm_hour, 0);
+    assert_eq!(tm.tm_mday, 1);
+    assert_eq!(tm.tm_mon, 0);
+    assert_eq!(tm.tm_year, 70);
+    assert_eq!(tm.tm_wday, 4); // Thursday
+    assert_eq!(tm.tm_yday, 0);
+    assert_eq!(tm.tm_isdst, -1);
+
+    #[cfg(any(
+        target_os = "linux",
+        target_os = "macos",
+        target_os = "freebsd",
+        target_os = "android"
+    ))]
+    {
+        assert_eq!(tm.tm_gmtoff, 0);
+        unsafe {
+            assert_eq!(std::ffi::CStr::from_ptr(tm.tm_zone).to_str().unwrap(), "+00");
+        }
+    }
+
+    assert!(ptr::eq(res, &mut tm));
+    env::remove_var(key);
+}
+
+/// Future date test (testing large values).
+#[cfg(target_pointer_width = "64")]
+fn test_localtime_r_future_64b() {
+    let key = "TZ";
+    env::set_var(key, "GMT");
+
+    // Using 2050-01-01 00:00:00 for 64-bit systems
+    // value that's safe for 64-bit time_t
+    const TIME_SINCE_EPOCH: libc::time_t = 2524608000;
+    let custom_time_ptr = &TIME_SINCE_EPOCH;
+    let mut tm = create_empty_tm();
+
+    let res = unsafe { libc::localtime_r(custom_time_ptr, &mut tm) };
+
+    assert_eq!(tm.tm_sec, 0);
+    assert_eq!(tm.tm_min, 0);
+    assert_eq!(tm.tm_hour, 0);
+    assert_eq!(tm.tm_mday, 1);
+    assert_eq!(tm.tm_mon, 0);
+    assert_eq!(tm.tm_year, 150); // 2050 - 1900
+    assert_eq!(tm.tm_wday, 6); // Saturday
+    assert_eq!(tm.tm_yday, 0);
+    assert_eq!(tm.tm_isdst, -1);
+
+    #[cfg(any(
+        target_os = "linux",
+        target_os = "macos",
+        target_os = "freebsd",
+        target_os = "android"
+    ))]
+    {
+        assert_eq!(tm.tm_gmtoff, 0);
+        unsafe {
+            assert_eq!(std::ffi::CStr::from_ptr(tm.tm_zone).to_str().unwrap(), "+00");
+        }
+    }
+
+    assert!(ptr::eq(res, &mut tm));
+    env::remove_var(key);
+}
+
+/// Future date test (testing large values for 32b target).
+#[cfg(target_pointer_width = "32")]
+fn test_localtime_r_future_32b() {
+    let key = "TZ";
+    env::set_var(key, "GMT");
+
+    // Using 2030-01-01 00:00:00 for 32-bit systems
+    // Safe value within i32 range
+    const TIME_SINCE_EPOCH: libc::time_t = 1893456000;
+    let custom_time_ptr = &TIME_SINCE_EPOCH;
+    let mut tm = create_empty_tm();
+
+    let res = unsafe { libc::localtime_r(custom_time_ptr, &mut tm) };
+
+    // Verify 2030-01-01 00:00:00
+    assert_eq!(tm.tm_sec, 0);
+    assert_eq!(tm.tm_min, 0);
+    assert_eq!(tm.tm_hour, 0);
+    assert_eq!(tm.tm_mday, 1);
+    assert_eq!(tm.tm_mon, 0);
+    assert_eq!(tm.tm_year, 130); // 2030 - 1900
+    assert_eq!(tm.tm_wday, 2); // Tuesday
+    assert_eq!(tm.tm_yday, 0);
+    assert_eq!(tm.tm_isdst, -1);
+
+    #[cfg(any(
+        target_os = "linux",
+        target_os = "macos",
+        target_os = "freebsd",
+        target_os = "android"
+    ))]
+    {
+        assert_eq!(tm.tm_gmtoff, 0);
+        unsafe {
+            assert_eq!(std::ffi::CStr::from_ptr(tm.tm_zone).to_str().unwrap(), "+00");
+        }
+    }
+
+    assert!(ptr::eq(res, &mut tm));
     env::remove_var(key);
 }
+
+/// Tests the behavior of `localtime_r` with multiple calls to ensure deduplication of `tm_zone` pointers.
+#[cfg(any(target_os = "linux", target_os = "macos", target_os = "freebsd", target_os = "android"))]
+fn test_localtime_r_multiple_calls_deduplication() {
+    let key = "TZ";
+    env::set_var(key, "PST8PDT");
+
+    const TIME_SINCE_EPOCH_BASE: libc::time_t = 1712475836; // Base timestamp: 2024-04-07 07:43:56 GMT
+    const NUM_CALLS: usize = 50;
+
+    let mut unique_pointers = std::collections::HashSet::new();
+
+    for i in 0..NUM_CALLS {
+        let timestamp = TIME_SINCE_EPOCH_BASE + (i as libc::time_t * 3600); // Increment by 1 hour for each call
+        let mut tm: libc::tm = create_empty_tm();
+        let tm_ptr = unsafe { libc::localtime_r(&timestamp, &mut tm) };
+
+        assert!(!tm_ptr.is_null(), "localtime_r failed for timestamp {timestamp}");
+
+        unique_pointers.insert(tm.tm_zone);
+    }
+
+    let unique_count = unique_pointers.len();
+
+    assert!(
+        unique_count >= 2 && unique_count <= (NUM_CALLS - 1),
+        "Unexpected number of unique tm_zone pointers: {} (expected between 2 and {})",
+        unique_count,
+        NUM_CALLS - 1
+    );
+}