[runtime] Fix potential overflow when using mono_msec_ticks
authorLudovic Henry <ludovic@xamarin.com>
Fri, 4 Mar 2016 11:37:38 +0000 (11:37 +0000)
committerLudovic Henry <ludovic@xamarin.com>
Tue, 3 May 2016 18:15:25 +0000 (14:15 -0400)
Because mono_msec_ticks would previously return the number of milliseconds since boot time, it would overflow after ~49 days. That would mean that anywhere you would use it, you would need to be careful of arithmetic overflow.

By simply returning a gint64 we will not overflow before ~30k years.

Fixes bug https://bugzilla.xamarin.com/show_bug.cgi?id=38666

mono/io-layer/processes.c
mono/metadata/gc.c
mono/metadata/icall-def.h
mono/metadata/icall.c
mono/metadata/monitor.c
mono/metadata/threadpool-ms.c
mono/metadata/threads.c
mono/mini/debugger-agent.c
mono/utils/mono-proclib.c
mono/utils/mono-time.c
mono/utils/mono-time.h

index 1136d17dcb2deb3aca5ab0c3d1daf0ca6244862d..212079346038545bd84f02e27809bacd4990ef9a 100644 (file)
@@ -2725,8 +2725,7 @@ process_wait (gpointer handle, guint32 timeout, gboolean alertable)
        WapiHandle_process *process_handle;
        pid_t pid G_GNUC_UNUSED, ret;
        int status;
