Add option to compile ocaml with leak sanitizer.

[toots]

This PR adds a configuration package to compile ocaml with -fsanitize=address (linux & macos) or -fsanitize=leak (linux).

These options provide tools to inspect and detect issues related to memory allocation and can be very useful to developers. See: https://github.com/google/sanitizers/wiki/ for more details.

Because compilation builds and executes its own binary, the checks must disabled during compilation, which is is documented in the package descriptions.

[toots]

I'm not sure why the CI tests are failing w/o more logs. I have tested the config successfully on linux and macos.

[dra27]

Sorry, I hadn't seen this one. This looks great, in principle, there are a few things which need doing:

• I think this should work with OCaml 4.12 and 4.13 - can we update the +options packages for those versions as well?
• The passage of time means that ocaml-variants.5.0.0~alpha1+options now needs updating, too
• All of the ocaml-options-only-* packages need these two packages added to their conflicts list (which I think may aid some of the CI failures)
• Does the address sanitizer work on macos arm64 as well as amd64?
• Rather than advising the user to build the switch with particular environment variables set, it would be much better to use build-env in the packages themselves. Unfortunately, it's not possible to gate build-env changes with a formula as you can in build, but I think this would be fine for ocaml.4.14.0 and ocaml-variants.4.14.0+options, etc.

