Undefined reference to "caml_startup" when trying to run dune utop

[afdw]

In an ocaml-rs project, for example ocaml-rust-starter, running dune utop gives the following error (while linking):

/usr/bin/ld: src/libocaml_rust_starter.a(ocaml_interop-4a398e80f3ca6ceb.ocaml_interop.8eq22uyz-cgu.12.rcgu.o): in function `_ZN3std4sync4once4Once9call_once28_$u7b$$u7b$closure$u7d$$u7d$17h7171c1b9a6fc7898E.llvm.3140953836603515050':
ocaml_interop.8eq22uyz-cgu.12:(.text._ZN3std4sync4once4Once9call_once28_$u7b$$u7b$closure$u7d$$u7d$17h7171c1b9a6fc7898E.llvm.3140953836603515050+0x3b): undefined reference to `caml_startup'
/usr/bin/ld: src/libocaml_rust_starter.a(ocaml_interop-4a398e80f3ca6ceb.ocaml_interop.8eq22uyz-cgu.12.rcgu.o): in function `_ZN4core3ops8function6FnOnce40call_once$u7b$$u7b$vtable.shim$u7d$$u7d$17h59471d095cabe6c5E.llvm.3140953836603515050':
ocaml_interop.8eq22uyz-cgu.12:(.text._ZN4core3ops8function6FnOnce40call_once$u7b$$u7b$vtable.shim$u7d$$u7d$17h59471d095cabe6c5E.llvm.3140953836603515050+0x3b): undefined reference to `caml_startup'
collect2: error: ld returned 1 exit status

Adding this function somewhere where the linker sees it, for example using the following code, "fixes" the issue and utop starts fine:

#[ocaml::func]
pub fn caml_startup() {}

OCaml version: 4.11.1
Rust version: nightly
GCC version: 10.2.0

[zshipko]

Thanks for the report! I will be able to take a closer look in the next few days.

It looks like caml_startup is getting optimized out at link time - I know it is defined in ocaml_sys.

[zshipko]

Hm, It looks like this may be an issue with ocaml-interop - I will open an issue there and keep investigating.

[tizoc]

Any idea of what dune utop does to link stuff? ocaml-interop makes all calls to the OCaml runtime through ocaml-sys, and doesn't do anything related to linking because that is up to the consumer program (which may be a Rust or OCaml program or library).

[zshipko]

I will need to look into what dune utop is actually doing.

Do you use ocaml-interop with the bytecode interpreter? I wonder if this could be due to some initialization difference there?

If I run dune runtest in the ocaml-rust-starter repo with (modes byte) set on the test stanza I get the same linker error as above. I don't know much about the bytecode runtime but I would assume that caml_startup is defined there as well, but maybe that's not correct

[tizoc]

@zshipko yes, it is defined there too, bytecode also uses caml_startup. What I am thinking is that dune utop is probably producing a .so, but because it is meant to be loaded by the bytecode runtime, then the runtime is not included in the .so itself. This means that ocaml-interop references caml_startup but the final object into which it is being linked to doesn't include it.

I have to reflect more on it, but I think the correct thing would be for ocaml-interop to have a feature flag that disables the compilation of the runtime initialization code to cover cases like this one. What I am not sure about is what would be the proper way to have dune utop enable that feature flag.

[afdw]

I have to reflect more on it, but I think the correct thing would be for ocaml-interop to have a feature flag that disables the compilation of the runtime initialization code to cover cases like this one. What I am not sure about is what would be the proper way to have dune utop enable that feature flag.

Just a thought, I do know what is really going on: both static library and dynamic library are built; maybe the library type should determine the behavior here?

[tizoc]

Probably not, because I use ocaml-interop to produce a library that is dynamically linked into a Rust program, and that library also includes the OCaml runtime. But to be honest, the reason I produce a dynamic library and not a static one is that I haven't figure out how to make dune produce a static library for me to use.

