Skip to content

CDRIVER-5876 detect aligned_alloc via CMake instead of feature test macros #1851

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

Merged
merged 5 commits into from
Feb 5, 2025

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Feb 4, 2025

Resolves CDRIVER-5876. Alternative (preferable?) solution to #1841.

When #1072 was implemented, check_symbol_exists was deliberately avoided due to difficulty with accurately setting the required CMAKE_REQUIRED_* arguments, which are handled independently from the actual definitions and options used with the libraries being built (i.e. the various *_SOURCE macros used to control POSIX, FreeBSD, and Darwin features). However, it seems there are enough platform-specific edge cases concerning the actual availability of aligned_alloc despite efforts to use approprate feature test macros that a build system check is sufficiently well-motivated at this stage.

Therefore, this PR proposes adding a new BSON_HAVE_ALIGNED_ALLOC option to bson-config.h, which is detected using check_symbol_exists alongside other existing checks. This PR also applies a drive-by scan and update to CMAKE_REQUIRED_* variables to ensure better consistency of definitions/options being used for check_*_exists and the built libraries.


As a reminder, check_symbol_exists behaves as follows:

If the header files define the symbol as a macro it is considered available and assumed to work. If the header files declare the symbol as a function or variable then the symbol must also be available for linking (so intrinsics may not be detected).

and:

The following variables may be set before calling this macro to modify the way the check is run: CMAKE_REQUIRED_*

Although add_definitions, add_compile_options, and other directory-level and target-specific properties are not observed by check_symbol_exists, configuration options such as CMAKE_C_STANDARD and CMAKE_C_FLAGS are observed by check_symbol_exists. It is not entirely clear where/how the line is drawn. For our purposes, maintaining CMAKE_REQUIRED_DEFINITIONS and CMAKE_REQUIRED_LIBRARIES is probably sufficient to ensure consistency between symbol checks and built libraries.

@eramongodb eramongodb self-assigned this Feb 4, 2025
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

I like the change to detect the symbol in CMake, but this appears to cause task failures with clang.

@eramongodb eramongodb requested a review from kevinAlbs February 4, 2025 21:38
@kevinAlbs kevinAlbs removed the request for review from jmikola February 5, 2025 16:49
@eramongodb eramongodb merged commit 2a35499 into mongodb:master Feb 5, 2025
41 of 43 checks passed
@eramongodb eramongodb deleted the cdriver-5876 branch February 5, 2025 16:57
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.

2 participants