Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-115491: Keep some fields valid across allocations (free-threading) #115573

Merged
merged 2 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Include/internal/mimalloc/mimalloc/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ typedef struct mi_page_s {
uint8_t is_committed : 1; // `true` if the page virtual memory is committed
uint8_t is_zero_init : 1; // `true` if the page was initially zero initialized
uint8_t tag : 4; // tag from the owning heap
uint8_t debug_offset; // number of bytes to preserve when filling freed or uninitialized memory

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


Expand Down
17 changes: 13 additions & 4 deletions Objects/mimalloc/alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ terms of the MIT license. A copy of the license can be found in the file
// Allocation
// ------------------------------------------------------

#if (MI_DEBUG>0)
static void mi_debug_fill(mi_page_t* page, mi_block_t* block, int c, size_t size) {
size_t offset = (size_t)page->debug_offset;
if (offset < size) {
memset((char*)block + offset, c, size - offset);
}
}
#endif

// Fast allocation in a page: just pop from the free list.
// Fall back to generic allocation only if the list is empty.
extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t size, bool zero) mi_attr_noexcept {
Expand Down Expand Up @@ -65,7 +74,7 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz

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

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

Expand Down Expand Up @@ -480,7 +489,7 @@ static inline void _mi_free_block(mi_page_t* page, bool local, mi_block_t* block
mi_check_padding(page, block);
#if (MI_DEBUG>0) && !MI_TRACK_ENABLED && !MI_TSAN
if (!mi_page_is_huge(page)) { // huge page content may be already decommitted
memset(block, MI_DEBUG_FREED, mi_page_block_size(page));
mi_debug_fill(page, block, MI_DEBUG_FREED, mi_page_block_size(page));
}
#endif
mi_block_set_next(page, block, page->local_free);
Expand Down Expand Up @@ -575,7 +584,7 @@ void mi_free(void* p) mi_attr_noexcept
mi_check_padding(page, block);
mi_stat_free(page, block);
#if (MI_DEBUG>0) && !MI_TRACK_ENABLED && !MI_TSAN
memset(block, MI_DEBUG_FREED, mi_page_block_size(page));
mi_debug_fill(page, block, MI_DEBUG_FREED, mi_page_block_size(page));
#endif
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
mi_block_set_next(page, block, page->local_free);
Expand Down
23 changes: 2 additions & 21 deletions Objects/mimalloc/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,7 @@ terms of the MIT license. A copy of the license can be found in the file


// Empty page used to initialize the small free pages array
const mi_page_t _mi_page_empty = {
0, false, false, false, 0,
0, // capacity
0, // reserved capacity
{ 0 }, // flags
false, // is_zero
0, // retire_expire
NULL, // free
0, // used
0, // xblock_size
NULL, // local_free
#if (MI_PADDING || MI_ENCODE_FREELIST)
{ 0, 0 },
#endif
MI_ATOMIC_VAR_INIT(0), // xthread_free
MI_ATOMIC_VAR_INIT(0), // xheap
NULL, NULL
#if MI_INTPTR_SIZE==8
, { 0 } // padding
#endif
};
const mi_page_t _mi_page_empty;

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

Expand Down Expand Up @@ -122,6 +102,7 @@ mi_decl_cache_align const mi_heap_t _mi_heap_empty = {
MI_BIN_FULL, 0, // page retired min/max
NULL, // next
false,
0,
0
};

Expand Down
1 change: 1 addition & 0 deletions Objects/mimalloc/page.c
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@ static void mi_page_init(mi_heap_t* heap, mi_page_t* page, size_t block_size, mi
// set fields
mi_page_set_heap(page, heap);
page->tag = heap->tag;
page->debug_offset = heap->debug_offset;
page->xblock_size = (block_size < MI_HUGE_BLOCK_SIZE ? (uint32_t)block_size : MI_HUGE_BLOCK_SIZE); // initialize before _mi_segment_page_start
size_t page_size;
const void* page_start = _mi_segment_page_start(segment, page, &page_size);
Expand Down
15 changes: 15 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -2845,9 +2845,24 @@
// pools to keep Python objects from different interpreters separate.
tld->segments.abandoned = &tstate->interp->mimalloc.abandoned_pool;

// Don't fill in the first N bytes up to ob_type in debug builds. We may
// access ob_tid and the refcount fields in the dict and list lock-less
// accesses, so they must remain valid for a while after deallocation.
size_t base_offset = offsetof(PyObject, ob_type);
if (_PyMem_DebugEnabled()) {
// The debug allocator adds two words at the beginning of each block.
base_offset += 2 * sizeof(size_t);
}
size_t debug_offsets[_Py_MIMALLOC_HEAP_COUNT] = {
[_Py_MIMALLOC_HEAP_OBJECT] = base_offset,
[_Py_MIMALLOC_HEAP_GC] = base_offset,
[_Py_MIMALLOC_HEAP_GC_PRE] = base_offset + 2 * sizeof(PyObject *),
};

// Initialize each heap
for (uint8_t i = 0; i < _Py_MIMALLOC_HEAP_COUNT; i++) {
_mi_heap_init_ex(&mts->heaps[i], tld, _mi_arena_id_none(), false, i);
mts->heaps[i].debug_offset = debug_offsets[i];

Check warning on line 2865 in Python/pystate.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build and test (x64)

'=': conversion from 'size_t' to 'uint8_t', possible loss of data [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 2865 in Python/pystate.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build and test (x64)

'=': conversion from 'size_t' to 'uint8_t', possible loss of data [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Check warning on line 2865 in Python/pystate.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build (arm64)

'=': conversion from 'size_t' to 'uint8_t', possible loss of data [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check warning on line 2865 in Python/pystate.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build (arm64)

'=': conversion from 'size_t' to 'uint8_t', possible loss of data [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]
}

// By default, object allocations use _Py_MIMALLOC_HEAP_OBJECT.
Expand Down
Loading