Skip to content

Commit 3924f93

Browse files
committed
improvements to code that checks whether Python (obmalloc) allocated an address
- Rename Py_ADDRESS_IN_RANGE to address_in_range and make it a static function instead of macro. Any compiler worth its salt will inline this function. - Remove the duplicated function version of Py_ADDRESS_IN_RANGE used when memory analysis was active. Instead, we can simply mark address_in_range as allergic to dynamic memory checking. We can now remove the __attribute__((no_address_safety_analysis)) from _PyObject_Free and _PyObject_Realloc. All the badness is contained in address_in_range now. - Fix the code that tried to only read pool->arenaindex once. Putting something in a variable is no guarantee that it won't be read multiple times. We must use volatile for that.
1 parent ac965ca commit 3924f93

File tree

1 file changed

+22
-76
lines changed

1 file changed

+22
-76
lines changed

Objects/obmalloc.c

Lines changed: 22 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "Python.h"
22

3+
#include <stdbool.h>
4+
35

46
/* Defined in tracemalloc.c */
57
extern void _PyMem_DumpTraceback(int fd, const void *ptr);
@@ -37,16 +39,14 @@ static void _PyMem_DebugCheckAddress(char api_id, const void *p);
3739
#if defined(__has_feature) /* Clang */
3840
#if __has_feature(address_sanitizer) /* is ASAN enabled? */
3941
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \
40-
__attribute__((no_address_safety_analysis)) \
41-
__attribute__ ((noinline))
42+
__attribute__((no_address_safety_analysis))
4243
#else
4344
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
4445
#endif
4546
#else
4647
#if defined(__SANITIZE_ADDRESS__) /* GCC 4.8.x, is ASAN enabled? */
4748
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \
48-
__attribute__((no_address_safety_analysis)) \
49-
__attribute__ ((noinline))
49+
__attribute__((no_address_safety_analysis))
5050
#else
5151
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
5252
#endif
@@ -1124,13 +1124,13 @@ new_arena(void)
11241124
}
11251125

11261126
/*
1127-
Py_ADDRESS_IN_RANGE(P, POOL)
1127+
address_in_range(P, POOL)
11281128
11291129
Return true if and only if P is an address that was allocated by pymalloc.
11301130
POOL must be the pool address associated with P, i.e., POOL = POOL_ADDR(P)
11311131
(the caller is asked to compute this because the macro expands POOL more than
11321132
once, and for efficiency it's best for the caller to assign POOL_ADDR(P) to a
1133-
variable and pass the latter to the macro; because Py_ADDRESS_IN_RANGE is
1133+
variable and pass the latter to the macro; because address_in_range is
11341134
called on every alloc/realloc/free, micro-efficiency is important here).
11351135
11361136
Tricky: Let B be the arena base address associated with the pool, B =
@@ -1155,7 +1155,7 @@ arenas[(POOL)->arenaindex]. Suppose obmalloc controls P. Then (barring wild
11551155
stores, etc), POOL is the correct address of P's pool, AO.address is the
11561156
correct base address of the pool's arena, and P must be within ARENA_SIZE of
11571157
AO.address. In addition, AO.address is not 0 (no arena can start at address 0
1158-
(NULL)). Therefore Py_ADDRESS_IN_RANGE correctly reports that obmalloc
1158+
(NULL)). Therefore address_in_range correctly reports that obmalloc
11591159
controls P.
11601160
11611161
Now suppose obmalloc does not control P (e.g., P was obtained via a direct
@@ -1196,51 +1196,21 @@ that this test determines whether an arbitrary address is controlled by
11961196
obmalloc in a small constant time, independent of the number of arenas
11971197
obmalloc controls. Since this test is needed at every entry point, it's
11981198
extremely desirable that it be this fast.
1199-
1200-
Since Py_ADDRESS_IN_RANGE may be reading from memory which was not allocated
1201-
by Python, it is important that (POOL)->arenaindex is read only once, as
1202-
another thread may be concurrently modifying the value without holding the
1203-
GIL. To accomplish this, the arenaindex_temp variable is used to store
1204-
(POOL)->arenaindex for the duration of the Py_ADDRESS_IN_RANGE macro's
1205-
execution. The caller of the macro is responsible for declaring this
1206-
variable.
12071199
*/
1208-
#define Py_ADDRESS_IN_RANGE(P, POOL) \
1209-
((arenaindex_temp = (POOL)->arenaindex) < maxarenas && \
1210-
(uptr)(P) - arenas[arenaindex_temp].address < (uptr)ARENA_SIZE && \
1211-
arenas[arenaindex_temp].address != 0)
1212-
1213-
1214-
/* This is only useful when running memory debuggers such as
1215-
* Purify or Valgrind. Uncomment to use.
1216-
*
1217-
#define Py_USING_MEMORY_DEBUGGER
1218-
*/
1219-
1220-
#ifdef Py_USING_MEMORY_DEBUGGER
1221-
1222-
/* Py_ADDRESS_IN_RANGE may access uninitialized memory by design
1223-
* This leads to thousands of spurious warnings when using
1224-
* Purify or Valgrind. By making a function, we can easily
1225-
* suppress the uninitialized memory reads in this one function.
1226-
* So we won't ignore real errors elsewhere.
1227-
*
1228-
* Disable the macro and use a function.
1229-
*/
1230-
1231-
#undef Py_ADDRESS_IN_RANGE
1232-
1233-
#if defined(__GNUC__) && ((__GNUC__ == 3) && (__GNUC_MINOR__ >= 1) || \
1234-
(__GNUC__ >= 4))
1235-
#define Py_NO_INLINE __attribute__((__noinline__))
1236-
#else
1237-
#define Py_NO_INLINE
1238-
#endif
12391200