[tizoc]

But my point is that when producing a .so you may want to include the OCaml runtime (along with other OCaml code) in it, so knowing that it is a .so is not enough.

[mimoo]

Anybody has found a work around for this?

[afdw]

Anybody has found a work around for this?

There is partial one is the description of the issue which work for me.

[mimoo]

This is what I get when I use your trick:

ld: warning: directory not found for option '-L/opt/local/lib'
duplicate symbol '_caml_startup' in:
    /xxx/_opam/lib/ocaml/libasmrun.a(startup_nat_n.o)
    src/lib/marlin_plonk_bindings/stubs/libmarlin_plonk_stubs.a(marlin_plonk_stubs-6a29015a694fc5ab.marlin_plonk_stubs.77bua11a-cgu.15.rcgu.o)
ld: 1 duplicate symbol for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
File "caml_startup", line 1:
Error: Error during linking (exit code 1)

[tizoc]

FYI @mrmr1993 made a PR that should solve this https://github.com/tezedge/ocaml-interop/pull/36

Once all is ready I will merge and make a new ocaml-interop release. @zshipko you will then need to bump the dependency version in ocaml-rs.

[zshipko]

@tizoc Cool, thanks for the update! I will make sure to follow that PR

[mrmr1993]

@zshipko I have a fix that I'm planning to roll out to MinaProtocol/mina when I get the time in the next few days (backed by the changes in the PR above). Is it useful to PR here yet, or would you prefer to wait / implement yourself once it's ready?

[zshipko]

@mrmr1993 A PR would definitely be appreciated! I’m not very familiar with the mina repo, but a link to your changes would also work.

[tizoc]

@mrmr1993 @zshipko after looking at this a bit more, it seems to me that this could be fixed with the addition of a new feature flag no-caml-startup in ocaml-interop (that ocaml-rs will have to support too), that will cause the OCamlRuntime::init_persistent() to be a noop (right now it calls caml_startup).

Then the dune file would have to include a profile for building for dune loading that will enable this feature flag when calling cargo.

[tizoc]

Quick proof of concept here: https://github.com/tezedge/ocaml-interop/pull/37

[tizoc]

Seems to work after adding the same feature flag to ocaml-rs:

> dune utop
──────────────────┬─────────────────────────────────────────────────────────────┬──────────────────
                  │ Welcome to utop version 2.8.0 (using OCaml version 4.11.1)! │                  
                  └─────────────────────────────────────────────────────────────┘                  
Findlib has been successfully loaded. Additional directives:
  #require "package";;      to load a package
  #list;;                   to list the available packages
  #camlp4o;;                to load camlp4 (standard syntax)
  #camlp4r;;                to load camlp4 (revised syntax)
  #predicates "p,q,...";;   to set these predicates
  Topfind.reset();;         to force that packages will be reloaded
  #thread;;                 to enable threads


Type #utop_help for help about using utop.

# Ocaml_rust_starter.hello_world ();;
- : string = "hello, world!"
# 

[tizoc]

Here is the diff to ocaml-rust-starter (not including the overrides to use my modified versions of ocaml-rs and ocaml-interop):

diff --git a/Cargo.toml b/Cargo.toml
index 5b4fa55..a99a1c5 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -10,5 +10,8 @@ crate-type = ["staticlib", "cdylib"]
 [dependencies]
 ocaml = "*"
 
+[features]
+dune = ["ocaml/no-caml-startup"]
+
 # Or use the development version:
 # ocaml = {git = "git://github.com/zshipko/ocaml-rs"}
