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

Fix a segfault on macOS when _dyld_shared_cache_contains_path is not available #615

Merged
merged 1 commit into from
May 30, 2024

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented May 28, 2024

The _dyld_shared_cache_contains_path is only available in macOS 11.0
or later. As we are compiling in a new system that's newer than 11.0 but
setting the deployment target to an older version, it's possible that
the _dyld_shared_cache_contains_path is not available in the target
platform and in that case we will segfault when calling it. It's also
possible that when compiling memray in an old system
_dyld_shared_cache_contains_path is not even prototyped in the headers.

To handle this, we change to fetch the function via a dlopen/dlsym approach
that will be generic in all macOS versions at the cost of an extra call
to dlopen the first time we interpose symbols.

@pablogsal pablogsal force-pushed the checking branch 3 times, most recently from ab34848 to 33d589a Compare May 29, 2024 16:21
Copy link
Contributor

@godlygeek godlygeek left a comment

Choose a reason for hiding this comment

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

LGTM

@godlygeek godlygeek changed the title Try to fix segfault when _dyld_shared_cache_contains_path is not available Fix a segfault on macOS when _dyld_shared_cache_contains_path is not available May 30, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.97%. Comparing base (41248ed) to head (a347134).
Report is 72 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
+ Coverage   92.55%   92.97%   +0.42%     
==========================================
  Files          91       92       +1     
  Lines       11304    11323      +19     
  Branches     1581     2076     +495     
==========================================
+ Hits        10462    10528      +66     
+ Misses        837      795      -42     
+ Partials        5        0       -5     
Flag Coverage Δ
cpp 92.97% <ø> (+7.03%) ⬆️
python_and_cython 92.97% <ø> (-2.75%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pablogsal pablogsal force-pushed the checking branch 2 times, most recently from ef9abc1 to 7337dae Compare May 30, 2024 21:08
@pablogsal pablogsal enabled auto-merge (rebase) May 30, 2024 21:08
The _dyld_shared_cache_contains_path is only available in macOS 11.0 or
later. As we are compiling in a new system that's newer than 11.0 but
setting the deployment target to an older version, it's possible that
the _dyld_shared_cache_contains_path is not available in the target
platform and in that case we will segfault when calling it. It's also
possible that when compiling memray in an old system
_dyld_shared_cache_contains_path is not even prototyped in the headers.

To handle this, we change to fetch the function via a dlopen/dlsym
approach that will be generic in all macOS versions at the cost of an
extra call to dlopen the first time we interpose symbols.

Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
@pablogsal pablogsal merged commit 7c4966b into bloomberg:main May 30, 2024
17 checks passed
@pablogsal pablogsal mentioned this pull request May 30, 2024
1 task
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.

None yet

3 participants