Skip to content

Commit

Permalink
Protect LTCG placeholders with Sys.opaque_identity (#3599)
Browse files Browse the repository at this point in the history
This ensures that they are not inlined when using flambda.
This problem is only present in OCaml 4.10.0, so it is fine not to patch
on 4.02 where `Sys.opaque_identity` is not available.
See discussion in #1930.

Signed-off-by: Etienne Millon <[email protected]>
  • Loading branch information
emillon authored Jul 6, 2020
1 parent 8d7e18a commit 2d8398f
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ next
- `$ dune utop` no longer tries to load optional libraries that are unavailable
(#3612, fixes #3188, @anuragsoni)

- Fix dune-build-info on 4.10.0+flambda (#3599, @emillon, @jeremiedimino).

2.6.1 (02/07/2020)
------------------

Expand Down
6 changes: 3 additions & 3 deletions otherlibs/build-info/test/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ Check what the generated build info module looks like:
None
[@@inline never]

let p1 = eval "%%DUNE_PLACEHOLDER:64:vcs-describe:1:a%%%%%%%%%%%%%%%%%%%%%%%%%%"
let p2 = eval "%%DUNE_PLACEHOLDER:64:vcs-describe:1:b%%%%%%%%%%%%%%%%%%%%%%%%%%"
let p0 = eval "%%DUNE_PLACEHOLDER:64:vcs-describe:1:c%%%%%%%%%%%%%%%%%%%%%%%%%%"
let p1 = eval (Sys.opaque_identity "%%DUNE_PLACEHOLDER:64:vcs-describe:1:a%%%%%%%%%%%%%%%%%%%%%%%%%%")
let p2 = eval (Sys.opaque_identity "%%DUNE_PLACEHOLDER:64:vcs-describe:1:b%%%%%%%%%%%%%%%%%%%%%%%%%%")
let p0 = eval (Sys.opaque_identity "%%DUNE_PLACEHOLDER:64:vcs-describe:1:c%%%%%%%%%%%%%%%%%%%%%%%%%%")

let version = p0

Expand Down
10 changes: 9 additions & 1 deletion src/dune/link_time_code_gen.ml
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ let build_info_code cctx ~libs ~api_version =
in
placeholder p ) ))
in
let context = CC.context cctx in
let ocaml_version = Ocaml_version.of_ocaml_config context.ocaml_config in
let buf = Buffer.create 1024 in
(* Parse the replacement format described in [artifact_substitution.ml]. *)
pr buf "let eval s =";
Expand All @@ -159,8 +161,14 @@ let build_info_code cctx ~libs ~api_version =
pr buf " None";
pr buf "[@@inline never]";
pr buf "";
let fmt_eval : _ format6 =
if Ocaml_version.has_sys_opaque_identity ocaml_version then
"let %s = eval (Sys.opaque_identity %S)"
else
"let %s = eval %S"
in
Path.Source.Map.iteri !placeholders ~f:(fun path var ->
pr buf "let %s = eval %S" var
pr buf fmt_eval var
(Artifact_substitution.encode ~min_len:64 (Vcs_describe path)));
if not (Path.Source.Map.is_empty !placeholders) then pr buf "";
pr buf "let version = %s" version;
Expand Down
2 changes: 2 additions & 0 deletions src/dune/ocaml_version.ml
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,5 @@ let custom_or_output_complete_exe version =
"-custom"

let ocamlopt_always_calls_library_linker version = version < (4, 12, 0)

let has_sys_opaque_identity version = version >= (4, 3, 0)
3 changes: 3 additions & 0 deletions src/dune/ocaml_version.mli
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,6 @@ val custom_or_output_complete_exe : t -> string

(** ocamlopt -a always calls the native C linker, even for empty archives *)
val ocamlopt_always_calls_library_linker : t -> bool

(** Whether [Sys.opaque_identity] is in the standard library *)
val has_sys_opaque_identity : t -> bool

0 comments on commit 2d8398f

Please sign in to comment.