Skip to content

Commit

Permalink
[runtime] Fix potential overflow when using mono_msec_ticks
Browse files Browse the repository at this point in the history
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
  • Loading branch information
luhenry committed Mar 17, 2016
1 parent 12ff2e4 commit 6277ba5
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 46 deletions.
3 changes: 1 addition & 2 deletions mono/io-layer/processes.c
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions mono/metadata/gc.c
Expand Up @@ -827,14 +827,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)
Expand Down
2 changes: 1 addition & 1 deletion mono/metadata/icall-def.h
Expand Up @@ -242,7 +242,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)
Expand Down
8 changes: 8 additions & 0 deletions mono/metadata/icall.c
Expand Up @@ -6967,6 +6967,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)
{
Expand Down
11 changes: 3 additions & 8 deletions mono/metadata/monitor.c
Expand Up @@ -742,7 +742,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;
Expand Down Expand Up @@ -899,14 +899,9 @@ mono_monitor_try_enter_inflated (MonoObject *obj, guint32 ms, gboolean allow_int
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) {
Expand Down
38 changes: 20 additions & 18 deletions mono/metadata/threadpool-ms.c
Expand Up @@ -139,10 +139,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;

Expand Down Expand Up @@ -847,7 +847,7 @@ monitor_should_keep_running (void)
static gboolean
monitor_sufficient_delay_since_last_dequeue (void)
{
guint32 threshold;
gint64 threshold;

g_assert (threadpool);

Expand Down Expand Up @@ -885,7 +885,7 @@ monitor_thread (void)
mono_gc_set_skip_thread (TRUE);

do {
guint32 ts;
gint64 ts;
gboolean alerted = FALSE;

if (mono_runtime_is_shutting_down ())
Expand Down Expand Up @@ -1041,7 +1041,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;
Expand Down Expand Up @@ -1281,8 +1281,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;
Expand Down Expand Up @@ -1402,7 +1402,7 @@ gboolean
mono_threadpool_ms_remove_domain_jobs (MonoDomain *domain, int timeout)
{
gboolean res = TRUE;
guint32 start;
gint64 end;
gpointer sem;

g_assert (domain);
Expand All @@ -1411,13 +1411,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
Expand All @@ -1436,16 +1435,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;
Expand Down
2 changes: 1 addition & 1 deletion mono/metadata/threads.c
Expand Up @@ -3749,7 +3749,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;

Expand Down
4 changes: 2 additions & 2 deletions mono/mini/debugger-agent.c
Expand Up @@ -1119,8 +1119,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;

Expand Down
2 changes: 1 addition & 1 deletion mono/utils/mono-proclib.c
Expand Up @@ -323,7 +323,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);
}
Expand Down
24 changes: 18 additions & 6 deletions mono/utils/mono-time.c
Expand Up @@ -15,16 +15,27 @@
#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'" */
ULONGLONG 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 */
Expand Down Expand Up @@ -113,15 +124,16 @@ 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 ();
/*printf ("now: %llu (boot: %llu) ticks: %llu\n", (gint64)now, (gint64)boot_time, (gint64)(now - boot_time));*/
g_assert (now > boot_time);
return (now - boot_time)/10000;
}

Expand Down
13 changes: 9 additions & 4 deletions mono/utils/mono-time.h
Expand Up @@ -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);
Expand Down

0 comments on commit 6277ba5

Please sign in to comment.