Commit b030748a authored by Benjamin Peterson's avatar Benjamin Peterson

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.
parent 2262e5df
#include "Python.h" #include "Python.h"
#include <stdbool.h>
/* Defined in tracemalloc.c */ /* Defined in tracemalloc.c */
extern void _PyMem_DumpTraceback(int fd, const void *ptr); extern void _PyMem_DumpTraceback(int fd, const void *ptr);
...@@ -37,16 +39,14 @@ static void _PyMem_DebugCheckAddress(char api_id, const void *p); ...@@ -37,16 +39,14 @@ static void _PyMem_DebugCheckAddress(char api_id, const void *p);
#if defined(__has_feature) /* Clang */ #if defined(__has_feature) /* Clang */
#if __has_feature(address_sanitizer) /* is ASAN enabled? */ #if __has_feature(address_sanitizer) /* is ASAN enabled? */
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \ #define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \
__attribute__((no_address_safety_analysis)) \ __attribute__((no_address_safety_analysis))
__attribute__ ((noinline))
#else #else
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS #define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
#endif #endif
#else #else
#if defined(__SANITIZE_ADDRESS__) /* GCC 4.8.x, is ASAN enabled? */ #if defined(__SANITIZE_ADDRESS__) /* GCC 4.8.x, is ASAN enabled? */
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \ #define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS \
__attribute__((no_address_safety_analysis)) \ __attribute__((no_address_safety_analysis))
__attribute__ ((noinline))
#else #else
#define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS #define ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
#endif #endif
...@@ -1124,13 +1124,13 @@ new_arena(void) ...@@ -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. 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) 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 (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 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). called on every alloc/realloc/free, micro-efficiency is important here).
Tricky: Let B be the arena base address associated with the pool, B = 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 ...@@ -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 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 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 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. controls P.
Now suppose obmalloc does not control P (e.g., P was obtained via a direct 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 ...@@ -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 in a small constant time, independent of the number of arenas
obmalloc controls. Since this test is needed at every entry point, it's obmalloc controls. Since this test is needed at every entry point, it's
extremely desirable that it be this fast. 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. */ static bool ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
int Py_ADDRESS_IN_RANGE(void *P, poolp pool) Py_NO_INLINE; address_in_range(void *p, poolp pool)
#undef Py_NO_INLINE {
#endif // 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) ...@@ -1485,7 +1455,6 @@ _PyObject_Calloc(void *ctx, size_t nelem, size_t elsize)
/* free */ /* free */
ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
static void static void
_PyObject_Free(void *ctx, void *p) _PyObject_Free(void *ctx, void *p)
{ {
...@@ -1493,9 +1462,6 @@ _PyObject_Free(void *ctx, void *p) ...@@ -1493,9 +1462,6 @@ _PyObject_Free(void *ctx, void *p)
block *lastfree; block *lastfree;
poolp next, prev; poolp next, prev;
uint size; uint size;
#ifndef Py_USING_MEMORY_DEBUGGER
uint arenaindex_temp;
#endif
if (p == NULL) /* free(NULL) has no effect */ if (p == NULL) /* free(NULL) has no effect */
return; return;
...@@ -1508,7 +1474,7 @@ _PyObject_Free(void *ctx, void *p) ...@@ -1508,7 +1474,7 @@ _PyObject_Free(void *ctx, void *p)
#endif #endif
pool = POOL_ADDR(p); pool = POOL_ADDR(p);
if (Py_ADDRESS_IN_RANGE(p, pool)) { if (address_in_range(p, pool)) {
/* We allocated this address. */ /* We allocated this address. */
LOCK(); LOCK();
/* Link p to the start of the pool's freeblock list. Since /* Link p to the start of the pool's freeblock list. Since
...@@ -1714,16 +1680,12 @@ redirect: ...@@ -1714,16 +1680,12 @@ redirect:
* return a non-NULL result. * return a non-NULL result.
*/ */
ATTRIBUTE_NO_ADDRESS_SAFETY_ANALYSIS
static void * static void *
_PyObject_Realloc(void *ctx, void *p, size_t nbytes) _PyObject_Realloc(void *ctx, void *p, size_t nbytes)
{ {
void *bp; void *bp;
poolp pool; poolp pool;
size_t size; size_t size;
#ifndef Py_USING_MEMORY_DEBUGGER
uint arenaindex_temp;
#endif
if (p == NULL) if (p == NULL)
return _PyObject_Alloc(0, ctx, 1, nbytes); return _PyObject_Alloc(0, ctx, 1, nbytes);
...@@ -1735,7 +1697,7 @@ _PyObject_Realloc(void *ctx, void *p, size_t nbytes) ...@@ -1735,7 +1697,7 @@ _PyObject_Realloc(void *ctx, void *p, size_t nbytes)
#endif #endif
pool = POOL_ADDR(p); pool = POOL_ADDR(p);
if (Py_ADDRESS_IN_RANGE(p, pool)) { if (address_in_range(p, pool)) {
/* We're in charge of this block */ /* We're in charge of this block */
size = INDEX2SIZE(pool->szidx); size = INDEX2SIZE(pool->szidx);
if (nbytes <= size) { if (nbytes <= size) {
...@@ -2432,19 +2394,3 @@ _PyObject_DebugMallocStats(FILE *out) ...@@ -2432,19 +2394,3 @@ _PyObject_DebugMallocStats(FILE *out)
} }
#endif /* #ifdef WITH_PYMALLOC */ #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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment