Skip to content

improve MacOS interposes to make mimalloc compile for older MacOS #1028

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.

Sign up for GitHub

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

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

improve MacOS interposes to make mimalloc compile for older MacOS #1028

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 9 additions & 6 deletions src/alloc-override.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,12 @@ typedef void* mi_nothrow_t;
MI_INTERPOSE_MI(calloc),
MI_INTERPOSE_MI(realloc),
MI_INTERPOSE_MI(strdup),
#if defined(MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's worth deleting it.
Instead replace all MAC_OS_X_VERSION_MAX_ALLOWED with MAC_OS_X_VERSION_MIN_REQUIRED in this file (or better yet, everywhere else in this repository).
FYI read thread in this pull request: hacl-star/hacl-star#1042

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've god runtime error when I've done this. See #1028 (comment)

MI_INTERPOSE_MI(strndup),
#endif
MI_INTERPOSE_MI(realpath),
MI_INTERPOSE_MI(posix_memalign),
MI_INTERPOSE_MI(reallocf),
MI_INTERPOSE_MI(valloc),
MI_INTERPOSE_FUN(malloc_size,mi_malloc_size_checked),
MI_INTERPOSE_MI(malloc_good_size),
#if defined(MAC_OS_X_VERSION_10_15) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_15
MI_INTERPOSE_MI(aligned_alloc),
#endif
#ifdef MI_OSX_ZONE
// we interpose malloc_default_zone in alloc-override-osx.c so we can use mi_free safely
MI_INTERPOSE_MI(free),
Expand All @@ -99,6 +93,15 @@ typedef void* mi_nothrow_t;
MI_INTERPOSE_FUN(vfree,mi_cfree),
#endif
};
__attribute__((used)) static struct mi_interpose_s _mi_interposes_10_7[]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also seems redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying availability attribute to a declaration is a different thing. It should make this declaration conditionally accessible (weakly linked).
I've got this info from here: https://epir.at/2019/10/30/api-availability-and-target-conditionals/

A declaration can typically be used even when deploying back to a platform version prior to when the declaration was introduced. When this happens, the declaration is weakly linked, as if the weak_import attribute were added to the declaration. A weakly-linked declaration may or may not be present a run-time, and a program can determine whether the declaration is present by checking whether the address of that declaration is non-NULL

In reality, I was observing a crash on modern MacOSes with the current compile time exclusion of such interposes, but it works fine when is a separate decl.

__attribute__((section("__DATA, __interpose"))) __OSX_AVAILABLE(10.7) = {
MI_INTERPOSE_MI(strndup),
};

__attribute__((used)) static struct mi_interpose_s _mi_interposes_10_15[]
__attribute__((section("__DATA, __interpose"))) __OSX_AVAILABLE(10.15) = {
MI_INTERPOSE_MI(aligned_alloc),
};

#ifdef __cplusplus
extern "C" {
Expand Down