Skip to content

Commit 3937083

Browse files
jyasskinjyasskin
committed
Make (most of) Python's tests pass under Thread Sanitizer.
http://code.google.com/p/data-race-test/wiki/ThreadSanitizer is a dynamic data race detector that runs on top of valgrind. With this patch, the binaries at http://code.google.com/p/data-race-test/wiki/ThreadSanitizer#Binaries pass many but not all of the Python tests. All of regrtest still passes outside of tsan. I've implemented part of the C1x atomic types so that we can explicitly mark variables that are used across threads, and get defined behavior as compilers advance. I've added tsan's client header and implementation to the codebase in dynamic_annotations.{h,c} (docs at http://code.google.com/p/data-race-test/wiki/DynamicAnnotations). Unfortunately, I haven't been able to get helgrind and drd to give sensible error messages, even when I use their client annotations, so I'm not supporting them.
1 parent 6be8876 commit 3937083

File tree

17 files changed

+971
-63
lines changed

17 files changed

+971
-63
lines changed

‎Include/Python.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949

5050
#include "pyport.h"
5151

52+
#include "pyatomic.h"
53+
5254
/* Debug-mode build with pymalloc implies PYMALLOC_DEBUG.
5355
* PYMALLOC_DEBUG is in error if pymalloc is not in use.
5456
*/

‎Include/dynamic_annotations.h‎

Lines changed: 499 additions & 0 deletions
Large diffs are not rendered by default.

