3snowp7im / urn

Split tracker / timer with GTK+ frontend
GNU General Public License v3.0
126 stars 36 forks source link

Long overflow on 32-bit systems #35

Closed ghost closed 3 years ago

ghost commented 6 years ago

Issue

Hi,

I am using urn on 32-bit Linux and have noticed some randomly occurring problems with the timer suddenly jumping to a very large negative value.

I believe the culprit is in urn_time_now() in urn.c:

long long urn_time_now(void) {
    struct timespec timespec;
    clock_gettime(CLOCK_MONOTONIC, &timespec);
    return timespec.tv_sec * 1000000L + timespec.tv_nsec / 1000;
}

The issue is that generally on a 32-bit platform sizeof(long) == 32, so the use of 1000000L can cause an overflow. This can be fixed by always using 1000000LL when returning a long long. This overflow doesn't occur on 64-bit systems because generally sizeof(long) == sizeof(long long) on 64-bit.

How to reproduce

Below is a concrete example of an overflow. On Linux the time returned from clock_gettime(CLOCK_MONOTONIC,...) is based on system uptime. The following code hardcodes specific time values that cause an overflow. It calls urn_time_now() twice, 1 second apart, then subtracts the times.

/*
 * test.c: Compile with: cc test.c -o test
 */

#include <stdio.h>
#include <time.h>

long long urn_time_now_LONG(int sec) {
    struct timespec timespec;
    timespec.tv_sec = sec;
    timespec.tv_nsec = 299000000;
    return timespec.tv_sec * 1000000L + timespec.tv_nsec / 1000;
}
long long urn_time_now_LONG_LONG(int sec) {
    struct timespec timespec;
    timespec.tv_sec = sec;
    timespec.tv_nsec = 299000000;
    return timespec.tv_sec * 1000000LL + timespec.tv_nsec / 1000;
}

int main(void)
{
   long long time1,time2,diff;

   time1=urn_time_now_LONG(36506);
   time2=urn_time_now_LONG(36507);
   diff = time2-time1;
   printf("Difference: %lld seconds (1000000L)\n", diff / 1000000);

   time1=urn_time_now_LONG_LONG(36506);
   time2=urn_time_now_LONG_LONG(36507);
   diff = time2-time1;
   printf("Difference: %lld seconds (1000000LL)\n", diff / 1000000);
}

When run on a 32 bit system:

$ cc test.c -o test && ./test
Difference: -4293 seconds (1000000L)
Difference: 1 seconds (1000000LL)

Other potential issues

There are other uses of 1000000L that might have potential overflow issues:

$ grep '[0-9]L\>' * -Rn
components/splits.c:182:#define SHOW_DELTA_THRESHOLD (-30 * 1000000L)
urn.c:11:    return timespec.tv_sec * 1000000L + timespec.tv_nsec / 1000;
urn.c:44:                    + seconds) * 1000000L

The first is used in a comparison against a long long, the second is the issue already mentioned, and the last one is in urn_time_value() where the return value is long long. So I think all three should be 1000000LL to avoid bugs on 32-bit systems.

Thanks for the great timer, hope you found this helpful :).