From 60ec6efd96d95476fe5e38c491491add04f026e5 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 7 Oct 2019 22:31:42 +0200 Subject: [PATCH] bpo-36389: Fix _PyBytesWriter in release mode (GH-16624) Fix _PyBytesWriter API when Python is built in release mode with assertions. --- Include/internal/pycore_pymem.h | 15 +++++++++ Objects/bytesobject.c | 21 ++++++------ Objects/obmalloc.c | 59 +++++++++++++-------------------- 3 files changed, 48 insertions(+), 47 deletions(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 5be2d3352b1..d61ff6ee4b7 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -154,6 +154,21 @@ PyAPI_FUNC(int) _PyMem_SetDefaultAllocator( PyMemAllocatorDomain domain, PyMemAllocatorEx *old_alloc); +/* Special bytes broadcast into debug memory blocks at appropriate times. + Strings of these are unlikely to be valid addresses, floats, ints or + 7-bit ASCII. + + - PYMEM_CLEANBYTE: clean (newly allocated) memory + - PYMEM_DEADBYTE dead (newly freed) memory + - PYMEM_FORBIDDENBYTE: untouchable bytes at each end of a block + + Byte patterns 0xCB, 0xBB and 0xFB have been replaced with 0xCD, 0xDD and + 0xFD to use the same values than Windows CRT debug malloc() and free(). + If modified, _PyMem_IsPtrFreed() should be updated as well. */ +#define PYMEM_CLEANBYTE 0xCD +#define PYMEM_DEADBYTE 0xDD +#define PYMEM_FORBIDDENBYTE 0xFD + /* Heuristic checking if a pointer value is newly allocated (uninitialized), newly freed or NULL (is equal to zero). diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 4223dc9977e..f9823f18e86 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -667,9 +667,6 @@ _PyBytes_FormatEx(const char *format, Py_ssize_t format_len, Py_ssize_t len = 0; char onechar; /* For byte_converter() */ Py_ssize_t alloc; -#ifdef Py_DEBUG - char *before; -#endif fmt++; if (*fmt == '%') { @@ -981,8 +978,8 @@ _PyBytes_FormatEx(const char *format, Py_ssize_t format_len, if (res == NULL) goto error; } -#ifdef Py_DEBUG - before = res; +#ifndef NDEBUG + char *before = res; #endif /* Write the sign if needed */ @@ -1047,7 +1044,7 @@ _PyBytes_FormatEx(const char *format, Py_ssize_t format_len, } Py_XDECREF(temp); -#ifdef Py_DEBUG +#ifndef NDEBUG /* check that we computed the exact size for this write */ assert((res - before) == alloc); #endif @@ -3168,8 +3165,9 @@ _PyBytesWriter_Init(_PyBytesWriter *writer) { /* Set all attributes before small_buffer to 0 */ memset(writer, 0, offsetof(_PyBytesWriter, small_buffer)); -#ifdef Py_DEBUG - memset(writer->small_buffer, 0xCB, sizeof(writer->small_buffer)); +#ifndef NDEBUG + memset(writer->small_buffer, PYMEM_CLEANBYTE, + sizeof(writer->small_buffer)); #endif } @@ -3297,8 +3295,9 @@ _PyBytesWriter_Resize(_PyBytesWriter *writer, void *str, Py_ssize_t size) } writer->use_small_buffer = 0; -#ifdef Py_DEBUG - memset(writer->small_buffer, 0xDB, sizeof(writer->small_buffer)); +#ifndef NDEBUG + memset(writer->small_buffer, PYMEM_CLEANBYTE, + sizeof(writer->small_buffer)); #endif } writer->allocated = allocated; @@ -3350,7 +3349,7 @@ _PyBytesWriter_Alloc(_PyBytesWriter *writer, Py_ssize_t size) assert(size >= 0); writer->use_small_buffer = 1; -#ifdef Py_DEBUG +#ifndef NDEBUG writer->allocated = sizeof(writer->small_buffer) - 1; /* In debug mode, don't use the full small buffer because it is less efficient than bytes and bytearray objects to detect buffer underflow diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 40c098ddc30..50701dbd384 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -2031,20 +2031,6 @@ _Py_GetAllocatedBlocks(void) * it wraps a real allocator, adding extra debugging info to the memory blocks. */ -/* Special bytes broadcast into debug memory blocks at appropriate times. - * Strings of these are unlikely to be valid addresses, floats, ints or - * 7-bit ASCII. If modified, _PyMem_IsPtrFreed() should be updated as well. - * - * Byte patterns 0xCB, 0xBB and 0xFB have been replaced with 0xCD, 0xDD and - * 0xFD to use the same values than Windows CRT debug malloc() and free(). - */ -#undef CLEANBYTE -#undef DEADBYTE -#undef FORBIDDENBYTE -#define CLEANBYTE 0xCD /* clean (newly allocated) memory */ -#define DEADBYTE 0xDD /* dead (newly freed) memory */ -#define FORBIDDENBYTE 0xFD /* untouchable bytes at each end of a block */ - /* Uncomment this define to add the "serialno" field */ /* #define PYMEM_DEBUG_SERIALNO */ @@ -2106,14 +2092,14 @@ p[0: S] p[S] API ID. See PEP 445. This is a character, but seems undocumented. p[S+1: 2*S] - Copies of FORBIDDENBYTE. Used to catch under- writes and reads. + Copies of PYMEM_FORBIDDENBYTE. Used to catch under- writes and reads. p[2*S: 2*S+n] - The requested memory, filled with copies of CLEANBYTE. + The requested memory, filled with copies of PYMEM_CLEANBYTE. Used to catch reference to uninitialized memory. &p[2*S] is returned. Note that this is 8-byte aligned if pymalloc handled the request itself. p[2*S+n: 2*S+n+S] - Copies of FORBIDDENBYTE. Used to catch over- writes and reads. + Copies of PYMEM_FORBIDDENBYTE. Used to catch over- writes and reads. p[2*S+n+S: 2*S+n+2*S] A serial number, incremented by 1 on each call to _PyMem_DebugMalloc and _PyMem_DebugRealloc. @@ -2170,15 +2156,15 @@ _PyMem_DebugRawAlloc(int use_calloc, void *ctx, size_t nbytes) /* at p, write size (SST bytes), id (1 byte), pad (SST-1 bytes) */ write_size_t(p, nbytes); p[SST] = (uint8_t)api->api_id; - memset(p + SST + 1, FORBIDDENBYTE, SST-1); + memset(p + SST + 1, PYMEM_FORBIDDENBYTE, SST-1); if (nbytes > 0 && !use_calloc) { - memset(data, CLEANBYTE, nbytes); + memset(data, PYMEM_CLEANBYTE, nbytes); } /* at tail, write pad (SST bytes) and serialno (SST bytes) */ tail = data + nbytes; - memset(tail, FORBIDDENBYTE, SST); + memset(tail, PYMEM_FORBIDDENBYTE, SST); #ifdef PYMEM_DEBUG_SERIALNO write_size_t(tail + SST, serialno); #endif @@ -2204,7 +2190,7 @@ _PyMem_DebugRawCalloc(void *ctx, size_t nelem, size_t elsize) /* The debug free first checks the 2*SST bytes on each end for sanity (in particular, that the FORBIDDENBYTEs with the api ID are still intact). - Then fills the original bytes with DEADBYTE. + Then fills the original bytes with PYMEM_DEADBYTE. Then calls the underlying free. */ static void @@ -2222,7 +2208,7 @@ _PyMem_DebugRawFree(void *ctx, void *p) _PyMem_DebugCheckAddress(api->api_id, p); nbytes = read_size_t(q); nbytes += PYMEM_DEBUG_EXTRA_BYTES; - memset(q, DEADBYTE, nbytes); + memset(q, PYMEM_DEADBYTE, nbytes); api->alloc.free(api->alloc.ctx, q); } @@ -2264,14 +2250,14 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) */ if (original_nbytes <= sizeof(save)) { memcpy(save, data, original_nbytes); - memset(data - 2 * SST, DEADBYTE, + memset(data - 2 * SST, PYMEM_DEADBYTE, original_nbytes + PYMEM_DEBUG_EXTRA_BYTES); } else { memcpy(save, data, ERASED_SIZE); - memset(head, DEADBYTE, ERASED_SIZE + 2 * SST); + memset(head, PYMEM_DEADBYTE, ERASED_SIZE + 2 * SST); memcpy(&save[ERASED_SIZE], tail - ERASED_SIZE, ERASED_SIZE); - memset(tail - ERASED_SIZE, DEADBYTE, + memset(tail - ERASED_SIZE, PYMEM_DEADBYTE, ERASED_SIZE + PYMEM_DEBUG_EXTRA_BYTES - 2 * SST); } @@ -2293,10 +2279,10 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) write_size_t(head, nbytes); head[SST] = (uint8_t)api->api_id; - memset(head + SST + 1, FORBIDDENBYTE, SST-1); + memset(head + SST + 1, PYMEM_FORBIDDENBYTE, SST-1); tail = data + nbytes; - memset(tail, FORBIDDENBYTE, SST); + memset(tail, PYMEM_FORBIDDENBYTE, SST); #ifdef PYMEM_DEBUG_SERIALNO write_size_t(tail + SST, block_serialno); #endif @@ -2320,7 +2306,8 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) if (nbytes > original_nbytes) { /* growing: mark new extra memory clean */ - memset(data + original_nbytes, CLEANBYTE, nbytes - original_nbytes); + memset(data + original_nbytes, PYMEM_CLEANBYTE, + nbytes - original_nbytes); } return data; @@ -2399,7 +2386,7 @@ _PyMem_DebugCheckAddress(char api, const void *p) * the tail could lead to a segfault then. */ for (i = SST-1; i >= 1; --i) { - if (*(q-i) != FORBIDDENBYTE) { + if (*(q-i) != PYMEM_FORBIDDENBYTE) { msg = "bad leading pad byte"; goto error; } @@ -2408,7 +2395,7 @@ _PyMem_DebugCheckAddress(char api, const void *p) nbytes = read_size_t(q - 2*SST); tail = q + nbytes; for (i = 0; i < SST; ++i) { - if (tail[i] != FORBIDDENBYTE) { + if (tail[i] != PYMEM_FORBIDDENBYTE) { msg = "bad trailing pad byte"; goto error; } @@ -2448,7 +2435,7 @@ _PyObject_DebugDumpAddress(const void *p) fprintf(stderr, " The %d pad bytes at p-%d are ", SST-1, SST-1); ok = 1; for (i = 1; i <= SST-1; ++i) { - if (*(q-i) != FORBIDDENBYTE) { + if (*(q-i) != PYMEM_FORBIDDENBYTE) { ok = 0; break; } @@ -2457,11 +2444,11 @@ _PyObject_DebugDumpAddress(const void *p) fputs("FORBIDDENBYTE, as expected.\n", stderr); else { fprintf(stderr, "not all FORBIDDENBYTE (0x%02x):\n", - FORBIDDENBYTE); + PYMEM_FORBIDDENBYTE); for (i = SST-1; i >= 1; --i) { const uint8_t byte = *(q-i); fprintf(stderr, " at p-%d: 0x%02x", i, byte); - if (byte != FORBIDDENBYTE) + if (byte != PYMEM_FORBIDDENBYTE) fputs(" *** OUCH", stderr); fputc('\n', stderr); } @@ -2476,7 +2463,7 @@ _PyObject_DebugDumpAddress(const void *p) fprintf(stderr, " The %d pad bytes at tail=%p are ", SST, (void *)tail); ok = 1; for (i = 0; i < SST; ++i) { - if (tail[i] != FORBIDDENBYTE) { + if (tail[i] != PYMEM_FORBIDDENBYTE) { ok = 0; break; } @@ -2485,12 +2472,12 @@ _PyObject_DebugDumpAddress(const void *p) fputs("FORBIDDENBYTE, as expected.\n", stderr); else { fprintf(stderr, "not all FORBIDDENBYTE (0x%02x):\n", - FORBIDDENBYTE); + PYMEM_FORBIDDENBYTE); for (i = 0; i < SST; ++i) { const uint8_t byte = tail[i]; fprintf(stderr, " at tail+%d: 0x%02x", i, byte); - if (byte != FORBIDDENBYTE) + if (byte != PYMEM_FORBIDDENBYTE) fputs(" *** OUCH", stderr); fputc('\n', stderr); }