Skip to content

Conversation

@Noxybot
Copy link
Contributor

@Noxybot Noxybot commented Mar 6, 2025

I've got a failure while trying to compile with "-mmacosx-version-min=10.7" since MAC_OS_X_VERSION_MAX_ALLOWED is greater than MAC_OS_X_VERSION_10_15 but still we can't use aligned_alloc since of the lower bound (10.7). I'm not sure about internal details of how __OSX_AVAILABLE works, but my hope is the it would only make this interpose available starting from the OS version specified. Also, it helps me to compile mimalloc with "-mmacosx-version-min=10.7".

I've got a failure while trying to compile with "-mmacosx-version-min=10.7" since MAC_OS_X_VERSION_MAX_ALLOWED is greater than MAC_OS_X_VERSION_10_15 but still we can't use `aligned_alloc` since of the lower bound (10.7).
I'm not sure about internal details of how `__OSX_AVAILABLE` works, but my hope is the it would only make this interpose available starting from the OS version specified.
Also, it helps me to compile mimalloc with "-mmacosx-version-min=10.7".
@Noxybot
Copy link
Contributor Author

Noxybot commented Mar 31, 2025

Hey @daanx - could you check this change once again, please?
It makes some interposes more robust (so they are always present when available rather than excluded depending on target MacOS version that mimalloc is compiled for).
I've been running both dev2 and dev3 with this change locally for quite some time (and mimalloc seems to not work without it in our environment).

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

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_FUN(vfree,mi_cfree),
#endif
};
__attribute__((used)) static struct mi_interpose_s _mi_interposes_10_7[]

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.

@aeiouaeiouaeiouaeiouaeiouaeiou

I've got a failure while trying to compile with "-mmacosx-version-min=10.7" since MAC_OS_X_VERSION_MAX_ALLOWED is greater than MAC_OS_X_VERSION_10_15 but still we can't use aligned_alloc since of the lower bound (10.7).

Because MAC_OS_X_VERSION_MAX_ALLOWED forces the current SDK version by default.

@daanx
Copy link
Collaborator

daanx commented Jun 8, 2025

I took this PR and manually integrated it. Let me know if this works for you @Noxybot. Thanks!

@Noxybot Noxybot closed this Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants