Code sections of ocamlopt-generated executables must remain readable

[xavierleroy]

This is required for marshaling of function closures. OpenBSD 7.3 makes code sections non-readable by default. This PR adds magic to configure so as to detect OpenBSD >= 7.3 and activate the --no-execute-only linker flag.

[Octachron]

cc @shindere

[shindere]

The comment in `configure..ac` refers to `OCaml-generated executables` whereas the PR title and commit message only mention ocamlopt. Which one of the two is correct? The `AS_CASE` is on `$host`. Although I think it won't make a bitg difference yet, wouldn't it be more accurate to do the test on `$target`? And finally, if I am reading the pattern correctly I understand that it will detect only versions 8 and 9 of OpenBSD but that the flag won't be added on version 10 and above. Is the OpenBSD release cycle such that it's reasonoable to not take this into account? One thing worth exploring is `ax_compare_version.html`. We could add that file to our `build-aux` directory and use the macros it provides and which look nicer, especially given that we do version comparison in several places.

[xavierleroy]

The comment in configure..ac refers to OCaml-generated executables
whereas the PR title and commit message only mention ocamlopt. Which one
of the two is correct?

Probably neither, but the former is less wrong than the latter. If I read the script correctly, oc_ldflags ends up in mkexe and perhaps other places, so it will be used not just when ocamlopt produces an executable.

The AS_CASE is on $host. Although I think it won't make a bitg
difference yet, wouldn't it be more accurate to do the test on
$target?

All the case analyses that affect oc_flags use $host. It's probably wrong but at least it's consistently wrong.

And finally, if I am reading the pattern correctly I understand that it
will detect only versions 8 and 9 of OpenBSD but that the flag won't be
added on version 10 and above. Is the OpenBSD release cycle such that
it's reasonoable to not take this into account?

By the time OpenBSD 10 is released, I fully expect them to have changed their security model and added custom linker flags several times. We'll adapt as needed.

[avsm]

Ran the testsuite on OpenBSD-current/amd64 where it passes with this PR (and fails without). Will try on OpenBSD/arm64 tomorrow; not got access to that machine right now.

[xavierleroy]

Will try on OpenBSD/arm64 tomorrow; not got access to that machine right now.

Thanks! FYI, the Jenkins CI at Inria includes an amd64/OpenBSD machine, but no other OpenBSD configuration.

[xavierleroy]

Ping! This PR is being considered for inclusion in 5.1, so we have to decide quickly whether it's good enough for that .

[avsm]

I just finishing triaging what's going on; we were triggering a base OS bug on OpenBSD 7.3 as well!

• OpenBSD 7.3's dlopen(RTLD_LOCAL) seems to be broken on arm64, and that's what was causing all the weird failures from Fix OpenBSD/arm64 support #12221. When running the 'load private module with the same name' test from lib-dynlink-private, it recurses and blows the stack reliably. openbsd/src@ba367c0 looks like it might possibly be the revert that fixed it there, so I didn't go in further.
• Upgrading to OpenBSD-current has much better results. This PR worked fine on the OpenBSD-current/amd64 box I tried it on.
• OpenBSD-current/arm64 fails until mkdll_flags also has -no-execute-onlypassed as well. avsm@47be6a5 is that commit over this PR (partially cherry-picked from Fix OpenBSD/arm64 support #12221).

So in summary, this PR fixes OpenBSD on amd64, and https://github.com/avsm/ocaml/tree/openbsd-no-execute-only has the extra commit to fix it on arm64 (for the next release of OpenBSD) as well. I'll follow up separately by email about getting OpenBSD/arm64 added to the Inria testing matrix...

[avsm]

Minor tweak: to the arm64 fix to use the same OS version detection as the other check: avsm@32fe3b9

[shindere]

Just one additional question. Wouldn't it be worth checking which compiler is used before adding compiler flags? Just in case the user had overriden the default compiler.

[avsm]

Just in case the user had overriden the default compiler

This is usually true on operating systems like Linux, but on OpenBSD the base system depends on the distributed compiler included with that release. There are often a number of OpenBSD-specific changes; see gcc-local(1) and clang-local(1). So there is no expectation that a third-party compiler will work out-of-the-box without having some of those local adaptations taken into account, and therefore no need to check for that in our OCaml configure scripts.

[shindere]

Okay, many thanks for your feedback on that, Anil. Since you have done much more testing on this PR than me I'd feel more comfortable if it is you (rather thanme) who approve it. I think it would have been nice to use the version-comparison macro refered to earlier.

[xavierleroy]

Wouldn't it be worth checking which compiler is used before adding compiler flags?

It's a linker flag, so it doesn't matter which C compiler is used.

[shindere]

Xavier Leroy (2023/07/17 07:37 -0700):

> Wouldn't it be worth checking which compiler is used before adding compiler flags? It's a linker flag, so it doesn't matter which C compiler is used.

OK, got it.

[xavierleroy]

Minor tweak: to the arm64 fix to use the same OS version detection as the other check: avsm@32fe3b9

Merged in this PR, thanks. I'm running a round of CI precheck before merging this PR.

[xavierleroy]

Merged and cherry-picked to 5.1

[xavierleroy]

Backported to the 4.14 branch (49bff4c).

[xavierleroy]

Backported to the 4.14 LTS branch (49bff4c), as it is required for 4.14 to work on OpenBSD.