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

Remove -flat_namespace flag in configure.ac on Darwin #10723

Merged
merged 3 commits into from
Feb 18, 2022
Merged

Remove -flat_namespace flag in configure.ac on Darwin #10723

merged 3 commits into from
Feb 18, 2022

Conversation

carlocab
Copy link
Contributor

Apple discourages use of the -flat_namespace flag, and it exists only
for compatibility with very old versions of OS X. Using
-flat_namespace can cause various cryptic linker errors (e.g. due to
name collisions), so it's best to avoid its use unless needed.

This change removes the flag, and also changes -undefined suppress to
-undefined dynamic_lookup. We need to change the argument to
-undefined because omitting the -flat_namespace flag will build
shared libraries with a two-level namespace, and this does not support
-undefined suppress. dynamic_lookup means that the dynamic linker
will resolve undefined symbols at runtime.

This should have no visible impact on any users who do not explicitly
exploit the flat namespace, but this is of often only useful for
debugging purposes. Users who need this still have alternatives such as
setting DYLD_FORCE_FLAT_NAMESPACE, which instructs the linker to treat
libraries as if they were compiled with a flat namespace.

@carlocab
Copy link
Contributor Author

Incidentally, build-aux/libtool.m4 also has a bug that misidentifies new
versions of macOS (Big Sur or newer) as old ones, therefore erroneously passing
-flat_namespace -undefined suppress instead of -undefined dynamic_lookup.

This occurs in this snippet:

ocaml/build-aux/libtool.m4

Lines 1065 to 1081 in 4c52549

rhapsody* | darwin1.[[012]])
_lt_dar_allow_undefined='$wl-undefined ${wl}suppress' ;;
darwin1.*)
_lt_dar_allow_undefined='$wl-flat_namespace $wl-undefined ${wl}suppress' ;;
darwin*) # darwin 5.x on
# if running on 10.5 or later, the deployment target defaults
# to the OS version, if on x86, and 10.4, the deployment
# target defaults to 10.4. Don't you love it?
case ${MACOSX_DEPLOYMENT_TARGET-10.0},$host in
10.0,*86*-darwin8*|10.0,*-darwin[[91]]*)
_lt_dar_allow_undefined='$wl-undefined ${wl}dynamic_lookup' ;;
10.[[012]][[,.]]*)
_lt_dar_allow_undefined='$wl-flat_namespace $wl-undefined ${wl}suppress' ;;
10.*)
_lt_dar_allow_undefined='$wl-undefined ${wl}dynamic_lookup' ;;
esac
;;

A patch has been sent to Libtool to fix this.

@xavierleroy
Copy link
Contributor

@damiendoligez @garrigue : what do you think about this change? Thanks.

@carlocab
Copy link
Contributor Author

carlocab commented Nov 5, 2021

Here's an example of what can go wrong if you use -flat_namespace -undefined suppress instead of -undefined dynamic_lookup: https://developer.apple.com/forums/thread/689991 (Notice an Apple engineer mentioning that -flat_namespace is "effectively deprecated".)

Some additional background:

If it helps: I maintain Homebrew, and we package a lot of software on macOS. -flat_namespace is almost always a poor choice for any version of macOS that isn't very old. Libtool won't even give you this flag unless you're using an old version of OS X (or if you're bitten by the version detection bug I mention above).

The only time I'd consider using it is if I'd want to retain support for users running OS X on PowerPC. I don't have access to a PowerPC machine to even check if I can build a recent version of OCaml on it, but MacPorts does, and it doesn't look like they've managed to build OCaml on any version of OS X that can still run on a PowerPC Mac.

@garrigue
Copy link
Contributor

garrigue commented Nov 5, 2021

This code is so old I have little idea what changed since then.
My only suggestion is to try as suggested by Apple, and see whether it breaks any package. If there is a real problem, it should appear soon enough.

IIRC, the windows code was originally designed to allow compiling with a hierarchized namespace, but flexdll then emulated a flat namespace. There is a risk behavior on MacOS would diverge with this change, but I don't know enough to see the implications.

BTW, I do have a powerPC machine in my office. I do not turn it up often, but if there is something to test I can try.

carlocab added a commit to carlocab/homebrew-core that referenced this pull request Nov 7, 2021
We apply two patches: one of our generic Libtool patches, and a patch
upstreamed upstreamed at ocaml/ocaml#10723. We embed a patch here to
avoid having to regenerate `configure`, which we would need to do to use
the upstreamed patch.

This is needed for bottling on Monterey.
carlocab added a commit to Homebrew/homebrew-core that referenced this pull request Nov 8, 2021
We apply two patches: one of our generic Libtool patches, and a patch
upstreamed upstreamed at ocaml/ocaml#10723. We embed a patch here to
avoid having to regenerate `configure`, which we would need to do to use
the upstreamed patch.

This is needed for bottling on Monterey.
@damiendoligez
Copy link
Member

The -flat_namespace was added in 2002 to support dynamic linking on PowerPC... If it's deprecated, let's try and remove it. I will run an opam check to see if it breaks anything.

damiendoligez
damiendoligez previously approved these changes Dec 8, 2021
Copy link
Member

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

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