‎Include/pyatomic.h‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
#ifndef Py_ATOMIC_H
2+
#define Py_ATOMIC_H
3+
/* XXX: When compilers start offering a stdatomic.h with lock-free
4+
atomic_int and atomic_address types, include that here and rewrite
5+
the atomic operations in terms of it. */
6+
7+
#include "dynamic_annotations.h"
8+
9+
#ifdef __cplusplus
10+
extern "C" {
11+
#endif
12+
13+
/* This is modeled after the atomics interface from C1x, according to
14+
* the draft at
15+
* http://www.open-std.org/JTC1/SC22/wg14/www/docs/n1425.pdf.
16+
* Operations and types are named the same except with a _Py_ prefix
17+
* and have the same semantics.
18+
*
19+
* Beware, the implementations here are deep magic.
20+
*/
21+
22+
typedef enum _Py_memory_order {
23+
_Py_memory_order_relaxed,
24+
_Py_memory_order_acquire,
25+
_Py_memory_order_release,
26+
_Py_memory_order_acq_rel,
27+
_Py_memory_order_seq_cst
28+
} _Py_memory_order;
29+
30+
typedef struct _Py_atomic_address {
31+
void *_value;
32+
} _Py_atomic_address;
33+
34+
typedef struct _Py_atomic_int {
35+
int _value;
36+
} _Py_atomic_int;
37+
38+
/* Only support GCC (for expression statements) and x86 (for simple
39+
* atomic semantics) for now */
40+
#if defined(__GNUC__) && (defined(__i386__) || defined(__amd64))
41+
42+
static __inline__ void
43+
_Py_atomic_signal_fence(_Py_memory_order order)
44+
{
45+
if (order != _Py_memory_order_relaxed)
46+
__asm__ volatile("":::"memory");
47+
}
48+
49+
static __inline__ void
50+
_Py_atomic_thread_fence(_Py_memory_order order)
51+
{
52+
if (order != _Py_memory_order_relaxed)
53+
__asm__ volatile("mfence":::"memory");
54+
}
55+
56+
/* Tell the race checker about this operation's effects. */
57+
static __inline__ void
58+
_Py_ANNOTATE_MEMORY_ORDER(const volatile void *address, _Py_memory_order order)
59+
{
60+
switch(order) {
61+
case _Py_memory_order_release:
62+
case _Py_memory_order_acq_rel:
63+
case _Py_memory_order_seq_cst:
64+
_Py_ANNOTATE_HAPPENS_BEFORE(address);
65+
break;
66+
default:
67+
break;
68+
}
69+
switch(order) {
70+
case _Py_memory_order_acquire:
71+
case _Py_memory_order_acq_rel:
72+
case _Py_memory_order_seq_cst:
73+
_Py_ANNOTATE_HAPPENS_AFTER(address);
74+
break;
75+
default:
76+
break;
77+
}
78+
}
79+
80+
#define _Py_atomic_store_explicit(ATOMIC_VAL, NEW_VAL, ORDER) \
81+
__extension__ ({ \
82+
__typeof__(ATOMIC_VAL) atomic_val = ATOMIC_VAL; \
83+
__typeof__(atomic_val->_value) new_val = NEW_VAL;\
84+
volatile __typeof__(new_val) *volatile_data = &atomic_val->_value; \
85+
_Py_memory_order order = ORDER; \
86+
_Py_ANNOTATE_MEMORY_ORDER(atomic_val, order); \
87+
\
88+
/* Perform the operation. */ \
89+
_Py_ANNOTATE_IGNORE_WRITES_BEGIN(); \
90+
switch(order) { \
91+
case _Py_memory_order_release: \
92+
_Py_atomic_signal_fence(_Py_memory_order_release); \
93+
/* fallthrough */ \
94+
case _Py_memory_order_relaxed: \
95+
*volatile_data = new_val; \
96+
break; \
97+
\
98+
case _Py_memory_order_acquire: \
99+
case _Py_memory_order_acq_rel: \
100+
case _Py_memory_order_seq_cst: \
101+
__asm__ volatile("xchg %0, %1" \
102+
: "+r"(new_val) \
103+
: "m"(atomic_val->_value) \
104+
: "memory"); \
105+
break; \
106+
} \
107+
_Py_ANNOTATE_IGNORE_WRITES_END(); \
108+
})
109+
110+
#define _Py_atomic_load_explicit(ATOMIC_VAL, ORDER) \
111+
__extension__ ({ \
112+
__typeof__(ATOMIC_VAL) atomic_val = ATOMIC_VAL; \
113+
__typeof__(atomic_val->_value) result; \
114+
volatile __typeof__(result) *volatile_data = &atomic_val->_value; \
115+
_Py_memory_order order = ORDER; \
116+
_Py_ANNOTATE_MEMORY_ORDER(atomic_val, order); \
117+
\
118+
/* Perform the operation. */ \
119+
_Py_ANNOTATE_IGNORE_READS_BEGIN(); \
120+
switch(order) { \
121+
case _Py_memory_order_release: \
122+
case _Py_memory_order_acq_rel: \
123+
case _Py_memory_order_seq_cst: \
124+
/* Loads on x86 are not releases by default, so need a */ \
125+
/* thread fence. */ \
126+
_Py_atomic_thread_fence(_Py_memory_order_release); \
127+
break; \
128+
default: \
129+
/* No fence */ \
130+
break; \
131+
} \
132+
result = *volatile_data; \
133+
switch(order) { \
134+
case _Py_memory_order_acquire: \
135+
case _Py_memory_order_acq_rel: \
136+
case _Py_memory_order_seq_cst: \
137+
/* Loads on x86 are automatically acquire operations so */ \
138+
/* can get by with just a compiler fence. */ \
139+
_Py_atomic_signal_fence(_Py_memory_order_acquire); \
140+
break; \
141+
default: \
142+
/* No fence */ \
143+
break; \
144+
} \
145+
_Py_ANNOTATE_IGNORE_READS_END(); \
146+
result; \
147+
})
148+
149+
#else /* !gcc x86 */
150+
/* Fall back to other compilers and processors by assuming that simple
151+
volatile accesses are atomic. This is false, so people should port
152+
this. */
153+
#define _Py_atomic_signal_fence(/*memory_order*/ ORDER) ((void)0)
154+
#define _Py_atomic_thread_fence(/*memory_order*/ ORDER) ((void)0)
155+
#define _Py_atomic_store_explicit(ATOMIC_VAL, NEW_VAL, ORDER) \
156+
((ATOMIC_VAL)->_value = NEW_VAL)
157+
#define _Py_atomic_load_explicit(ATOMIC_VAL, ORDER) \
158+
((ATOMIC_VAL)->_value)
159+
160+
#endif /* !gcc x86 */
161+
162+
/* Standardized shortcuts. */
163+
#define _Py_atomic_store(ATOMIC_VAL, NEW_VAL) \
164+
_Py_atomic_store_explicit(ATOMIC_VAL, NEW_VAL, _Py_memory_order_seq_cst)
165+
#define _Py_atomic_load(ATOMIC_VAL) \
166+
_Py_atomic_load_explicit(ATOMIC_VAL, _Py_memory_order_seq_cst)
167+
168+
/* Python-local extensions */
169+
170+
#define _Py_atomic_store_relaxed(ATOMIC_VAL, NEW_VAL) \
171+
_Py_atomic_store_explicit(ATOMIC_VAL, NEW_VAL, _Py_memory_order_relaxed)
172+
#define _Py_atomic_load_relaxed(ATOMIC_VAL) \
173+
_Py_atomic_load_explicit(ATOMIC_VAL, _Py_memory_order_relaxed)
174+
175+
#ifdef __cplusplus
176+
}
177+
#endif
178+
179+
#endif /* Py_ATOMIC_H */

‎Include/pystate.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,15 @@ PyAPI_FUNC(int) PyThreadState_SetAsyncExc(long, PyObject *);
131131

132132
/* Variable and macro for in-line access to current thread state */
133133