diff --git a/src/dune b/src/dune
index f9c6767..7c89616 100644
--- a/src/dune
+++ b/src/dune
@@ -3,7 +3,7 @@
  (deps (glob_files *.rs))
  (action
   (progn
-   (run cargo build --target-dir %{project_root}/../../target --release)
+   (run cargo build --target-dir %{project_root}/../../target --release --features "%{env:cargo_feature_flags=}")
    (run sh -c
      "mv %{project_root}/../../target/release/libocaml_rust_starter.so ./dllocaml_rust_starter.so 2> /dev/null || \
       mv %{project_root}/../../target/release/libocaml_rust_starter.dylib ./dllocaml_rust_starter.so")

A better version would use dune's support for profiles and define one specifically for dune.

What do you think of this solution?

[zshipko]

Nice, this looks good and it seems to be working for the intended use case. I think a feature flag is preferable to moving that code out to a separate package.

[tizoc]

This dune at the root of the project + the edits I am about to make to the previous comment make dune utop --profile dune work:

(env
 (dune
  (env-vars (cargo_feature_flags dune))))

[tizoc]

@zshipko yes, splitting the crate has other implications I am not sure about, and it looks like it will only work for anyone not explicitly depending on the runtime initialization, but projects that do would still need to do something extra to make it work.

[tizoc]

I will release ocaml-interop 0.8.5 with this new feature flag tomorrow (or maybe even later today) and open a PR with the relevant changes here and on the ocaml-rust-starter repo (cargo and dune files + changes to README mentioning that).

[mrmr1993]

addition of a new feature flag

@tizoc doesn't this make every consumer of your library need to re-expose this flag, and every downstream consumer decide to turn on all of those flags? I had considered this, but it seems like an exponentially bad nuisance; I would prefer to avoid it.

[tizoc]

@mrmr1993 how about controlling it through an env variable? it was my original intention but I wanted to avoid build.rs. Now that I am thinking more about it, it is probably the best option (small non-invasive change and should avoid the issue you mention).

[mrmr1993]

how about controlling it through an env variable?

The issue is fundamentally the same: if the library has some configurations that expose the unlinkable symbols, then every library using it needs to support both the with and without configurations.

It still makes most sense to me to have the bindings as a separate but related library.

[tizoc]

@mrmr1993 that is not the case, the environment variable will be picked up by ocaml-interop's build script even if crates depending on it change nothing.

This is the build script (note that I removed the feature and use a plain cfg, so in addition to this only runtime.rs changes with the conditional compilation annotation -- although I am still not sure, maybe the feature flag is still useful):

const OCAML_INTEROP_NO_CAML_STARTUP: &'static str = "OCAML_INTEROP_NO_CAML_STARTUP";

fn main() {
    println!("cargo:rerun-if-env-changed={}", OCAML_INTEROP_NO_CAML_STARTUP);
    if  std::env::var(OCAML_INTEROP_NO_CAML_STARTUP).is_ok() {
        println!("cargo:rustc-cfg=no_caml_startup");
    }
}

No changes need to be made to ocaml-rs or any other crate for this to work.

Then optionally this dune file can be added at the root of ocaml-rust-starter (this is the only change, nothing else):

(env
 (utop
  (env-vars (OCAML_INTEROP_NO_CAML_STARTUP true))))

With this change, dune utop --profile utop will compile ocaml-interop without any references to caml_startup. And in reality this file is not even needed, it is just that it gives you something a bit nicer than OCAML_INTEROP_NO_CAML_STARTUP=true dune utop

As for splitting the crate, don't we still have the same issue as with the feature flag? it still leaks. ocaml-rs still needs to be updated to take that into account, for example. Maybe I am missing something but I don't understand why you wouldn't have the same problem as with the feature flag there. Upstream crates would still need to take it into account in the same way ocaml-rs would have to take it into account if ocaml-interop makes that change.

To me splitting the crate, in principle sounds nice, but I think it requires quite a bit more work than https://github.com/tezedge/ocaml-interop/pull/36 (fix tests, update docs, then repeat for ocaml-rs, and more consideration about how it will affect code that works with ocaml-rs and ocaml-interop as they are now). Another aspect is that I don't think we fully understand the problem yet -- we know that caml_startup cannot be there, otherwise things break, but why is that? what about caml_shutdown? should it still be called if caml_startup was disabled? should it be on the runtime crate too? are the other things that we may be missing here?

The solutions I propose don't solve any of this, but are far less invasive.

[tizoc]

Btw I guess that in addition to (or instead of) OCAML_INTEROP_NO_CAML_STARTUP, the UTOP env var could be used, since it is shorter and this is the main (and only) use case for this.

[tizoc]

Ok, here is why caml_startup is not present. When generating an executable to run bytecode (which dune does by generating a custom utop toplevel and using the -output-complete-exe compiler flag), the OCaml compiler generates a main function, but not the startup functions:

https://github.com/ocaml/ocaml/blob/9b0847d3e2065ce0b954ff785282aa9b691ad8ce/bytecomp/bytelink.ml#L513-L566

So another alternative would be to just have the necessary rules in dune (taking advantage of profiles) to also compile and link a .c file with a dummy caml_startup function defined. This would work at the edge project level and no crate needs to know about it.

[mrmr1993]

@mrmr1993 that is not the case, the environment variable will be picked up by ocaml-interop's build script even if crates depending on it change nothing.

Okay, makes sense. tezedge/ocaml-interop#37 seems to work for me with minimal changes.

So another alternative would be to just have the necessary rules in dune (taking advantage of profiles) to also compile and link a .c file with a dummy caml_startup function defined. This would work at the edge project level and no crate needs to know about it.

This seems far scarier to me: code that depends on caml_startup (e.g. if it uses it in combination with caml_shutdown, requiring it to work correctly) will now link but may have different behaviour.

[tizoc]

Code that depends on caml_startup is code that is Rust-driven (that is, the main() is under Rust's control), which is not the case when you load code from utop, or when you are calling Rust code from an OCaml program. In those cases calling caml_startup is not valid, because the runtime is already running, and you will receive a reference to the runtime handle in the functions you expose from Rust to be called by OCaml code anyway (there is no need to create the runtime handles in such cases).

So yes, what you say about the behavior changing in that case is correct, but it is not a function that should ever be called in that situation (it is not even available from a regular utop execution, we are making it available just so that it links).

[tizoc]

Btw, in the case somebody misuses caml_startup, the dummy version could abort with an error to avoid surprises.

[mrmr1993]

Btw, in the case somebody misuses caml_startup, the dummy version could abort with an error to avoid surprises.

This sounds like a good compromise 👍

[tizoc]

Ok, cool. This is what I have in mind for the C-stubs version, but the env-var driven change to how ocaml-interop gets compiled would do the same, but with a panic instead:

#include <stdio.h>
#include <stdlib.h>

// The utop program generated by `dune utop` doesn't contain this symbol,
// causing the linking of ocaml-interop to fail.
// This file provides a dummy version of the function as a workaround.
void caml_startup(char **argv) {
    fputs("ERROR: `caml_startup` cannot be called from a bytecode program\n", stderr);
    fputs("Rust code that is called from an OCaml program should not try to initialize the runtime.", stderr);
    exit(1);
}

[tizoc]

re: runtime crate split, I will open a new issue later so that the idea doesn't get lost. It is a bigger change and requires more thought and work. From the point of view of things being different when the program is Rust-driven from when the program is OCaml-driven, the split makes sense, because the dependencies and requirements are not the same. It is just that I don't think it is as simple as just splitting off caml_startup.

[mrmr1993]

This is what I have in mind for the C-stubs version, but the env-var driven change to how ocaml-interop gets compiled would do the same, but with a panic instead:

Ah, I thought you were talking about going down the feature flag route, and replacing the body of init_persistent with a panic. Can we do that instead?

[tizoc]

Yes, sure, that works too and is actually the easier one to implement (I was experimenting with this pure-dune + stub solution, but turns out it is not that easy to make the dependency on the C stub optional).