mirror of https://github.com/python/cpython
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.
This commit is contained in:
Benjamin Peterson
parent
ac965ca16c
commit
3924f93794
|
|
@ -1,5 +1,7 @@
|
|||
#include "Python.h"
|
||||
|
||||
#include <stdbool.h>
|
||||
|
||||
|
||||
/* Defined in tracemalloc.c */
|
||||
extern void _PyMem_DumpTraceback(int fd, const void *ptr);
|
||||
|
|
@ -37,16 +39,14 @@ static void _PyMem_DebugCheckAddress(char api_id, const void *p);
|
|||
#if defined(__has_feature) /* Clang */
|
||||
#if __has_feature(address_sanitizer) /* is ASAN enabled? */
|
||||
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \
|
||||
__attribute__((no_address_safety_analysis)) \
|
||||
__attribute__ ((noinline))
|
||||
__attribute__((no_address_safety_analysis))
|
||||
#else
|
||||
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
|
||||
#endif
|
||||
#else
|
||||
#if defined(__SANITIZE_ADDRESS__) /* GCC 4.8.x, is ASAN enabled? */
|
||||
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \
|
||||
__attribute__((no_address_safety_analysis)) \
|
||||
__attribute__ ((noinline))
|
||||
__attribute__((no_address_safety_analysis))
|
||||
#else
|
||||
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
|
||||
#endif
|
||||
|
|
@ -1124,13 +1124,13 @@ new_arena(void)
|
|||
}
|
||||
|
||||
/*
|
||||
Py_ADDRESS_IN_RANGE(P, POOL)
|
||||
address_in_range(P, POOL)
|
||||
|
||||
Return true if and only if P is an address that was allocated by pymalloc.
|
||||
POOL must be the pool address associated with P, i.e., POOL = POOL_ADDR(P)
|
||||
(the caller is asked to compute this because the macro expands POOL more than
|
||||
once, and for efficiency it's best for the caller to assign POOL_ADDR(P) to a
|
||||
variable and pass the latter to the macro; because Py_ADDRESS_IN_RANGE is
|
||||
variable and pass the latter to the macro; because address_in_range is
|
||||
called on every alloc/realloc/free, micro-efficiency is important here).
|
||||
|
||||
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
|
|||
stores, etc), POOL is the correct address of P's pool, AO.address is the
|
||||
correct base address of the pool's arena, and P must be within ARENA_SIZE of
|
||||
AO.address. In addition, AO.address is not 0 (no arena can start at address 0
|
||||
(NULL)). Therefore Py_ADDRESS_IN_RANGE correctly reports that obmalloc
|
||||
(NULL)). Therefore address_in_range correctly reports that obmalloc
|
||||
controls P.
|
||||
|
||||
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
|
|||
obmalloc in a small constant time, independent of the number of arenas
|
||||
obmalloc controls. Since this test is needed at every entry point, it's
|
||||
extremely desirable that it be this fast.
|
||||
|
||||
Since Py_ADDRESS_IN_RANGE may be reading from memory which was not allocated
|
||||
by Python, it is important that (POOL)->arenaindex is read only once, as
|
||||
another thread may be concurrently modifying the value without holding the
|
||||
GIL. To accomplish this, the arenaindex_temp variable is used to store
|
||||
(POOL)->arenaindex for the duration of the Py_ADDRESS_IN_RANGE macro's
|
||||
execution. The caller of the macro is responsible for declaring this
|
||||
variable.
|
||||
*/
|
||||
#define Py_ADDRESS_IN_RANGE(P, POOL) \
|
||||
((arenaindex_temp = (POOL)->arenaindex) < maxarenas && \
|
||||
(uptr)(P) - arenas[arenaindex_temp].address < (uptr)ARENA_SIZE && \
|
||||
arenas[arenaindex_temp].address != 0)
|
||||
|
||||
|
||||
/* This is only useful when running memory debuggers such as
|
||||
* Purify or Valgrind. Uncomment to use.
|
||||
*
|
||||
#define Py_USING_MEMORY_DEBUGGER
|
||||
*/
|
||||
|
||||
#ifdef Py_USING_MEMORY_DEBUGGER
|
||||
|
||||
/* Py_ADDRESS_IN_RANGE may access uninitialized memory by design
|
||||
* This leads to thousands of spurious warnings when using
|
||||
* Purify or Valgrind. By making a function, we can easily
|
||||
* suppress the uninitialized memory reads in this one function.
|
||||
* So we won't ignore real errors elsewhere.
|
||||
*
|
||||
* Disable the macro and use a function.
|
||||
*/
|
||||
|
||||
#undef Py_ADDRESS_IN_RANGE
|
||||
|
||||
#if defined(__GNUC__) && ((__GNUC__ == 3) && (__GNUC_MINOR__ >= 1) || \
|
||||
(__GNUC__ >= 4))
|
||||
#define Py_NO_INLINE __attribute__((__noinline__))
|
||||
#else
|
||||
#define Py_NO_INLINE
|
||||
#endif
|
||||
|
||||
/* Don't make static, to try to ensure this isn't inlined. */
|
||||
int Py_ADDRESS_IN_RANGE(void *P, poolp pool) Py_NO_INLINE;
|
||||
#undef Py_NO_INLINE
|
||||
#endif
|
||||
static bool ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
|
||||
address_in_range(void *p, poolp pool)
|
||||
{
|
||||
// Since address_in_range may be reading from memory which was not allocated
|
||||
// by Python, it is important that pool->arenaindex is read only once, as
|
||||
// another thread may be concurrently modifying the value without holding
|
||||
// the GIL. The following dance forces the compiler to read pool->arenaindex
|
||||
// only once.
|
||||
uint arenaindex = *((volatile uint *)&pool->arenaindex);
|
||||
return arenaindex < maxarenas &&
|
||||
(uptr)p - arenas[arenaindex].address < ARENA_SIZE &&
|
||||
arenas[arenaindex].address != 0;
|
||||
}
|
||||
|
||||
/*==========================================================================*/
|
||||
|
||||
|
|
@ -1485,7 +1455,6 @@ _PyObject_Calloc(void *ctx, size_t nelem, size_t elsize)
|
|||
|
||||
/* free */
|
||||
|
||||
ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
|
||||
static void
|
||||
_PyObject_Free(void *ctx, void *p)
|
||||
{
|
||||
|
|
@ -1493,9 +1462,6 @@ _PyObject_Free(void *ctx, void *p)
|
|||
block *lastfree;
|
||||
poolp next, prev;
|
||||
uint size;
|
||||
#ifndef Py_USING_MEMORY_DEBUGGER
|
||||
uint arenaindex_temp;
|
||||
#endif
|
||||
|
||||
if (p == NULL) /* free(NULL) has no effect */
|
||||
return;
|
||||
|
|
@ -1508,7 +1474,7 @@ _PyObject_Free(void *ctx, void *p)
|
|||
#endif
|
||||
|
||||
pool = POOL_ADDR(p);
|
||||
if (Py_ADDRESS_IN_RANGE(p, pool)) {
|
||||
if (address_in_range(p, pool)) {
|
||||
/* We allocated this address. */
|
||||
LOCK();
|
||||
/* Link p to the start of the pool's freeblock list. Since
|
||||
|
|
@ -1714,16 +1680,12 @@ redirect:
|
|||
* return a non-NULL result.
|
||||
*/
|
||||
|
||||
ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
|
||||
static void *
|
||||
_PyObject_Realloc(void *ctx, void *p, size_t nbytes)
|
||||
{
|
||||
void *bp;
|
||||
poolp pool;
|
||||
size_t size;
|
||||
#ifndef Py_USING_MEMORY_DEBUGGER
|
||||
uint arenaindex_temp;
|
||||
#endif
|
||||
|
||||
if (p == NULL)
|
||||
return _PyObject_Alloc(0, ctx, 1, nbytes);
|
||||
|
|
@ -1735,7 +1697,7 @@ _PyObject_Realloc(void *ctx, void *p, size_t nbytes)
|
|||
#endif
|
||||
|
||||
pool = POOL_ADDR(p);
|
||||
if (Py_ADDRESS_IN_RANGE(p, pool)) {
|
||||
if (address_in_range(p, pool)) {
|
||||
/* We're in charge of this block */
|
||||
size = INDEX2SIZE(pool->szidx);
|
||||
if (nbytes <= size) {
|
||||
|
|
@ -2432,19 +2394,3 @@ _PyObject_DebugMallocStats(FILE *out)
|
|||
}
|
||||
|
||||
#endif /* #ifdef WITH_PYMALLOC */
|
||||
|
||||
|
||||
#ifdef Py_USING_MEMORY_DEBUGGER
|
||||
/* Make this function last so gcc won't inline it since the definition is
|
||||
* after the reference.
|
||||
*/
|
||||
int
|
||||
Py_ADDRESS_IN_RANGE(void *P, poolp pool)
|
||||
{
|
||||
uint arenaindex_temp = pool->arenaindex;
|
||||
|
||||
return arenaindex_temp < maxarenas &&
|
||||
(uptr)P - arenas[arenaindex_temp].address < (uptr)ARENA_SIZE &&
|
||||
arenas[arenaindex_temp].address != 0;
|
||||
}
|
||||
#endif
|
||||
|
|
|
|||
Loading…
Reference in New Issue