From 8a3f06c54b52e5e7490a131b261384baac9d6090 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 8 Dec 2022 16:46:09 -0700 Subject: [PATCH] gh-81057: Move time Globals to _PyRuntimeState (gh-100122) https://github.com/python/cpython/issues/81057 --- Include/internal/pycore_os.h | 38 --------- Include/internal/pycore_pylifecycle.h | 1 + Include/internal/pycore_runtime.h | 4 +- Include/internal/pycore_runtime_init.h | 1 - Include/internal/pycore_time.h | 25 ++++++ Makefile.pre.in | 2 +- Modules/posixmodule.c | 30 ++----- Modules/timemodule.c | 94 ++++++++++++--------- PCbuild/_freeze_module.vcxproj | 1 + PCbuild/_freeze_module.vcxproj.filters | 3 + PCbuild/pythoncore.vcxproj | 2 +- PCbuild/pythoncore.vcxproj.filters | 6 +- Python/pylifecycle.c | 5 ++ Tools/c-analyzer/cpython/globals-to-fix.tsv | 6 -- 14 files changed, 100 insertions(+), 118 deletions(-) delete mode 100644 Include/internal/pycore_os.h create mode 100644 Include/internal/pycore_time.h diff --git a/Include/internal/pycore_os.h b/Include/internal/pycore_os.h deleted file mode 100644 index e4899bd5032..00000000000 --- a/Include/internal/pycore_os.h +++ /dev/null @@ -1,38 +0,0 @@ -#ifndef Py_INTERNAL_OS_H -#define Py_INTERNAL_OS_H -#ifdef __cplusplus -extern "C" { -#endif - -#ifndef Py_BUILD_CORE -# error "this header requires Py_BUILD_CORE define" -#endif - - -#ifndef MS_WINDOWS -#define _OS_NEED_TICKS_PER_SECOND -# define need_ticks_per_second_STATE \ - long ticks_per_second; -# define need_ticks_per_second_INIT \ - .ticks_per_second = -1, -#else -# define need_ticks_per_second_STATE -# define need_ticks_per_second_INIT -#endif /* MS_WINDOWS */ - - -struct _os_runtime_state { - int _not_used; - need_ticks_per_second_STATE -}; -# define _OS_RUNTIME_INIT \ - { \ - ._not_used = 0, \ - need_ticks_per_second_INIT \ - } - - -#ifdef __cplusplus -} -#endif -#endif /* !Py_INTERNAL_OS_H */ diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index 4c0ffa7a9b1..370e4cbd59f 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -44,6 +44,7 @@ extern void _PySys_Fini(PyInterpreterState *interp); extern int _PyBuiltins_AddExceptions(PyObject * bltinmod); extern PyStatus _Py_HashRandomization_Init(const PyConfig *); +extern PyStatus _PyTime_Init(void); extern PyStatus _PyImportZip_Init(PyThreadState *tstate); extern PyStatus _PyGC_Init(PyInterpreterState *interp); extern PyStatus _PyAtExit_Init(PyInterpreterState *interp); diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index c6b48cae776..458493eac70 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -21,7 +21,7 @@ extern "C" { #include "pycore_pymem.h" // struct _pymem_allocators #include "pycore_pyhash.h" // struct pyhash_runtime_state #include "pycore_obmalloc.h" // struct obmalloc_state -#include "pycore_os.h" // struct _os_runtime_state +#include "pycore_time.h" // struct _time_runtime_state #include "pycore_unicodeobject.h" // struct _Py_unicode_runtime_ids struct _getargs_runtime_state { @@ -104,7 +104,7 @@ typedef struct pyruntimestate { * KeyboardInterrupt exception, suggesting the user pressed ^C. */ int unhandled_keyboard_interrupt; } signals; - struct _os_runtime_state os; + struct _time_runtime_state time; struct pyinterpreters { PyThread_type_lock mutex; diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 70263892e57..ab53876e355 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -26,7 +26,6 @@ extern "C" { }, \ .obmalloc = _obmalloc_state_INIT(runtime.obmalloc), \ .pyhash_state = pyhash_state_INIT, \ - .os = _OS_RUNTIME_INIT, \ .interpreters = { \ /* This prevents interpreters from getting created \ until _PyInterpreterState_Enable() is called. */ \ diff --git a/Include/internal/pycore_time.h b/Include/internal/pycore_time.h new file mode 100644 index 00000000000..949170c4493 --- /dev/null +++ b/Include/internal/pycore_time.h @@ -0,0 +1,25 @@ +#ifndef Py_INTERNAL_TIME_H +#define Py_INTERNAL_TIME_H +#ifdef __cplusplus +extern "C" { +#endif + +#ifndef Py_BUILD_CORE +# error "this header requires Py_BUILD_CORE define" +#endif + + +struct _time_runtime_state { +#ifdef HAVE_TIMES + int ticks_per_second_initialized; + long ticks_per_second; +#else + int _not_used; +#endif +}; + + +#ifdef __cplusplus +} +#endif +#endif /* !Py_INTERNAL_TIME_H */ diff --git a/Makefile.pre.in b/Makefile.pre.in index 80144e52887..c21826fbb94 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1654,7 +1654,6 @@ PYTHON_HEADERS= \ $(srcdir)/Include/internal/pycore_object.h \ $(srcdir)/Include/internal/pycore_obmalloc.h \ $(srcdir)/Include/internal/pycore_obmalloc_init.h \ - $(srcdir)/Include/internal/pycore_os.h \ $(srcdir)/Include/internal/pycore_pathconfig.h \ $(srcdir)/Include/internal/pycore_pyarena.h \ $(srcdir)/Include/internal/pycore_pyerrors.h \ @@ -1673,6 +1672,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/internal/pycore_structseq.h \ $(srcdir)/Include/internal/pycore_symtable.h \ $(srcdir)/Include/internal/pycore_sysmodule.h \ + $(srcdir)/Include/internal/pycore_time.h \ $(srcdir)/Include/internal/pycore_token.h \ $(srcdir)/Include/internal/pycore_traceback.h \ $(srcdir)/Include/internal/pycore_tuple.h \ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index cbf4d5b3fce..f5175350e12 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -9065,24 +9065,6 @@ build_times_result(PyObject *module, double user, double system, } -#ifdef _OS_NEED_TICKS_PER_SECOND -#define ticks_per_second _PyRuntime.os.ticks_per_second -static void -ticks_per_second_init(void) -{ - if (ticks_per_second != -1) { - return; - } -# if defined(HAVE_SYSCONF) && defined(_SC_CLK_TCK) - ticks_per_second = sysconf(_SC_CLK_TCK); -# elif defined(HZ) - ticks_per_second = HZ; -# else - ticks_per_second = 60; /* magic fallback value; may be bogus */ -# endif -} -#endif - /*[clinic input] os.times @@ -9116,22 +9098,24 @@ os_times_impl(PyObject *module) (double)0, (double)0); } -#elif !defined(_OS_NEED_TICKS_PER_SECOND) -# error "missing ticks_per_second" #else /* MS_WINDOWS */ { struct tms t; clock_t c; errno = 0; c = times(&t); - if (c == (clock_t) -1) + if (c == (clock_t) -1) { return posix_error(); + } + assert(_PyRuntime.time.ticks_per_second_initialized); +#define ticks_per_second _PyRuntime.time.ticks_per_second return build_times_result(module, (double)t.tms_utime / ticks_per_second, (double)t.tms_stime / ticks_per_second, (double)t.tms_cutime / ticks_per_second, (double)t.tms_cstime / ticks_per_second, (double)c / ticks_per_second); +#undef ticks_per_second } #endif /* MS_WINDOWS */ #endif /* HAVE_TIMES */ @@ -15950,10 +15934,6 @@ posixmodule_exec(PyObject *m) PyModule_AddObject(m, "statvfs_result", Py_NewRef(StatVFSResultType)); state->StatVFSResultType = StatVFSResultType; -#ifdef _OS_NEED_TICKS_PER_SECOND - ticks_per_second_init(); -#endif - #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM) sched_param_desc.name = MODNAME ".sched_param"; PyObject *SchedParamType = (PyObject *)PyStructSequence_NewType(&sched_param_desc); diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 11c888af03e..ba4128c0fdf 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -62,6 +62,54 @@ #define SEC_TO_NS (1000 * 1000 * 1000) +#ifdef HAVE_TIMES + +static int +check_ticks_per_second(long tps, const char *context) +{ + /* Effectively, check that _PyTime_MulDiv(t, SEC_TO_NS, ticks_per_second) + cannot overflow. */ + if (tps >= 0 && (_PyTime_t)tps > _PyTime_MAX / SEC_TO_NS) { + PyErr_Format(PyExc_OverflowError, "%s is too large", context); + return -1; + } + return 0; +} + +# define ticks_per_second _PyRuntime.time.ticks_per_second + +static void +ensure_ticks_per_second(void) +{ + if (_PyRuntime.time.ticks_per_second_initialized) { + return; + } + _PyRuntime.time.ticks_per_second_initialized = 1; +# if defined(HAVE_SYSCONF) && defined(_SC_CLK_TCK) + ticks_per_second = sysconf(_SC_CLK_TCK); + if (ticks_per_second < 1) { + ticks_per_second = -1; + } +# elif defined(HZ) + ticks_per_second = HZ; +# else + ticks_per_second = 60; /* magic fallback value; may be bogus */ +# endif +} + +#endif /* HAVE_TIMES */ + + +PyStatus +_PyTime_Init(void) +{ +#ifdef HAVE_TIMES + ensure_ticks_per_second(); +#endif + return PyStatus_Ok(); +} + + /* Forward declarations */ static int pysleep(_PyTime_t timeout); @@ -140,18 +188,8 @@ Return the current time in nanoseconds since the Epoch."); static int _PyTime_GetClockWithInfo(_PyTime_t *tp, _Py_clock_info_t *info) { - static int initialized = 0; - - if (!initialized) { - initialized = 1; - - /* Make sure that _PyTime_MulDiv(ticks, SEC_TO_NS, CLOCKS_PER_SEC) - above cannot overflow */ - if ((_PyTime_t)CLOCKS_PER_SEC > _PyTime_MAX / SEC_TO_NS) { - PyErr_SetString(PyExc_OverflowError, - "CLOCKS_PER_SEC is too large"); - return -1; - } + if (check_ticks_per_second(CLOCKS_PER_SEC, "CLOCKS_PER_SEC") < 0) { + return -1; } if (info) { @@ -1308,36 +1346,10 @@ _PyTime_GetProcessTimeWithInfo(_PyTime_t *tp, _Py_clock_info_t *info) struct tms t; if (times(&t) != (clock_t)-1) { - static long ticks_per_second = -1; - - if (ticks_per_second == -1) { - long freq; -#if defined(HAVE_SYSCONF) && defined(_SC_CLK_TCK) - freq = sysconf(_SC_CLK_TCK); - if (freq < 1) { - freq = -1; - } -#elif defined(HZ) - freq = HZ; -#else - freq = 60; /* magic fallback value; may be bogus */ -#endif - - if (freq != -1) { - /* check that _PyTime_MulDiv(t, SEC_TO_NS, ticks_per_second) - cannot overflow below */ -#if LONG_MAX > _PyTime_MAX / SEC_TO_NS - if ((_PyTime_t)freq > _PyTime_MAX / SEC_TO_NS) { - PyErr_SetString(PyExc_OverflowError, - "_SC_CLK_TCK is too large"); - return -1; - } -#endif - - ticks_per_second = freq; - } + assert(_PyRuntime.time.ticks_per_second_initialized); + if (check_ticks_per_second(ticks_per_second, "_SC_CLK_TCK") < 0) { + return -1; } - if (ticks_per_second != -1) { if (info) { info->implementation = "times()"; diff --git a/PCbuild/_freeze_module.vcxproj b/PCbuild/_freeze_module.vcxproj index 8454bd67b1d..fce1f670510 100644 --- a/PCbuild/_freeze_module.vcxproj +++ b/PCbuild/_freeze_module.vcxproj @@ -110,6 +110,7 @@ + diff --git a/PCbuild/_freeze_module.vcxproj.filters b/PCbuild/_freeze_module.vcxproj.filters index 6e8498dceb1..dce6278987c 100644 --- a/PCbuild/_freeze_module.vcxproj.filters +++ b/PCbuild/_freeze_module.vcxproj.filters @@ -367,6 +367,9 @@ Source Files + + Source Files + Source Files diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index fa3924968a6..f9f0c191b5e 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -236,7 +236,6 @@ - @@ -255,6 +254,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index e29c6b2330d..e683650ad7d 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -612,9 +612,6 @@ Include\internal - - Include\internal - Include\internal @@ -666,6 +663,9 @@ Include\internal + + Include\internal + Include\internal diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 8209132ebc6..e13660f5113 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -606,6 +606,11 @@ pycore_init_runtime(_PyRuntimeState *runtime, return status; } + status = _PyTime_Init(); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + status = _PyImport_Init(); if (_PyStatus_EXCEPTION(status)) { return status; diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index 8e05bc3e4f2..3f6e1c74053 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -387,12 +387,6 @@ Modules/faulthandler.c - old_stack - ################################## ## global non-objects to fix in builtin modules -##----------------------- -## initialized once - -Modules/timemodule.c _PyTime_GetClockWithInfo initialized - -Modules/timemodule.c _PyTime_GetProcessTimeWithInfo ticks_per_second - - ##----------------------- ## state