Skip to content

Commit 0bb51fd

Browse files
colesburyLukasWoodtli
authored andcommitted
pythongh-115491: Keep some fields valid across allocations (free-threading) (python#115573)
This avoids filling the memory occupied by ob_tid, ob_ref_local, and ob_ref_shared with debug bytes (e.g., 0xDD) in mimalloc in the free-threaded build.
1 parent b7abd7a commit 0bb51fd

File tree

5 files changed

+33
-25
lines changed

5 files changed

+33
-25
lines changed

Include/internal/mimalloc/mimalloc/types.h

+2
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@ typedef struct mi_page_s {
312312
uint8_t is_committed : 1; // `true` if the page virtual memory is committed
313313
uint8_t is_zero_init : 1; // `true` if the page was initially zero initialized
314314
uint8_t tag : 4; // tag from the owning heap
315+
uint8_t debug_offset; // number of bytes to preserve when filling freed or uninitialized memory
315316

316317
// layout like this to optimize access in `mi_malloc` and `mi_free`
317318
uint16_t capacity; // number of blocks committed, must be the first field, see `segment.c:page_clear`
@@ -553,6 +554,7 @@ struct mi_heap_s {
553554
mi_heap_t* next; // list of heaps per thread
554555
bool no_reclaim; // `true` if this heap should not reclaim abandoned pages
555556
uint8_t tag; // custom identifier for this heap
557+
uint8_t debug_offset; // number of bytes to preserve when filling freed or uninitialized memory
556558
};
557559

558560

Objects/mimalloc/alloc.c

+13-4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ terms of the MIT license. A copy of the license can be found in the file
2626
// Allocation
2727
// ------------------------------------------------------
2828

29+
#if (MI_DEBUG>0)
30+
static void mi_debug_fill(mi_page_t* page, mi_block_t* block, int c, size_t size) {
31+
size_t offset = (size_t)page->debug_offset;
32+
if (offset < size) {
33+
memset((char*)block + offset, c, size - offset);
34+
}
35+
}
36+
#endif
37+
2938
// Fast allocation in a page: just pop from the free list.
3039
// Fall back to generic allocation only if the list is empty.
3140
extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t size, bool zero) mi_attr_noexcept {
@@ -65,7 +74,7 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz
6574

6675
#if (MI_DEBUG>0) && !MI_TRACK_ENABLED && !MI_TSAN
6776
if (!zero && !mi_page_is_huge(page)) {
68-
memset(block, MI_DEBUG_UNINIT, mi_page_usable_block_size(page));
77+
mi_debug_fill(page, block, MI_DEBUG_UNINIT, mi_page_usable_block_size(page));
6978
}
7079
#elif (MI_SECURE!=0)
7180
if (!zero) { block->next = 0; } // don't leak internal data
@@ -426,7 +435,7 @@ static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* bloc
426435

427436
#if (MI_DEBUG>0) && !MI_TRACK_ENABLED && !MI_TSAN // note: when tracking, cannot use mi_usable_size with multi-threading
428437
if (segment->kind != MI_SEGMENT_HUGE) { // not for huge segments as we just reset the content
429-
memset(block, MI_DEBUG_FREED, mi_usable_size(block));
438+
mi_debug_fill(page, block, MI_DEBUG_FREED, mi_usable_size(block));
430439
}
431440
#endif
432441

@@ -480,7 +489,7 @@ static inline void _mi_free_block(mi_page_t* page, bool local, mi_block_t* block
480489
mi_check_padding(page, block);
481490
#if (MI_DEBUG>0) && !MI_TRACK_ENABLED && !MI_TSAN
482491
if (!mi_page_is_huge(page)) { // huge page content may be already decommitted
483-
memset(block, MI_DEBUG_FREED, mi_page_block_size(page));
492+
mi_debug_fill(page, block, MI_DEBUG_FREED, mi_page_block_size(page));
484493
}
485494
#endif
486495
mi_block_set_next(page, block, page->local_free);
@@ -575,7 +584,7 @@ void mi_free(void* p) mi_attr_noexcept
575584
mi_check_padding(page, block);
576585
mi_stat_free(page, block);
577586
#if (MI_DEBUG>0) && !MI_TRACK_ENABLED && !MI_TSAN
578-
memset(block, MI_DEBUG_FREED, mi_page_block_size(page));
587+
mi_debug_fill(page, block, MI_DEBUG_FREED, mi_page_block_size(page));
579588
#endif
580589
mi_track_free_size(p, mi_page_usable_size_of(page,block)); // faster then mi_usable_size as we already know the page and that p is unaligned
581590
mi_block_set_next(page, block, page->local_free);

Objects/mimalloc/init.c

+2-21
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,7 @@ terms of the MIT license. A copy of the license can be found in the file
1313

1414

1515
// Empty page used to initialize the small free pages array
16-
const mi_page_t _mi_page_empty = {
17-
0, false, false, false, 0,
18-
0, // capacity
19-
0, // reserved capacity
20-
{ 0 }, // flags
21-
false, // is_zero
22-
0, // retire_expire
23-
NULL, // free
24-
0, // used
25-
0, // xblock_size
26-
NULL, // local_free
27-
#if (MI_PADDING || MI_ENCODE_FREELIST)
28-
{ 0, 0 },
29-
#endif
30-
MI_ATOMIC_VAR_INIT(0), // xthread_free
31-
MI_ATOMIC_VAR_INIT(0), // xheap
32-
NULL, NULL
33-
#if MI_INTPTR_SIZE==8
34-
, { 0 } // padding
35-
#endif
36-
};
16+
const mi_page_t _mi_page_empty;
3717

3818
#define MI_PAGE_EMPTY() ((mi_page_t*)&_mi_page_empty)
3919

@@ -122,6 +102,7 @@ mi_decl_cache_align const mi_heap_t _mi_heap_empty = {
122102
MI_BIN_FULL, 0, // page retired min/max
123103
NULL, // next
124104
false,
105+
0,
125106
0
126107
};
127108

Objects/mimalloc/page.c

+1
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,7 @@ static void mi_page_init(mi_heap_t* heap, mi_page_t* page, size_t block_size, mi
661661
// set fields
662662
mi_page_set_heap(page, heap);
663663
page->tag = heap->tag;
664+
page->debug_offset = heap->debug_offset;
664665
page->xblock_size = (block_size < MI_HUGE_BLOCK_SIZE ? (uint32_t)block_size : MI_HUGE_BLOCK_SIZE); // initialize before _mi_segment_page_start
665666
size_t page_size;
666667
const void* page_start = _mi_segment_page_start(segment, page, &page_size);

Python/pystate.c

+15
Original file line numberDiff line numberDiff line change
@@ -2845,9 +2845,24 @@ tstate_mimalloc_bind(PyThreadState *tstate)
28452845
// pools to keep Python objects from different interpreters separate.
28462846
tld->segments.abandoned = &tstate->interp->mimalloc.abandoned_pool;
28472847

2848+
// Don't fill in the first N bytes up to ob_type in debug builds. We may
2849+
// access ob_tid and the refcount fields in the dict and list lock-less
2850+
// accesses, so they must remain valid for a while after deallocation.
2851+
size_t base_offset = offsetof(PyObject, ob_type);
2852+
if (_PyMem_DebugEnabled()) {
2853+
// The debug allocator adds two words at the beginning of each block.
2854+
base_offset += 2 * sizeof(size_t);
2855+
}
2856+
size_t debug_offsets[_Py_MIMALLOC_HEAP_COUNT] = {
2857+
[_Py_MIMALLOC_HEAP_OBJECT] = base_offset,
2858+
[_Py_MIMALLOC_HEAP_GC] = base_offset,
2859+
[_Py_MIMALLOC_HEAP_GC_PRE] = base_offset + 2 * sizeof(PyObject *),
2860+
};
2861+
28482862
// Initialize each heap
28492863
for (uint8_t i = 0; i < _Py_MIMALLOC_HEAP_COUNT; i++) {
28502864
_mi_heap_init_ex(&mts->heaps[i], tld, _mi_arena_id_none(), false, i);
2865+
mts->heaps[i].debug_offset = (uint8_t)debug_offsets[i];
28512866
}
28522867

28532868
// By default, object allocations use _Py_MIMALLOC_HEAP_OBJECT.

0 commit comments

Comments
 (0)