LGTM
[edit: wait a minute, configure is stale, so I don't know whether I tested correctly]

@damiendoligez damiendoligez dismissed their stale review December 8, 2021 11:19

not sure I tested correctly

@carlocab
Copy link
Contributor Author

carlocab commented Dec 8, 2021

Oops, sorry. I forgot to regenerate it. I've done that now , but it looks like I'll need to resolve some conflicts from it.

@carlocab
Copy link
Contributor Author

carlocab commented Dec 8, 2021

Looks like my running autoconf made a bunch of unrelated changes... I'm not actually that familiar with autoconf, so if I did this wrong I'd appreciate some pointers.

@nojb
Copy link
Contributor

nojb commented Dec 8, 2021

Looks like my running autoconf made a bunch of unrelated changes... I'm not actually that familiar with autoconf, so if I did this wrong I'd appreciate some pointers.

I think you should use the tools/autogen script to regenerate the configure script.

@carlocab
Copy link
Contributor Author

carlocab commented Dec 8, 2021

Aha, that's done now. Thanks for your patience!

Copy link
Member

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

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

I did test correctly, all OPAM packages (that I can install on my machine) are compatible with this PR.

carlocab and others added 3 commits February 18, 2022 18:26
Apple discourages use of the `-flat_namespace` flag, and it exists only
for compatibility with very old versions of OS X. Using
`-flat_namespace` can cause various cryptic linker errors (e.g. due to
name collisions), so it's best to avoid its use unless needed.

This change removes the flag, and also changes `-undefined suppress` to
`-undefined dynamic_lookup`. We need to change the argument to
`-undefined` because omitting the `-flat_namespace` flag will build
shared libraries with a two-level namespace, and this does not support
`-undefined suppress`. `dynamic_lookup` means that the dynamic linker
will resolve undefined symbols at runtime.

This should have no visible impact on any users who do not explicitly
exploit the flat namespace, but this is of often only useful for
debugging purposes. Users who need this still have alternatives such as
setting `DYLD_FORCE_FLAT_NAMESPACE`, which instructs the linker to treat
libraries as if they were compiled with a flat namespace.
@xavierleroy xavierleroy merged commit a88c1db into ocaml:trunk Feb 18, 2022
@carlocab carlocab deleted the flat-namespace branch February 19, 2022 19:41
@carlocab
Copy link
Contributor Author

Thanks for cleaning this up for me.

Octachron pushed a commit to Octachron/ocaml that referenced this pull request Oct 17, 2023
Apple discourages use of the `-flat_namespace` flag, and it exists only
for compatibility with very old versions of OS X. Using
`-flat_namespace` can cause various cryptic linker errors (e.g. due to
name collisions), so it's best to avoid its use unless needed.

This change removes the flag, and also changes `-undefined suppress` to
`-undefined dynamic_lookup`. We need to change the argument to
`-undefined` because omitting the `-flat_namespace` flag will build
shared libraries with a two-level namespace, and this does not support
`-undefined suppress`. `dynamic_lookup` means that the dynamic linker
will resolve undefined symbols at runtime.

This should have no visible impact on any users who do not explicitly
exploit the flat namespace, but this is of often only useful for
debugging purposes. Users who need this still have alternatives such as
setting `DYLD_FORCE_FLAT_NAMESPACE`, which instructs the linker to treat
libraries as if they were compiled with a flat namespace.

(cherry picked from commit a88c1db)
dra27 pushed a commit to dra27/ocaml that referenced this pull request Jul 29, 2024
Apple discourages use of the `-flat_namespace` flag, and it exists only
for compatibility with very old versions of OS X. Using
`-flat_namespace` can cause various cryptic linker errors (e.g. due to
name collisions), so it's best to avoid its use unless needed.

This change removes the flag, and also changes `-undefined suppress` to
`-undefined dynamic_lookup`. We need to change the argument to
`-undefined` because omitting the `-flat_namespace` flag will build
shared libraries with a two-level namespace, and this does not support
`-undefined suppress`. `dynamic_lookup` means that the dynamic linker
will resolve undefined symbols at runtime.

This should have no visible impact on any users who do not explicitly
exploit the flat namespace, but this is of often only useful for
debugging purposes. Users who need this still have alternatives such as
setting `DYLD_FORCE_FLAT_NAMESPACE`, which instructs the linker to treat
libraries as if they were compiled with a flat namespace.

(cherry picked from commit a88c1db)
dra27 pushed a commit to dra27/ocaml that referenced this pull request Jul 29, 2024
Apple discourages use of the `-flat_namespace` flag, and it exists only
for compatibility with very old versions of OS X. Using
`-flat_namespace` can cause various cryptic linker errors (e.g. due to
name collisions), so it's best to avoid its use unless needed.

This change removes the flag, and also changes `-undefined suppress` to
`-undefined dynamic_lookup`. We need to change the argument to
`-undefined` because omitting the `-flat_namespace` flag will build
shared libraries with a two-level namespace, and this does not support
`-undefined suppress`. `dynamic_lookup` means that the dynamic linker
will resolve undefined symbols at runtime.

This should have no visible impact on any users who do not explicitly
exploit the flat namespace, but this is of often only useful for
debugging purposes. Users who need this still have alternatives such as
setting `DYLD_FORCE_FLAT_NAMESPACE`, which instructs the linker to treat
libraries as if they were compiled with a flat namespace.

(cherry picked from commit a88c1db)
carlocab added a commit to carlocab/AFLplusplus that referenced this pull request Oct 2, 2024
`-flat_namespace` is effectively deprecated and doesn't really work as
expected these days. Omitting the `-flat_namespace` means that binaries
are built with a two-level namespace, which don't support
`-undefined suppress`.

The idiomatic way of telling the linker to look up undefined symbols at
runtime is using `-undefined dynamic_lookup`, which is supported by a
two-level namespace.

See also:
ocaml/ocaml#10723
mono/mono#21257
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.

5 participants