1240-
/* Don't make static, to try to ensure this isn't inlined. */
1241-
int Py_ADDRESS_IN_RANGE(void *P, poolp pool) Py_NO_INLINE;
1242-
#undef Py_NO_INLINE
1243-
#endif
1201+
static bool ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
1202+
address_in_range(void *p, poolp pool)
1203+
{
1204+
// Since address_in_range may be reading from memory which was not allocated
1205+
// by Python, it is important that pool->arenaindex is read only once, as
1206+
// another thread may be concurrently modifying the value without holding
1207+
// the GIL. The following dance forces the compiler to read pool->arenaindex
1208+
// only once.
1209+
uint arenaindex = *((volatile uint *)&pool->arenaindex);
1210+
return arenaindex < maxarenas &&
1211+
(uptr)p - arenas[arenaindex].address < ARENA_SIZE &&
1212+
arenas[arenaindex].address != 0;
1213+
}
12441214

12451215
/*==========================================================================*/
12461216

@@ -1485,17 +1455,13 @@ _PyObject_Calloc(void *ctx, size_t nelem, size_t elsize)
14851455

14861456
/* free */
14871457

1488-
ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
14891458
static void
14901459
_PyObject_Free(void *ctx, void *p)
14911460
{
14921461
poolp pool;
14931462
block *lastfree;
14941463
poolp next, prev;
14951464
uint size;
1496-
#ifndef Py_USING_MEMORY_DEBUGGER
1497-
uint arenaindex_temp;
1498-
#endif
14991465

15001466
if (p == NULL) /* free(NULL) has no effect */
15011467
return;
@@ -1508,7 +1474,7 @@ _PyObject_Free(void *ctx, void *p)
15081474
#endif
15091475

15101476
pool = POOL_ADDR(p);
1511-
if (Py_ADDRESS_IN_RANGE(p, pool)) {
1477+
if (address_in_range(p, pool)) {
15121478
/* We allocated this address. */
15131479
LOCK();
15141480
/* Link p to the start of the pool's freeblock list. Since
@@ -1714,16 +1680,12 @@ _PyObject_Free(void *ctx, void *p)
17141680
* return a non-NULL result.
17151681
*/
17161682

1717-
ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
17181683
static void *
17191684
_PyObject_Realloc(void *ctx, void *p, size_t nbytes)
17201685
{
17211686
void *bp;
17221687
poolp pool;
17231688
size_t size;
1724-
#ifndef Py_USING_MEMORY_DEBUGGER
1725-
uint arenaindex_temp;
1726-
#endif
17271689

17281690
if (p == NULL)
17291691
return _PyObject_Alloc(0, ctx, 1, nbytes);
@@ -1735,7 +1697,7 @@ _PyObject_Realloc(void *ctx, void *p, size_t nbytes)
17351697
#endif
17361698

17371699
pool = POOL_ADDR(p);
1738-
if (Py_ADDRESS_IN_RANGE(p, pool)) {
1700+
if (address_in_range(p, pool)) {
17391701
/* We're in charge of this block */
17401702
size = INDEX2SIZE(pool->szidx);
17411703
if (nbytes <= size) {
@@ -2432,19 +2394,3 @@ _PyObject_DebugMallocStats(FILE *out)
24322394
}
24332395

24342396
#endif /* #ifdef WITH_PYMALLOC */
2435-
2436-
2437-
#ifdef Py_USING_MEMORY_DEBUGGER
2438-
/* Make this function last so gcc won't inline it since the definition is
2439-
* after the reference.
2440-
*/
2441-
int
2442-
Py_ADDRESS_IN_RANGE(void *P, poolp pool)
2443-
{
2444-
uint arenaindex_temp = pool->arenaindex;
2445-
2446-
return arenaindex_temp < maxarenas &&
2447-
(uptr)P - arenas[arenaindex_temp].address < (uptr)ARENA_SIZE &&
2448-
arenas[arenaindex_temp].address != 0;
2449-
}
2450-
#endif

0 commit comments

Comments
 (0)