Skip to content

Fix linking library for POSIX shared memory functions #13822

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 1 commit into from
Mar 29, 2024

Conversation

petk
Copy link
Member

@petk petk commented Mar 28, 2024

I'll recheck this in the morning but I think I got this right. The Autoconf CS is horrible yet it is something like this:

The POSIX shared memory object operations functions (shm_open, shm_unlink...) are in:

  • C library on most systems (newer Linux, Solaris 11.4, illumos, BSD*, macOS, Haiku R1, etc.)
  • real-time (rt) library on older Linux distributions and Solaris <= 10.
  • root library on Haiku nightly

Previous check always added additional rt or root library to global LIBS and rt to OPCACHE_SHARED_LIBADD. Now, the library containing shm_open is linked as needed to the always shared opcache extension.

This also removes unused HAVE_SHM_OPEN and HAVE_LIBROOT symbols.

@petk
Copy link
Member Author

petk commented Mar 29, 2024

@devnexen, sorry to ping you again. I'm rechecking this libroot (Haiku's C standard library) on Haiku and now I can't reproduce this anymore. I have no idea how I managed to do that shm_open was not found in the default linked C standard library on Haiku and that it would be required to explicitly link the libroot. It works on all Haiku versions. I've probably just added it in the PR and was checking some other function locally in my test files or I've hacked the Haiku way too much manually somehow.

From what I understand, is that the libroot is linked in by default on Haiku: https://www.haiku-os.org/docs/api/libroot.html
This applies for versions at least somewhere since 2013 or even earlier - 2005. BeOS didn't have it.
haiku/haiku@74ca3b2

So basically, we don't need to link libroot anymore. This sound ok to you or do you remember perhaps some case where this would be needed?

I'll append this to the PR here:

--- a/ext/opcache/config.m4
+++ b/ext/opcache/config.m4
@@ -239,12 +239,12 @@ int main(void) {
   AC_MSG_RESULT([$have_shm_mmap_anon])
 
   dnl Check POSIX shared memory object operations and link required library as
-  dnl needed: rt (older Linux and Solaris <= 10), root (Haiku nightly), other
-  dnl systems have it in C library (newer Linux, illumos, Solaris 11.4, macOS,
-  dnl BSD-based systems, Haiku R1...).
+  dnl needed: rt (older Linux and Solaris <= 10). Most systems have it in the C
+  dnl standard library (newer Linux, illumos, Solaris 11.4, macOS, BSD-based
+  dnl systems). Haiku has it in root, which is linked in by default,
   LIBS_save="$LIBS"
   LIBS=
-  AC_SEARCH_LIBS([shm_open], [rt root],
+  AC_SEARCH_LIBS([shm_open], [rt],
     [AC_CACHE_CHECK([for mmap() using shm_open() shared memory support],
       [php_cv_shm_mmap_posix],
       [AC_RUN_IFELSE([AC_LANG_SOURCE([[

@devnexen
Copy link
Member

I can t think of one reason, it might have been needed for a specific haiku beta version then might have been fixed at some point.

The POSIX shared memory object operations functions (shm_open,
shm_unlink...) are in:
* C library on most systems (newer Linux, Solaris 11.4, illumos, BSD*,
  macOS, Haiku, etc.)
* real-time (rt) library on older Linux distributions and Solaris <= 10.

Haiku C library (called root library) on Haiku is linked in by default.

Previous check always added additional rt or root library to global LIBS
and rt to OPCACHE_SHARED_LIBADD. Now, the library containing shm_open is
linked as needed to the always shared opcache extension.

This also removes unused HAVE_SHM_OPEN and HAVE_LIBROOT symbols.
@petk petk merged commit 6c2d5e5 into php:master Mar 29, 2024
@petk petk deleted the patch-shm-open branch March 29, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants