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

Producing a cmxs fails with -cc gcc #12284

Open
hyphenrf opened this issue Jun 3, 2023 · 5 comments
Open

Producing a cmxs fails with -cc gcc #12284

hyphenrf opened this issue Jun 3, 2023 · 5 comments
Assignees

Comments

@hyphenrf
Copy link

hyphenrf commented Jun 3, 2023

steps to reproduce

$ touch lib.ml
$ ocamlopt -c lib.ml
$ ocamlopt -shared -o lib.cmxs lib.cmx && echo ok
ok
$ ocamlopt -shared -o lib.cmxs lib.cmx -cc gcc && echo ok
ld: undefined reference to `main'
File "caml_startup", line 1:
Error: Error during linking (exit code 1)

details

ocamlopt -verbose shows gcc called with -shared -o ... as the final step to produce lib.cmxs.
ocamlopt -verbose -cc gcc, however, shows that gcc is called with just -o ....

background info

  • os: nixos
  • ocamlopt -config | grep ^c_compiler returns gcc.
  • bug discovery:
    • overriding the c compiler in a dune project by setting (flags :standard -cc my-cc) in dune-workspace
    • building lib/* modules, which dune generates .cmxs for each of

possible patch

EDIT: this doesn't do the right thing in presence of flexdll. I'll write something more robust if this is indeed considered a bug and someone more familiar with the build system is willing to outline how -cc should behave.

diff --git a/utils/ccomp.ml b/utils/ccomp.ml
index 33a4c9d..566cd00 100644
--- a/utils/ccomp.ml
+++ b/utils/ccomp.ml
@@ -189,13 +189,19 @@ let call_linker mode output_name files extra =
           (quote_files ~response_files:true (remove_Wl files))
           extra
       else
+        let cmd = match mode with
+          | Exe -> Config.mkexe
+          | Dll -> Config.mkdll
+          | MainDll -> Config.mkmaindll
+          | Partial -> assert false
+        in
+        assert (String.starts_with ~prefix:Config.c_compiler cmd);
         Printf.sprintf "%s -o %s %s %s %s %s %s"
-          (match !Clflags.c_compiler, mode with
-          | Some cc, _ -> cc
-          | None, Exe -> Config.mkexe
-          | None, Dll -> Config.mkdll
-          | None, MainDll -> Config.mkmaindll
-          | None, Partial -> assert false
+          (match !Clflags.c_compiler with
+          | None -> cmd
+          | Some cc ->
+            let old_cc = Config.c_compiler in
+            cc ^ String.(sub cmd (length old_cc) (length cmd - length old_cc))
           )
           (Filename.quote output_name)
           ""  (*(Clflags.std_include_flag "-I")*)
@dra27
Copy link
Member

dra27 commented Jun 3, 2023

TL;DR I don't think this is a bug, but there's certainly a patch which could be made to facilitate this...!

I think that what we have here is a mismatch between what -cc sounds like it should do (from its name) and what it actually does (from the help text description). From its name, it sounds like it replaces the C compiler executable name, but from its help text it really does need to be the entire command. There's some mechanics in utils/config.generated.ml.in which does the command substitution specifically for flexlink which could be applied here, but I don't think that's a good idea (separately, the complex mechanics will be removed as part of #12278).

As part of #12278, I looked quite closely at this mechanism, but I think that changes to it are likely to be very slow to get in, because it's a very difficult part of the driver to test the impact of changes. Also, I don't think we can change the interpretation of -cc, so it would mean adding a new flag completely.

For your use-case would it be sufficient to recreate the entire command being passed to -cc instead? The output of ocamlc -config also includes ocamlc_cflags and ocamlc_cppflags. In theory, (flags :standard -cc "my-cc %{dune-fu-for-ocamlc_cflags} %{dune-fu-for-ocaml-cppflags}") should be correct for linking an executable. However, we don't write the value of $mkdll_flags from configure anywhere where it's extractable, so you can't get the flags needed for making a DLL directly from ocamlc -config. I think changing that - to include extra output in ocamlc -config (say ocamlc_shared_cflags or some such) is a perfectly viable patch (in fact, I have a vague feeling @shindere has mentioned this in the past). From Dune's perspective, it is possible for old/current versions of OCaml to extract this information from Makefile.config - just parse the MKDLL line and eliminate all GNU make macros from it (MKDLL is a templated call to $(CC) followed by the actual flags needed followed by either $(OC_DLL_FLAGS) $(LDFLAGS) or similar macro mess for flexlink, all of which can be eliminated by just removing balanced parentheses following a $). I don't know what Dune does today, but in the past it has had to parse OCaml's installed configuration files to glean some config information, so hopefully this isn't too hideous a suggestion!

Another benefit to simply adding output to ocamlc -config vs adding flags to OCaml is that you completely sidestep the flexlink complexity - because whatever you're doing here for nix I assume cannot possibly involve Windows? Or at least, if it did, it'd be the responsibility of the dune files to work out the incantation (see the horrors in configure.ac for why avoiding this is a good idea! 🙂)

@hyphenrf
Copy link
Author

hyphenrf commented Jun 3, 2023

It was the help text that gave me the wrong impression:
-cc <command> Use <command> as the C compiler and linker

I'm happy to implement the low-impact patch on ocamlc -config that you mentioned. It won't be solving my problem but it sounds like a good step for dune to later be able to cooperate more easily with the driver. (cc @rgrinberg: does this look like it'd be a helpful change for dune?).
An easier way than what you outlined to currently recover the cc command for shared libs is to create a dummy lib and run ocamlopt -verbose then parse the output.. unless there's a caveat I'm not aware of in doing it like this.

I have to be clear on who I am to this, I'm just a user trying to override the C compiler OCaml uses to test features of other compilers. I'm not a dune contributor or a distro maintainer etc..
I'm not aware of a dune mechanism which would do this generally without significant efforts like the ones you outlined. I'm not even aware of a dune mechanism which disables generating cmxs for native libraries to sidestep the link flags problem—I had to go with (modes byte). That could just be because I'm not a well-versed dune user!
That being said, I could just specialize the flags field on a per-component basis and hardcode the shared libraries flags manually for my local projects. So it's not yet a big problem for me. This obviously won't be adequate when I can't control the build machine.. e.g. if I'm publishing a library.

As for the -cc behavior, I realize it's technically a breaking change what my patch does, for anyone who used -cc to override the default command params supplied by OCaml, or had to resupply the params after facing this issue. This is only breaking if such behavior was supported to begin with. And I'm guessing it's only breaking expectations, not breaking compilation, as all the C compilers I'm aware of allow conflicting/repeated args, with the last supplied of which being taken as canonical.
dune could even be the target for an improvement where it supplies different :standard flags depending on what it's doing, regardless of -cc overrides which may potentially break compilation like this issue shows.

It's just that the current (well, for the last, like, 15 years) use of -cc strikes me as strange.. -cc being used to override just the compiler makes more sense to me, especially when the c flags change depending on the nature of what the ocamlopt driver is compiling.. I'd define $ocaml='ocamlopt -cc my-cc' once and it'd work wherever a bare ocamlopt call would, but if this is the intended behavior, and no-one seems to have complained about it for so long, then it's on me to adapt to it :P

@shindere
Copy link
Contributor

shindere commented Jun 5, 2023 via email

@lthls
Copy link
Contributor

lthls commented Nov 2, 2023

Cross referencing ocaml/ocamlfind#69, which is dealing with the same issue.

@ChenQi1989
Copy link

@shindere IMHO, there are two variables used by ocaml, MKEXE_EXP and MKDLL, but there's only one option for overriding (-cc).
If there are two commands in use, it needs two options to override them. So apart from '-cc', we may need a new option, e.g., '-mkdll' to cover such case. Does this make sense?

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

No branches or pull requests

5 participants