[OCamlc] fixes output-complete-object #1332

bobot · 2017-09-13T15:35:21Z

When trying to add support for --output-complete-obj for jbuilder [ocaml/dune#23], I was not able to make --output-complete-obj work in bytecode mode. ocamlc complains that the temporary files it just created exist. This PR fixes this option and add tests for it.

The support for --output-complete-obj has been added in ed60dec and 19d3bcc

ocamlc complained that the temporary files it just created existed. since ed60dec and 19d3bcc

for output_obj tests

lpw25 · 2017-09-14T15:57:57Z

bytecomp/bytelink.ml

    try
      link_bytecode_as_c ppf tolink c_file;
      if not (Filename.check_suffix output_name ".c") then begin
        temps := c_file :: !temps;
-        if Ccomp.compile_file c_file <> 0 then
+        (* -DCAML_INTERNALS because the .c use startup.h *)
+        if Ccomp.compile_file ~output:obj_file ~opt:"-DCAML_INTERNALS" c_file <> 0 then


I couldn't obviously see an equivalent use of -DCAML_INTERNALS in asmlink.ml, is that expected?

Since asmlink goes directly to assembly I think so. But indeed CAML_INTERNALS is not defined in this way: all the other C files that use startup.h just define CAML_INTERNALS directly in the file. It is now fixed.

lpw25 · 2017-09-14T15:59:29Z

utils/ccomp.ml

@@ -87,6 +87,8 @@ let compile_file name =
                  then (Config.ocamlopt_cflags, Config.ocamlopt_cppflags)
                  else (Config.ocamlc_cflags, Config.ocamlc_cppflags) in
              (String.concat " " [Config.c_compiler; cflags; cppflags]))
+         (match output with | None -> "" | Some o -> "-o " ^ o)
+         opt


Have you checked that all the C compilers support the -o option? (Sun compiler, Intel compiler, MSVC, gcc, clang, pcc)

I didn't, and it is not the case. It is fixed by adding OUTPUTOBJ to config.ml and using it.

in the generated file instead of in the command line

damiendoligez · 2017-09-18T13:03:49Z

Note: needs more work (fails on most architectures, including linux/amd64). I've disabled the test for the moment (commit 47a6e48).

bobot · 2017-09-18T13:18:12Z

@damiendoligez thank you for the news. Do you still have any log of the failures?

damiendoligez · 2017-09-18T13:32:18Z

@bobot I'll get back to you in a few days.

Oh, and it needs documentation too. I can't find any trace of it in the manual or even in the man pages.

bobot · 2017-09-18T15:58:29Z

Started #1351 for the doc and more importantly the possible deprecation of output-obj.

gasche · 2017-11-03T10:35:22Z

Changes

@@ -180,6 +180,8 @@ Working version

 ### Compiler user-interface and warnings:

+- GPR##1332: fix ocamlc handling of "-output-complete-obj"


Worst Changes entry ever! :-)

dbuenzli · 2017-11-03T10:45:54Z

Started #1351 for the doc and more importantly the possible deprecation of output-obj.

Since I see this. As I hope #1351 made clear, output-obj should not be deprecated.

damiendoligez · 2017-11-03T23:48:08Z

@bobot "a few" days have passed... When I enable the test I get this:

4.06/testsuite $ make one DIR=tests/output_obj
Running tests from 'tests/output_obj' ...
 ... testing 'test.ml' with native => failed
+ /Users/doligez/o/4.06/testsuite/../byterun/ocamlrun /Users/doligez/o/4.06/testsuite/../ocamlopt -nostdlib -I /Users/doligez/o/4.06/testsuite/../stdlib -w a -output-complete-obj -o test.ml.exe.o test.ml
+ gcc -O2 -fno-strict-aliasing -fwrapv -Wall -Werror -D_FILE_OFFSET_BITS=64 -D_REENTRANT -DCAML_NAME_SPACE -Wl,-no_compact_unwind -I/Users/doligez/o/4.06/testsuite/../byterun -o test.ml_stub test.ml.exe.o test.ml_stub.c
Undefined symbols for architecture x86_64:
  "_caml_code_area_end", referenced from:
      _segv_handler in test.ml.exe.o
  "_caml_code_area_start", referenced from:
      _segv_handler in test.ml.exe.o
  "_caml_startup", referenced from:
      _main in test-af8fba.o
     (maybe you meant: _caml_startup__frametable, _caml_startup__data_begin , _caml_startup_aux , _caml_startup__code_end , _caml_startup__code_begin , _caml_startup__data_end )
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
 ... testing 'test.ml' with byte => passed

[OCamlc] fixes output_complete_object

8afe629

ocamlc complained that the temporary files it just created existed. since ed60dec and 19d3bcc

bobot changed the title ~~[OCamlc] fixes output_complete_object~~ [OCamlc] fixes output-complete-object Sep 13, 2017

bobot added 2 commits September 14, 2017 09:53

[Tests] Print message in case of errors

122ec3c

for output_obj tests

[Tests] output_obj unhardcord linking options

2c682c4

mshinwell requested a review from lpw25 September 14, 2017 12:58

bobot added 2 commits September 14, 2017 17:17

[Changes] adds change note for GPR#1332

8f2172a

[Tests] try to fix output-obj tests on windows

e614ec5

lpw25 reviewed Sep 14, 2017

View reviewed changes

mshinwell added consider-for-release bug labels Sep 15, 2017

bobot added 2 commits September 15, 2017 21:28

[OCamlc] use OUTPUTOBJ and move CAML_INTERNAL

1343658

in the generated file instead of in the command line

[Tests] add gitignore for output_obj files

626a297

lpw25 merged commit e1fd8d5 into ocaml:trunk Sep 16, 2017

gasche reviewed Nov 3, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OCamlc] fixes output-complete-object #1332

[OCamlc] fixes output-complete-object #1332

bobot commented Sep 13, 2017 •

edited

lpw25 Sep 14, 2017

bobot Sep 15, 2017

lpw25 Sep 14, 2017

bobot Sep 15, 2017

damiendoligez commented Sep 18, 2017

bobot commented Sep 18, 2017

damiendoligez commented Sep 18, 2017

bobot commented Sep 18, 2017

gasche Nov 3, 2017

dbuenzli commented Nov 3, 2017 •

edited

damiendoligez commented Nov 3, 2017

		@@ -180,6 +180,8 @@ Working version

		### Compiler user-interface and warnings:

		- GPR##1332: fix ocamlc handling of "-output-complete-obj"

[OCamlc] fixes output-complete-object #1332

[OCamlc] fixes output-complete-object #1332

Conversation

bobot commented Sep 13, 2017 • edited

lpw25 Sep 14, 2017

Choose a reason for hiding this comment

bobot Sep 15, 2017

Choose a reason for hiding this comment

lpw25 Sep 14, 2017

Choose a reason for hiding this comment

bobot Sep 15, 2017

Choose a reason for hiding this comment

damiendoligez commented Sep 18, 2017

bobot commented Sep 18, 2017

damiendoligez commented Sep 18, 2017

bobot commented Sep 18, 2017

gasche Nov 3, 2017

Choose a reason for hiding this comment

dbuenzli commented Nov 3, 2017 • edited

damiendoligez commented Nov 3, 2017

bobot commented Sep 13, 2017 •

edited

dbuenzli commented Nov 3, 2017 •

edited