--- a/packages/ocaml-variants/ocaml-variants.4.14.0+options/opam
+++ b/packages/ocaml-variants/ocaml-variants.4.14.0+options/opam
@@ -15,6 +15,10 @@ depends: [
 conflict-class: "ocaml-core-compiler"
 flags: compiler
 setenv: CAML_LD_LIBRARY_PATH = "%{lib}%/stublibs"
+build-env: [
+  [LSAN_OPTIONS = "detect_leaks=0,exitcode=0"]
+  [ASAN_OPTIONS = "detect_leaks=0,exitcode=0"]
+]
 build: [
   [
     "./configure"
diff --git a/packages/ocaml/ocaml.4.14.0/opam b/packages/ocaml/ocaml.4.14.0/opam
index 91ef323cd5..81b186f1ac 100644
--- a/packages/ocaml/ocaml.4.14.0/opam
+++ b/packages/ocaml/ocaml.4.14.0/opam
@@ -17,7 +17,11 @@ setenv: [
   [OCAML_TOPLEVEL_PATH = "%{toplevel}%"]
 ]
 build: ["ocaml" "%{ocaml-config:share}%/gen_ocaml_config.ml" _:version _:name]
-build-env: CAML_LD_LIBRARY_PATH = ""
+build-env: [
+  [CAML_LD_LIBRARY_PATH = ""]
+  [LSAN_OPTIONS = "detect_leaks=0,exitcode=0"]
+  [ASAN_OPTIONS = "detect_leaks=0,exitcode=0"]
+]
 homepage: "https://ocaml.org"
 bug-reports: "https://github.com/ocaml/opam-repository/issues"
 authors: [

That said, I may be misunderstanding this, but is the aim here that the programs built and linked by the compiler want this, but not the compiler itself? In other words, do we want ocamlopt.opt to have been compiled completely without sanitizer options but to have the installed libasmrun.a compiled with with sanitizer options? cc @fabbing and @oliviernicole who've both been working TSAN in OCaml and have a better understanding of the mechanics of this than I do.

[OlivierNicole]

Just to clarify, is this to help compiler developers, or developers of regular OCaml programs? I would assume the former, as OCaml manages memory for the programmer. Or is this for programmers who develop OCaml libraries that contain C code?

[toots]

@OlivierNicole it's a bit of both I believe. Enabling this option can help tracking memory leaks in both the internal OCaml code (see: https://github.com/ocaml/ocaml/pull/11406) but also any code compiled when building, including dependencies.

I ended up going that route for a large project where rebuilding individual libraries with this enabled was not practical. We're still tracking memory usage issues, too.

Sorry I haven't had much time to go back to this. Will do my best to find some time soon. Thanks for the pointed advices @dra27

[toots]

Thanks for the great review @dra27 I've applied most changes. I say the compiler should be built w/o it but the library and default compiler have it enabled. This would make it possible to compile programs using ocamlopt without having to set env variables. I'm not sure how to do this w/o changing a lot of the compiler's code so, perhaps as a first support this would be fine like this?

[OlivierNicole]

That said, I may be misunderstanding this, but is the aim here that the programs built and linked by the compiler want this, but not the compiler itself? In other words, do we want ocamlopt.opt to have been compiled completely without sanitizer options but to have the installed libasmrun.a compiled with with sanitizer options?

I say the compiler should be built w/o it but the library and default compiler have it enabled. This would make it possible to compile programs using ocamlopt without having to set env variables. I'm not sure how to do this w/o changing a lot of the compiler's code so, perhaps as a first support this would be fine like this?

Yes, ideally ocamlc.opt and ocamlopt.opt should not be compiled instrumented, but the libraries should be. However, this is not possible in the current build process as the Stdlib compiled by ocamlopt is used to create ocamlc.opt and ocamlopt.opt. Ideally we would need to build two versions of the stdlib, one instrumented and one non instrumented. But in a meeting @dra27 you seemed to imply that this would be painful.

If that's any motivation, we have exactly the same problem with TSan, and for now I don't see how to do better than telling the user to set different env variables during build and during execution of user programs.

[toots]

Hey there! Any opinion on this? Would be nice to merge it before it gets outdated again. I think that the current implementation is a good start, I've been using it on different systems already and it's proven useful.

[raphael-proust]

These options should probably be exposed as an ocaml-options-only-* packages with conflicts against all other options. That's because the added environment variables will probably not survive the combination of two different options.

@toots Can you make that change? (Or maybe I missed something?)

[toots]

@raphael-proust packages converted to exclusive packages.

Y'all could we consider a decision here? This PR is 6 months old now. I believe that it's pretty useful but I cannot keep chasing new compilers and options forever.

[raphael-proust]

There are failures in the CI but they are just revdeps and the few I checked seem unrelated to these options being set. (Not sure it's the case for all of them.)

I'm ok with the PR as it stands but I'm pretty new here and I'm likely to have missed things.

It might be useful for the options-only packages to be able to use options-* in the conflicts field (obviously this shouldn't indicate a conflict with itself). It'd simplify the many options-only packages and PRs like this one. Of course, this is independent on merging this PR.

[toots]

Thanks for the review y'all. @dra27 rename suggestion have been applied and support has been drop for all 4.x versions. Hopefully this is the one!

[toots]

I think i forgot to keep the change in ocaml.5.0.0 as well. Could you also add the depopt and everything to the 5.1.0 packages as well?

Done!

[kit-ty-kate]

ocaml-option-address-sanitizer seems to be failing:

#=== ERROR while compiling ocaml-variants.5.0.0+trunk =========================#
# context              2.2.0~alpha~dev | linux/x86_64 | ocaml-variants.5.0.0+trunk | pinned(https://github.com/ocaml/ocaml/archive/5.0.tar.gz)
# path                 ~/.opam/5.0/.opam-switch/build/ocaml-variants.5.0.0+trunk
# command              ~/.opam/opam-init/hooks/sandbox.sh build ./configure --prefix=/home/opam/.opam/5.0 --docdir=/home/opam/.opam/5.0/doc/ocaml -C LDFLAGS=-Wl,--no-as-needed,-ldl CC=gcc -ldl -fsanitize=address -fno-omit-frame-pointer -O1 -g --disable-warn-error
# exit-code            77
# env-file             ~/.opam/log/ocaml-variants-7-7ca6b7.env
# output-file          ~/.opam/log/ocaml-variants-7-7ca6b7.out
### output ###
# configure: creating cache config.cache
# configure: Configuring OCaml version 5.0.0+dev8-2022-10-12
# checking build system type... x86_64-pc-linux-gnu
# checking host system type... x86_64-pc-linux-gnu
# checking target system type... x86_64-pc-linux-gnu
# checking for ld... ld
# checking how to print strings... printf
# checking for gcc... gcc -ldl -fsanitize=address -fno-omit-frame-pointer -O1 -g
# checking whether the C compiler works... yes
# checking for C compiler default output file name... a.out
# checking for suffix of executables... 
# checking whether we are cross compiling... configure: error: in `/home/opam/.opam/5.0/.opam-switch/build/ocaml-variants.5.0.0+trunk':
# configure: error: cannot run C compiled programs.
# If you meant to cross compile, use `--host'.
# See `config.log' for more details

[toots]

ocaml-option-address-sanitizer seems to be failing:

#=== ERROR while compiling ocaml-variants.5.0.0+trunk =========================#
# context              2.2.0~alpha~dev | linux/x86_64 | ocaml-variants.5.0.0+trunk | pinned(https://github.com/ocaml/ocaml/archive/5.0.tar.gz)
# path                 ~/.opam/5.0/.opam-switch/build/ocaml-variants.5.0.0+trunk
# command              ~/.opam/opam-init/hooks/sandbox.sh build ./configure --prefix=/home/opam/.opam/5.0 --docdir=/home/opam/.opam/5.0/doc/ocaml -C LDFLAGS=-Wl,--no-as-needed,-ldl CC=gcc -ldl -fsanitize=address -fno-omit-frame-pointer -O1 -g --disable-warn-error
# exit-code            77
# env-file             ~/.opam/log/ocaml-variants-7-7ca6b7.env
# output-file          ~/.opam/log/ocaml-variants-7-7ca6b7.out
### output ###
# configure: creating cache config.cache
# configure: Configuring OCaml version 5.0.0+dev8-2022-10-12
# checking build system type... x86_64-pc-linux-gnu
# checking host system type... x86_64-pc-linux-gnu
# checking target system type... x86_64-pc-linux-gnu
# checking for ld... ld
# checking how to print strings... printf
# checking for gcc... gcc -ldl -fsanitize=address -fno-omit-frame-pointer -O1 -g
# checking whether the C compiler works... yes
# checking for C compiler default output file name... a.out
# checking for suffix of executables... 
# checking whether we are cross compiling... configure: error: in `/home/opam/.opam/5.0/.opam-switch/build/ocaml-variants.5.0.0+trunk':
# configure: error: cannot run C compiled programs.
# If you meant to cross compile, use `--host'.
# See `config.log' for more details

This is odd. I can't reproduce locally and the invocation hasn't changed much. I'm temped to consider it a CI issue. I've rebased the PR, let's see if a new CI run works.

[toots]

@kit-ty-kate I can confirm that manually running:

opam switch create asan  --packages ocaml-option-address-sanitizer.1,ocaml-variants.5.0.0+trunk

does work on a ubuntu 22.04 LTS system.

My guess is that something changed in the docker CI config. These debugging tools might need specific OS package/versions, I'm not surprised that they wouldn't work in all cases.

[kit-ty-kate]

ping @dra27 for a review

(also, at this point let’s wait for the next OCaml 5.0 beta to get this merged and avoid unnecessary recompilation for the users)

[toots]

ping @dra27 for a review

(also, at this point let’s wait for the next OCaml 5.0 beta to get this merged and avoid unnecessary recompilation for the users)

Ok. Thanks for following-up!

[raphael-proust]

There is a merge conflict that's not syntactical so git doesn't complain.

Waiting for the beta to fix this.

[kit-ty-kate]

This PR got merged as part #22566. Thanks and sorry for the wait.

[toots]

Awesome thanks for taking care of this!