Skip to content
New issue

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

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

Already on GitHub? # to your account

adding check for ptr cookie to be the same as segment cookie to catch… #687

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robertblaketaylor
Copy link

mi_is_in_heap_region returns false for pointers allocated by mimalloc who were allocated along the overallocate path, see osc.c (782-794).

    // overallocate...
    p = mi_os_mem_alloc(over_size, 1, commit, false, is_large, stats);
    if (p == NULL) return NULL;
    // and selectively unmap parts around the over-allocated area. (noop on sbrk)
    void* aligned_p = mi_align_up_ptr(p, alignment);
    size_t pre_size = (uint8_t*)aligned_p - (uint8_t*)p;
    size_t mid_size = _mi_align_up(size, _mi_os_page_size());
    size_t post_size = over_size - pre_size - mid_size;
    mi_assert_internal(pre_size < over_size && post_size < over_size && mid_size >= size);
    if (pre_size > 0)  mi_os_mem_free(p, pre_size, commit, stats);
    if (post_size > 0) mi_os_mem_free((uint8_t*)aligned_p + mid_size, post_size, commit, stats);
    // we can return the aligned pointer on `mmap` (and sbrk) systems
    p = aligned_p;

… valid pointers when unaligned os memory is provided at allocation
@robertblaketaylor
Copy link
Author

Ya this is still not quite right. Will look for some feedback from the authors on how to back-track a pointer allocated on the path where the memory wasn't aligned.

@daanx
Copy link
Collaborator

daanx commented Mar 29, 2023

Ah I see. I will look into this later

@robertblaketaylor
Copy link
Author

Ah I see. I will look into this later

Any thoughts on when you may be able to resolve this one?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants