Skip to content

Commit

Permalink
faulthandler: use _PyTime_t rather than double for timeout (#4139)
Browse files Browse the repository at this point in the history
Use the _PyTime_t type rather than double for the faulthandler
timeout in dump_traceback_later().

This change should fix the following Coverity warning:

CID 1420311:  Incorrect expression  (UNINTENDED_INTEGER_DIVISION)
Dividing integer expressions "9223372036854775807LL" and "1000LL",
and then converting the integer quotient to type "double". Any
remainder, or fractional part of the quotient, is ignored.

    if ((timeout * 1e6) >= (double) PY_TIMEOUT_MAX) {

The warning comes from (double)PY_TIMEOUT_MAX with:

    #define PY_TIMEOUT_MAX (PY_LLONG_MAX / 1000)
  • Loading branch information
vstinner committed Oct 27, 2017
1 parent 7351f9e commit 93fd478
Showing 1 changed file with 31 additions and 20 deletions.
51 changes: 31 additions & 20 deletions Modules/faulthandler.c
Expand Up @@ -607,30 +607,33 @@ cancel_dump_traceback_later(void)
}
}

#define SEC_TO_US (1000 * 1000)

static char*
format_timeout(double timeout)
format_timeout(_PyTime_t us)
{
unsigned long us, sec, min, hour;
double intpart, fracpart;
unsigned long sec, min, hour;
char buffer[100];

fracpart = modf(timeout, &intpart);
sec = (unsigned long)intpart;
us = (unsigned long)(fracpart * 1e6);
/* the downcast is safe: the caller check that 0 < us <= LONG_MAX */
sec = (unsigned long)(us / SEC_TO_US);
us %= SEC_TO_US;

min = sec / 60;
sec %= 60;
hour = min / 60;
min %= 60;

if (us != 0)
if (us != 0) {
PyOS_snprintf(buffer, sizeof(buffer),
"Timeout (%lu:%02lu:%02lu.%06lu)!\n",
hour, min, sec, us);
else
"Timeout (%lu:%02lu:%02lu.%06u)!\n",
hour, min, sec, (unsigned int)us);
}
else {
PyOS_snprintf(buffer, sizeof(buffer),
"Timeout (%lu:%02lu:%02lu)!\n",
hour, min, sec);

}
return _PyMem_Strdup(buffer);
}

Expand All @@ -639,8 +642,8 @@ faulthandler_dump_traceback_later(PyObject *self,
PyObject *args, PyObject *kwargs)
{
static char *kwlist[] = {"timeout", "repeat", "file", "exit", NULL};
double timeout;
PY_TIMEOUT_T timeout_us;
PyObject *timeout_obj;
_PyTime_t timeout, timeout_us;
int repeat = 0;
PyObject *file = NULL;
int fd;
Expand All @@ -650,18 +653,25 @@ faulthandler_dump_traceback_later(PyObject *self,
size_t header_len;

if (!PyArg_ParseTupleAndKeywords(args, kwargs,
"d|iOi:dump_traceback_later", kwlist,
&timeout, &repeat, &file, &exit))
"O|iOi:dump_traceback_later", kwlist,
&timeout_obj, &repeat, &file, &exit))
return NULL;
if ((timeout * 1e6) >= (double) PY_TIMEOUT_MAX) {
PyErr_SetString(PyExc_OverflowError, "timeout value is too large");

if (_PyTime_FromSecondsObject(&timeout, timeout_obj,
_PyTime_ROUND_TIMEOUT) < 0) {
return NULL;
}
timeout_us = (PY_TIMEOUT_T)(timeout * 1e6);
timeout_us = _PyTime_AsMicroseconds(timeout, _PyTime_ROUND_TIMEOUT);
if (timeout_us <= 0) {
PyErr_SetString(PyExc_ValueError, "timeout must be greater than 0");
return NULL;
}
/* Limit to LONG_MAX seconds for format_timeout() */
if (timeout_us >= PY_TIMEOUT_MAX || timeout_us / SEC_TO_US >= LONG_MAX) {
PyErr_SetString(PyExc_OverflowError,
"timeout value is too large");
return NULL;
}

tstate = get_thread_state();
if (tstate == NULL)
Expand All @@ -672,7 +682,7 @@ faulthandler_dump_traceback_later(PyObject *self,
return NULL;

/* format the timeout */
header = format_timeout(timeout);
header = format_timeout(timeout_us);
if (header == NULL)
return PyErr_NoMemory();
header_len = strlen(header);
Expand All @@ -683,7 +693,8 @@ faulthandler_dump_traceback_later(PyObject *self,
Py_XINCREF(file);
Py_XSETREF(thread.file, file);
thread.fd = fd;
thread.timeout_us = timeout_us;
/* the downcast is safe: we check that 0 < timeout_us < PY_TIMEOUT_MAX */
thread.timeout_us = (PY_TIMEOUT_T)timeout_us;
thread.repeat = repeat;
thread.interp = tstate->interp;
thread.exit = exit;
Expand Down

0 comments on commit 93fd478

Please sign in to comment.