-       guint32 start;
-       guint32 now;
+       gint64 start, now;
        struct MonoProcess *mp;
 
        /* FIXME: We can now easily wait on processes that aren't our own children,
index 9ab8f7dd6d674aa778d9a9f89a871ec77d1381bb..2cff646db1771c099780ffccbf618800d049a270 100644 (file)
@@ -876,14 +876,14 @@ mono_gc_cleanup (void)
                finished = TRUE;
                if (mono_thread_internal_current () != gc_thread) {
                        gboolean timed_out = FALSE;
-                       guint32 start_ticks = mono_msec_ticks ();
-                       guint32 end_ticks = start_ticks + 2000;
+                       gint64 start_ticks = mono_msec_ticks ();
+                       gint64 end_ticks = start_ticks + 2000;
 
                        mono_gc_finalize_notify ();
                        /* Finishing the finalizer thread, so wait a little bit... */
                        /* MS seems to wait for about 2 seconds */
                        while (!finalizer_thread_exited) {
-                               guint32 current_ticks = mono_msec_ticks ();
+                               gint64 current_ticks = mono_msec_ticks ();
                                guint32 timeout;
 
                                if (current_ticks >= end_ticks)
index 9c345a269cde4a0e5a36d7b234a9450695c4a004..b74631fd9420a19a216568a45eb1d0c00380a785 100644 (file)
@@ -239,7 +239,7 @@ ICALL(ENV_10, "get_HasShutdownStarted", ves_icall_System_Environment_get_HasShut
 ICALL(ENV_11, "get_MachineName", ves_icall_System_Environment_get_MachineName)
 ICALL(ENV_13, "get_Platform", ves_icall_System_Environment_get_Platform)
 ICALL(ENV_14, "get_ProcessorCount", mono_cpu_count)
-ICALL(ENV_15, "get_TickCount", mono_msec_ticks)
+ICALL(ENV_15, "get_TickCount", ves_icall_System_Environment_get_TickCount)
 ICALL(ENV_16, "get_UserName", ves_icall_System_Environment_get_UserName)
 ICALL(ENV_16m, "internalBroadcastSettingChange", ves_icall_System_Environment_BroadcastSettingChange)
 ICALL(ENV_17, "internalGetEnvironmentVariable", ves_icall_System_Environment_GetEnvironmentVariable)
index 285905fb9b8b8369f009526ea213975274c8ff3c..e22c730342dc4a3bc8eadcb4932aa9722ad5cc93 100644 (file)
@@ -7278,6 +7278,14 @@ ves_icall_System_Environment_BroadcastSettingChange (void)
 #endif
 }
 
+ICALL_EXPORT
+gint32
+ves_icall_System_Environment_get_TickCount (void)
+{
+       /* this will overflow after ~24 days */
+       return (gint32) (mono_msec_boottime () & 0xffffffff);
+}
+
 ICALL_EXPORT gint32
 ves_icall_System_Runtime_Versioning_VersioningHelper_GetRuntimeId (void)
 {
index 45c13be4e0f780ed511c327c5c9925ff97626e36..510d1c18bcd31b3fcd86556f7ee0cea21b4fe0ef 100644 (file)
@@ -743,7 +743,7 @@ mono_monitor_try_enter_inflated (MonoObject *obj, guint32 ms, gboolean allow_int
        LockWord lw;
        MonoThreadsSync *mon;
        HANDLE sem;
-       guint32 then = 0, now, delta;
+       gint64 then = 0, now, delta;
        guint32 waitms;
        guint32 ret;
        guint32 new_status, old_status, tmp_status;
@@ -900,14 +900,9 @@ retry_contended:
                if (!mono_thread_test_state (mono_thread_internal_current (), (MonoThreadState)(ThreadState_StopRequested | ThreadState_SuspendRequested | ThreadState_AbortRequested))) {
                        if (ms != INFINITE) {
                                now = mono_msec_ticks ();
-                               if (now < then) {
-                                       LOCK_DEBUG (g_message ("%s: wrapped around! now=0x%x then=0x%x", __func__, now, then));
 
-                                       now += (0xffffffff - then);
-                                       then = 0;
-
-                                       LOCK_DEBUG (g_message ("%s: wrap rejig: now=0x%x then=0x%x delta=0x%x", __func__, now, then, now-then));
-                               }
+                               /* it should not overflow before ~30k years */
+                               g_assert (now >= then);
 
                                delta = now - then;
                                if (delta >= ms) {
index 6151395688005d2ba455d7684bfec5f3bdb67d8c..69ade32e593ef5fc5db50665d5924f1069595dcb 100644 (file)
@@ -140,10 +140,10 @@ typedef struct {
        MonoCoopMutex worker_creation_lock;
 
        gint32 heuristic_completions;
-       guint32 heuristic_sample_start;
-       guint32 heuristic_last_dequeue; // ms
-       guint32 heuristic_last_adjustment; // ms
-       guint32 heuristic_adjustment_interval; // ms
+       gint64 heuristic_sample_start;
+       gint64 heuristic_last_dequeue; // ms
+       gint64 heuristic_last_adjustment; // ms
+       gint64 heuristic_adjustment_interval; // ms
        ThreadPoolHillClimbing heuristic_hill_climbing;
        MonoCoopMutex heuristic_lock;
 
@@ -855,7 +855,7 @@ monitor_should_keep_running (void)
 static gboolean
 monitor_sufficient_delay_since_last_dequeue (void)
 {
-       guint32 threshold;
+       gint64 threshold;
 
        g_assert (threadpool);
 
@@ -893,7 +893,7 @@ monitor_thread (void)
                mono_gc_set_skip_thread (TRUE);
 
                do {
-                       guint32 ts;
+                       gint64 ts;
                        gboolean alerted = FALSE;
 
                        if (mono_runtime_is_shutting_down ())
@@ -1052,7 +1052,7 @@ hill_climbing_get_wave_component (gdouble *samples, guint sample_count, gdouble
 }
 
 static gint16
-hill_climbing_update (gint16 current_thread_count, guint32 sample_duration, gint32 completions, guint32 *adjustment_interval)
+hill_climbing_update (gint16 current_thread_count, guint32 sample_duration, gint32 completions, gint64 *adjustment_interval)
 {
        ThreadPoolHillClimbing *hc;
        ThreadPoolHeuristicStateTransition transition;
@@ -1292,8 +1292,8 @@ heuristic_adjust (void)
 
        if (mono_coop_mutex_trylock (&threadpool->heuristic_lock) == 0) {
                gint32 completions = InterlockedExchange (&threadpool->heuristic_completions, 0);
-               guint32 sample_end = mono_msec_ticks ();
-               guint32 sample_duration = sample_end - threadpool->heuristic_sample_start;
+               gint64 sample_end = mono_msec_ticks ();
+               gint64 sample_duration = sample_end - threadpool->heuristic_sample_start;
 
                if (sample_duration >= threadpool->heuristic_adjustment_interval / 2) {
                        ThreadPoolCounter counter;
@@ -1418,7 +1418,7 @@ gboolean
 mono_threadpool_ms_remove_domain_jobs (MonoDomain *domain, int timeout)
 {
        gboolean res = TRUE;
-       guint32 start;
+       gint64 end;
        gpointer sem;
 
        g_assert (domain);
@@ -1427,13 +1427,12 @@ mono_threadpool_ms_remove_domain_jobs (MonoDomain *domain, int timeout)
        g_assert (mono_domain_is_unloading (domain));
 
        if (timeout != -1)
-               start = mono_msec_ticks ();
+               end = mono_msec_ticks () + timeout;
 
 #ifndef DISABLE_SOCKETS
        mono_threadpool_ms_io_remove_domain_jobs (domain);
        if (timeout != -1) {
-               timeout -= mono_msec_ticks () - start;
-               if (timeout < 0)
+               if (mono_msec_ticks () > end)
                        return FALSE;
        }
 #endif
@@ -1452,16 +1451,19 @@ mono_threadpool_ms_remove_domain_jobs (MonoDomain *domain, int timeout)
        mono_memory_write_barrier ();
 
        while (domain->threadpool_jobs) {
-               MONO_PREPARE_BLOCKING;
-               WaitForSingleObject (sem, timeout);
-               MONO_FINISH_BLOCKING;
+               gint64 now;
+
                if (timeout != -1) {
-                       timeout -= mono_msec_ticks () - start;
-                       if (timeout <= 0) {
+                       now = mono_msec_ticks ();
+                       if (now > end) {
                                res = FALSE;
                                break;
                        }
                }
+
+               MONO_PREPARE_BLOCKING;
+               WaitForSingleObject (sem, timeout != -1 ? end - now : timeout);
+               MONO_FINISH_BLOCKING;
        }
 
        domain->cleanup_semaphore = NULL;
index 2f7fcabf081a4d25662147a77241d59472c17981..6dbba62b347d948ad99129603748dc6e967ee124 100644 (file)
@@ -3790,7 +3790,7 @@ mono_threads_abort_appdomain_threads (MonoDomain *domain, int timeout)
 #endif
 
        abort_appdomain_data user_data;
-       guint32 start_time;
+       gint64 start_time;
        int orig_timeout = timeout;
        int i;
 
index 5b05e57c90e39af906a32a24d9f876a0e900d337..8c4d16aa4f27b4a3fb88d33e64e4e858d048365e 100644 (file)
@@ -1120,8 +1120,8 @@ socket_transport_recv (void *buf, int len)
        int total = 0;
        int fd = conn_fd;
        int flags = 0;
-       static gint32 last_keepalive;
-       gint32 msecs;
+       static gint64 last_keepalive;
+       gint64 msecs;
 
        MONO_PREPARE_BLOCKING;
 
index 7a4a02e39a3f7fea5ca4a441251215d75d406bf8..efd732d6d56b7592b9de709a4d34c352e42baf8f 100644 (file)
@@ -325,7 +325,7 @@ mono_process_get_times (gpointer pid, gint64 *start_time, gint64 *user_time, gin
                if (*start_time == 0) {
                        static guint64 boot_time = 0;
                        if (!boot_time)
-                               boot_time = mono_100ns_datetime () - ((guint64)mono_msec_ticks ()) * 10000;
+                               boot_time = mono_100ns_datetime () - mono_msec_boottime () * 10000;
 
                        *start_time = boot_time + mono_process_get_data (pid, MONO_PROCESS_ELAPSED);
                }
index 5f0168ebb7475b94886d44a002331d2aa1184842..5cc82fccb55e082f6ecd6b198f1321eed06eda95 100644 (file)
 #include <utils/mono-time.h>
 
 
-#define MTICKS_PER_SEC 10000000
+#define MTICKS_PER_SEC (10 * 1000 * 1000)
+
+gint64
+mono_msec_ticks (void)
+{
+       return mono_100ns_ticks () / 10 / 1000;
+}
 
 #ifdef HOST_WIN32
 #include <windows.h>
 
-guint32
-mono_msec_ticks (void)
+#ifndef _MSC_VER
+/* we get "error: implicit declaration of function 'GetTickCount64'" */
+WINBASEAPI ULONGLONG WINAPI GetTickCount64(void);
+#endif
+
+gint64
+mono_msec_boottime (void)
 {
        /* GetTickCount () is reportedly monotonic */
-       return GetTickCount ();
+       return GetTickCount64 ();
 }
 
 /* Returns the number of 100ns ticks from unspecified time: this should be monotonic */
@@ -102,7 +113,7 @@ get_boot_time (void)
        if (uptime) {
                double upt;
                if (fscanf (uptime, "%lf", &upt) == 1) {
-                       gint64 now = mono_100ns_ticks ();
+                       gint64 now = mono_100ns_datetime ();
                        fclose (uptime);
                        return now - (gint64)(upt * MTICKS_PER_SEC);
                }
@@ -114,14 +125,14 @@ get_boot_time (void)
 }
 
 /* Returns the number of milliseconds from boot time: this should be monotonic */
-guint32
-mono_msec_ticks (void)
+gint64
+mono_msec_boottime (void)
 {
        static gint64 boot_time = 0;
        gint64 now;
        if (!boot_time)
                boot_time = get_boot_time ();
-       now = mono_100ns_ticks ();
+       now = mono_100ns_datetime ();
        /*printf ("now: %llu (boot: %llu) ticks: %llu\n", (gint64)now, (gint64)boot_time, (gint64)(now - boot_time));*/
        return (now - boot_time)/10000;
 }
index 95bda8e530979b740b5b7fa06a99b300031e7ede..438b9ef18815b1bdc0ee574c715638654f25f44f 100644 (file)
@@ -8,14 +8,19 @@
 #include <sys/time.h>
 #endif
 
-/* Returns the number of milliseconds from boot time: this should be monotonic */
-guint32 mono_msec_ticks      (void);
+/* Returns the number of milliseconds from boot time: this should be monotonic
+ *
+ * Prefer to use mono_msec_ticks for elapsed time calculation. */
+gint64 mono_msec_boottime (void);
+
+/* Returns the number of milliseconds ticks from unspecified time: this should be monotonic */
+gint64 mono_msec_ticks (void);
 
 /* Returns the number of 100ns ticks from unspecified time: this should be monotonic */
-gint64  mono_100ns_ticks     (void);
+gint64 mono_100ns_ticks (void);
 
 /* Returns the number of 100ns ticks since 1/1/1601, UTC timezone */
-gint64  mono_100ns_datetime  (void);
+gint64 mono_100ns_datetime (void);
 
 #ifndef HOST_WIN32
 gint64 mono_100ns_datetime_from_timeval (struct timeval tv);