Skip to content

Commit 78f1b64

Browse files
DinoVDinoV
committed
Small TSAN fixups for instrumentation
1 parent 8f25cc9 commit 78f1b64

File tree

7 files changed

+34
-10
lines changed

7 files changed

+34
-10
lines changed

‎Include/internal/pycore_pyatomic_ft_wrappers.h‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,35 @@ extern "C" {
2626
_Py_atomic_load_ssize_relaxed(&value)
2727
#define FT_ATOMIC_STORE_PTR(value, new_value) \
2828
_Py_atomic_store_ptr(&value, new_value)
29+
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) \
30+
_Py_atomic_load_uint8_relaxed(&value)
31+
#define FT_ATOMIC_LOAD_UINT16_RELAXED(value) \
32+
_Py_atomic_load_uint16_relaxed(&value)
33+
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) \
34+
_Py_atomic_load_uintptr_acquire(&value)
2935
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
3036
_Py_atomic_store_ptr_relaxed(&value, new_value)
3137
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
3238
_Py_atomic_store_ptr_release(&value, new_value)
3339
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \
3440
_Py_atomic_store_ssize_relaxed(&value, new_value)
41+
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \
42+
_Py_atomic_store_uint8_relaxed(&value, new_value)
43+
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) \
44+
_Py_atomic_store_uint16_relaxed(&value, new_value)
3545
#else
3646
#define FT_ATOMIC_LOAD_PTR(value) value
3747
#define FT_ATOMIC_LOAD_SSIZE(value) value
3848
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
3949
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
50+
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) value
51+
#define FT_ATOMIC_LOAD_UINT16_RELAXED(value) value
52+
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value
4053
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
4154
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
4255
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
56+
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
57+
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value
4358
#endif
4459

4560
#ifdef __cplusplus

‎Objects/genobject.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "pycore_modsupport.h" // _PyArg_CheckPositional()
1212
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
1313
#include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM
14+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*
1415
#include "pycore_pyerrors.h" // _PyErr_ClearExcState()
1516
#include "pycore_pystate.h" // _PyThreadState_GET()
1617

@@ -329,10 +330,11 @@ gen_close_iter(PyObject *yf)
329330
static inline bool
330331
is_resume(_Py_CODEUNIT *instr)
331332
{
333+
uint8_t code = FT_ATOMIC_LOAD_UINT8_RELAXED(instr->op.code);
332334
return (
333-
instr->op.code == RESUME ||
334-
instr->op.code == RESUME_CHECK ||
335-
instr->op.code == INSTRUMENTED_RESUME
335+
code == RESUME ||
336+
code == RESUME_CHECK ||
337+
code == INSTRUMENTED_RESUME
336338
);
337339
}
338340

‎Python/bytecodes.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "pycore_object.h" // _PyObject_GC_TRACK()
2121
#include "pycore_opcode_metadata.h" // uop names
2222
#include "pycore_opcode_utils.h" // MAKE_FUNCTION_*
23+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*
2324
#include "pycore_pyerrors.h" // _PyErr_GetRaisedException()
2425
#include "pycore_pystate.h" // _PyInterpreterState_GET()
2526
#include "pycore_range.h" // _PyRangeIterObject
@@ -161,7 +162,7 @@ dummy_func(
161162
if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) {
162163
CHECK_EVAL_BREAKER();
163164
}
164-
this_instr->op.code = RESUME_CHECK;
165+
FT_ATOMIC_STORE_UINT8_RELAXED(this_instr->op.code, RESUME_CHECK);
165166
}
166167
}
167168

@@ -178,7 +179,7 @@ dummy_func(
178179

179180
inst(INSTRUMENTED_RESUME, (--)) {
180181
uintptr_t global_version = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & ~_PY_EVAL_EVENTS_MASK;
181-
uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version;
182+
uintptr_t code_version = FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(_PyFrame_GetCode(frame)->_co_instrumentation_version);
182183
if (code_version != global_version) {
183184
if (_Py_Instrument(_PyFrame_GetCode(frame), tstate->interp)) {
184185
ERROR_NO_POP();

‎Python/ceval.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "pycore_opcode_metadata.h" // EXTRA_CASES
2121
#include "pycore_optimizer.h" // _PyUOpExecutor_Type
2222
#include "pycore_opcode_utils.h" // MAKE_FUNCTION_*
23+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*
2324
#include "pycore_pyerrors.h" // _PyErr_GetRaisedException()
2425
#include "pycore_pystate.h" // _PyInterpreterState_GET()
2526
#include "pycore_range.h" // _PyRangeIterObject

‎Python/ceval_macros.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ GETITEM(PyObject *v, Py_ssize_t i) {
150150
/* The integer overflow is checked by an assertion below. */
151151
#define INSTR_OFFSET() ((int)(next_instr - _PyCode_CODE(_PyFrame_GetCode(frame))))
152152
#define NEXTOPARG() do { \
153-
_Py_CODEUNIT word = *next_instr; \
153+
_Py_CODEUNIT word = {.cache = FT_ATOMIC_LOAD_UINT16_RELAXED(*(uint16_t*)next_instr)}; \
154154
opcode = word.op.code; \
155155
oparg = word.op.arg; \
156156
} while (0)

‎Python/generated_cases.c.h‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/instrumentation.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "pycore_namespace.h"
1414
#include "pycore_object.h"
1515
#include "pycore_opcode_metadata.h" // IS_VALID_OPCODE, _PyOpcode_Caches
16+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_STORE_UINTPTR_RELEASE
1617
#include "pycore_pyerrors.h"
1718
#include "pycore_pystate.h" // _PyInterpreterState_GET()
1819

@@ -588,9 +589,10 @@ de_instrument(PyCodeObject *code, int i, int event)
588589
return;
589590
}
590591
CHECK(_PyOpcode_Deopt[deinstrumented] == deinstrumented);
591-
*opcode_ptr = deinstrumented;
592+
FT_ATOMIC_STORE_UINT8_RELAXED(*opcode_ptr, deinstrumented);
592593
if (_PyOpcode_Caches[deinstrumented]) {
593-
instr[1].counter = adaptive_counter_warmup();
594+
FT_ATOMIC_STORE_UINT16_RELAXED(instr[1].counter.as_counter,
595+
adaptive_counter_warmup().as_counter);
594596
}
595597
}
596598

@@ -665,8 +667,11 @@ instrument(PyCodeObject *code, int i)
665667
int deopt = _PyOpcode_Deopt[opcode];
666668
int instrumented = INSTRUMENTED_OPCODES[deopt];
667669
assert(instrumented);
670+
FT_ATOMIC_STORE_UINT8_RELAXED(*opcode_ptr, instrumented);
668671
*opcode_ptr = instrumented;
669672
if (_PyOpcode_Caches[deopt]) {
673+
FT_ATOMIC_STORE_UINT16_RELAXED(instr[1].counter.as_counter,
674+
adaptive_counter_warmup().as_counter);
670675
instr[1].counter = adaptive_counter_warmup();
671676
}
672677
}

0 commit comments

Comments
 (0)