134-
PyAPI_DATA(PyThreadState *) _PyThreadState_Current;
134+
/* Assuming the current thread holds the GIL, this is the
135+
PyThreadState for the current thread. */
136+
PyAPI_DATA(_Py_atomic_address) _PyThreadState_Current;
135137

136138
#ifdef Py_DEBUG
137139
#define PyThreadState_GET() PyThreadState_Get()
138140
#else
139-
#define PyThreadState_GET() (_PyThreadState_Current)
141+
#define PyThreadState_GET() \
142+
((PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current))
140143
#endif
141144

142145
typedef

‎Makefile.pre.in‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ PARSER_OBJS= $(POBJS) Parser/myreadline.o Parser/tokenizer.o
238238

239239
PGOBJS= \
240240
Objects/obmalloc.o \
241+
Python/dynamic_annotations.o \
241242
Python/mysnprintf.o \
242243
Python/pyctype.o \
243244
Parser/tokenizer_pgen.o \
@@ -283,6 +284,7 @@ PYTHON_OBJS= \
283284
Python/ceval.o \
284285
Python/compile.o \
285286
Python/codecs.o \
287+
Python/dynamic_annotations.o \
286288
Python/errors.o \
287289
Python/frozen.o \
288290
Python/frozenmain.o \
@@ -644,6 +646,7 @@ PYTHON_HEADERS= \
644646
Include/descrobject.h \
645647
Include/dictobject.h \
646648
Include/dtoa.h \
649+
Include/dynamic_annotations.h \
647650
Include/enumobject.h \
648651
Include/errcode.h \
649652
Include/eval.h \
@@ -674,6 +677,7 @@ PYTHON_HEADERS= \
674677
Include/pgen.h \
675678
Include/pgenheaders.h \
676679
Include/pyarena.h \
680+
Include/pyatomic.h \
677681
Include/pycapsule.h \
678682
Include/pyctype.h \
679683
Include/pydebug.h \

‎Objects/dictobject.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,8 @@ PyDict_GetItem(PyObject *op, PyObject *key)
734734
Let's just hope that no exception occurs then... This must be
735735
_PyThreadState_Current and not PyThreadState_GET() because in debug
736736
mode, the latter complains if tstate is NULL. */
737-
tstate = _PyThreadState_Current;
737+
tstate = (PyThreadState*)_Py_atomic_load_relaxed(
738+
&_PyThreadState_Current);
738739
if (tstate != NULL && tstate->curexc_type != NULL) {
739740
/* preserve the existing exception */
740741
PyObject *err_type, *err_value, *err_tb;

‎PC/VS7.1/pythoncore.vcproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,9 @@
507507
<File
508508
RelativePath="..\..\PC\config.c">
509509
</File>
510+
<File
511+
RelativePath="..\..\Python\dynamic_annotations.c">
512+
</File>
510513
<File
511514
RelativePath="..\..\Modules\datetimemodule.c">
512515
</File>

‎PC/VS8.0/pythoncore.vcproj‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,10 @@
706706
RelativePath="..\..\Include\dtoa.h"
707707
>
708708
</File>
709+
<File
710+
RelativePath="..\..\Include\dynamic_annotations.h"
711+
>
712+
</File>
709713
<File
710714
RelativePath="..\..\Include\enumobject.h"
711715
>
@@ -1654,6 +1658,10 @@
16541658
RelativePath="..\..\Python\compile.c"
16551659
>
16561660
</File>
1661+
<File
1662+
RelativePath="..\..\Python\dynamic_annotations.c"
1663+
>
1664+
</File>
16571665
<File
16581666
RelativePath="..\..\Python\dtoa.c"
16591667
>

‎PC/os2emx/Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ SRC.PYTHON= $(addprefix $(TOP), \
332332
Python/ceval.c \
333333
Python/compile.c \
334334
Python/codecs.c \
335+
Python/dynamic_annotations.c \
335336
Python/errors.c \
336337
Python/frozen.c \
337338
Python/frozenmain.c \

‎PCbuild/pythoncore.vcproj‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,10 @@
702702
RelativePath="..\Include\dictobject.h"
703703
>
704704
</File>
705+
<File
706+
RelativePath="..\Include\dynamic_annotations.h"
707+
>
708+
</File>
705709
<File
706710
RelativePath="..\Include\enumobject.h"
707711
>
@@ -1659,6 +1663,10 @@
16591663
RelativePath="..\Python\compile.c"
16601664
>
16611665
</File>
1666+
<File
1667+
RelativePath="..\Python\dynamic_annotations.c"
1668+
>
1669+
</File>
16621670
<File
16631671
RelativePath="..\Python\dynload_win.c"
16641672
>

0 commit comments

Comments
 (0)