gh-117657: Fix small issues with instrumentation and TSAN (#118064)

Small TSAN fixups for instrumentation
This commit is contained in:
Dino Viehland 2024-04-30 11:38:05 -07:00 committed by GitHub
parent 1f16b4ce56
commit 4a1cf66c5c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 25 additions and 13 deletions

View File

@ -39,6 +39,10 @@ extern "C" {
_Py_atomic_load_uint8(&value) _Py_atomic_load_uint8(&value)
#define FT_ATOMIC_STORE_UINT8(value, new_value) \ #define FT_ATOMIC_STORE_UINT8(value, new_value) \
_Py_atomic_store_uint8(&value, new_value) _Py_atomic_store_uint8(&value, new_value)
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) \
_Py_atomic_load_uint8_relaxed(&value)
#define FT_ATOMIC_LOAD_UINT16_RELAXED(value) \
_Py_atomic_load_uint16_relaxed(&value)
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \ #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
_Py_atomic_store_ptr_relaxed(&value, new_value) _Py_atomic_store_ptr_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \ #define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
@ -49,7 +53,8 @@ extern "C" {
_Py_atomic_store_ssize_relaxed(&value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \
_Py_atomic_store_uint8_relaxed(&value, new_value) _Py_atomic_store_uint8_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) \
_Py_atomic_store_uint16_relaxed(&value, new_value)
#else #else
#define FT_ATOMIC_LOAD_PTR(value) value #define FT_ATOMIC_LOAD_PTR(value) value
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value #define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
@ -62,12 +67,14 @@ extern "C" {
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) value #define FT_ATOMIC_LOAD_PTR_RELAXED(value) value
#define FT_ATOMIC_LOAD_UINT8(value) value #define FT_ATOMIC_LOAD_UINT8(value) value
#define FT_ATOMIC_STORE_UINT8(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8(value, new_value) value = new_value
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) value
#define FT_ATOMIC_LOAD_UINT16_RELAXED(value) value
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value
#endif #endif
#ifdef __cplusplus #ifdef __cplusplus

View File

@ -11,6 +11,7 @@
#include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_modsupport.h" // _PyArg_CheckPositional()
#include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_object.h" // _PyObject_GC_UNTRACK()
#include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM #include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*
#include "pycore_pyerrors.h" // _PyErr_ClearExcState() #include "pycore_pyerrors.h" // _PyErr_ClearExcState()
#include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_pystate.h" // _PyThreadState_GET()
@ -329,10 +330,11 @@ gen_close_iter(PyObject *yf)
static inline bool static inline bool
is_resume(_Py_CODEUNIT *instr) is_resume(_Py_CODEUNIT *instr)
{ {
uint8_t code = FT_ATOMIC_LOAD_UINT8_RELAXED(instr->op.code);
return ( return (
instr->op.code == RESUME || code == RESUME ||
instr->op.code == RESUME_CHECK || code == RESUME_CHECK ||
instr->op.code == INSTRUMENTED_RESUME code == INSTRUMENTED_RESUME
); );
} }

View File

@ -20,7 +20,7 @@
#include "pycore_object.h" // _PyObject_GC_TRACK() #include "pycore_object.h" // _PyObject_GC_TRACK()
#include "pycore_opcode_metadata.h" // uop names #include "pycore_opcode_metadata.h" // uop names
#include "pycore_opcode_utils.h" // MAKE_FUNCTION_* #include "pycore_opcode_utils.h" // MAKE_FUNCTION_*
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_ACQUIRE #include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*
#include "pycore_pyerrors.h" // _PyErr_GetRaisedException() #include "pycore_pyerrors.h" // _PyErr_GetRaisedException()
#include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_pystate.h" // _PyInterpreterState_GET()
#include "pycore_range.h" // _PyRangeIterObject #include "pycore_range.h" // _PyRangeIterObject
@ -163,7 +163,7 @@ dummy_func(
if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) {
CHECK_EVAL_BREAKER(); CHECK_EVAL_BREAKER();
} }
this_instr->op.code = RESUME_CHECK; FT_ATOMIC_STORE_UINT8_RELAXED(this_instr->op.code, RESUME_CHECK);
} }
} }

View File

@ -20,7 +20,7 @@
#include "pycore_opcode_metadata.h" // EXTRA_CASES #include "pycore_opcode_metadata.h" // EXTRA_CASES
#include "pycore_optimizer.h" // _PyUOpExecutor_Type #include "pycore_optimizer.h" // _PyUOpExecutor_Type
#include "pycore_opcode_utils.h" // MAKE_FUNCTION_* #include "pycore_opcode_utils.h" // MAKE_FUNCTION_*
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_ACQUIRE #include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*
#include "pycore_pyerrors.h" // _PyErr_GetRaisedException() #include "pycore_pyerrors.h" // _PyErr_GetRaisedException()
#include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_pystate.h" // _PyInterpreterState_GET()
#include "pycore_range.h" // _PyRangeIterObject #include "pycore_range.h" // _PyRangeIterObject

View File

@ -162,7 +162,7 @@ GETITEM(PyObject *v, Py_ssize_t i) {
/* The integer overflow is checked by an assertion below. */ /* The integer overflow is checked by an assertion below. */
#define INSTR_OFFSET() ((int)(next_instr - _PyCode_CODE(_PyFrame_GetCode(frame)))) #define INSTR_OFFSET() ((int)(next_instr - _PyCode_CODE(_PyFrame_GetCode(frame))))
#define NEXTOPARG() do { \ #define NEXTOPARG() do { \
_Py_CODEUNIT word = *next_instr; \ _Py_CODEUNIT word = {.cache = FT_ATOMIC_LOAD_UINT16_RELAXED(*(uint16_t*)next_instr)}; \
opcode = word.op.code; \ opcode = word.op.code; \
oparg = word.op.arg; \ oparg = word.op.arg; \
} while (0) } while (0)

View File

@ -4955,7 +4955,7 @@
if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) {
CHECK_EVAL_BREAKER(); CHECK_EVAL_BREAKER();
} }
this_instr->op.code = RESUME_CHECK; FT_ATOMIC_STORE_UINT8_RELAXED(this_instr->op.code, RESUME_CHECK);
} }
DISPATCH(); DISPATCH();
} }

View File

@ -626,9 +626,10 @@ de_instrument(PyCodeObject *code, int i, int event)
return; return;
} }
CHECK(_PyOpcode_Deopt[deinstrumented] == deinstrumented); CHECK(_PyOpcode_Deopt[deinstrumented] == deinstrumented);
*opcode_ptr = deinstrumented; FT_ATOMIC_STORE_UINT8_RELAXED(*opcode_ptr, deinstrumented);
if (_PyOpcode_Caches[deinstrumented]) { if (_PyOpcode_Caches[deinstrumented]) {
instr[1].counter = adaptive_counter_warmup(); FT_ATOMIC_STORE_UINT16_RELAXED(instr[1].counter.as_counter,
adaptive_counter_warmup().as_counter);
} }
} }
@ -703,8 +704,10 @@ instrument(PyCodeObject *code, int i)
int deopt = _PyOpcode_Deopt[opcode]; int deopt = _PyOpcode_Deopt[opcode];
int instrumented = INSTRUMENTED_OPCODES[deopt]; int instrumented = INSTRUMENTED_OPCODES[deopt];
assert(instrumented); assert(instrumented);
*opcode_ptr = instrumented; FT_ATOMIC_STORE_UINT8_RELAXED(*opcode_ptr, instrumented);
if (_PyOpcode_Caches[deopt]) { if (_PyOpcode_Caches[deopt]) {
FT_ATOMIC_STORE_UINT16_RELAXED(instr[1].counter.as_counter,
adaptive_counter_warmup().as_counter);
instr[1].counter = adaptive_counter_warmup(); instr[1].counter = adaptive_counter_warmup();
} }
} }