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

Fix #8816 (Dynlink/packing issue in bytecode with 4.08) #8818

Merged
merged 3 commits into from Jul 23, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -388,6 +388,10 @@ OCaml 4.08 maintenance branch:
- #8769, #8770: Fix assertion failure with -pack
(Leo White, review by Gabriel Scherer, report by Fabian @copy)

- #8816, #8818: fix loading of packed modules with Dynlink (regression in
#2176).
(Leo White, report by Andre Maroneze, review by Gabriel Scherer)

OCaml 4.08.0 (13 June 2019)
---------------------------

@@ -39,7 +39,11 @@ module Bytecode = struct
@ Symtable.required_globals t.cu_reloc
in
let required =
List.filter (fun id -> not (Ident.is_predef id)) required
List.filter
(fun id ->
not (Ident.is_predef id)
&& not (String.contains (Ident.name id) '.'))

This comment has been minimized.

Copy link
@gasche

gasche Jul 19, 2019

Member

Without knowing the context, this looks a bit strange. Where is the code that creates predefined identifiers with . in their name?

This comment has been minimized.

Copy link
@gasche

gasche Jul 19, 2019

Member

(Well I guess those are the Ident.create_persistent (packagename ^ "." ^ name) calls in bytecomp/bytepackager.ml.)

This comment has been minimized.

Copy link
@lpw25

lpw25 Jul 19, 2019

Author Contributor

Yeah, and you'll also notice a use of String.contains ... '.' in that file as well. It's not pleasant, but there are a couple of PRs coming down the pipeline that will tidy up this stuff so I'm not in a hurry to fix it right now.

required
in
List.map
(fun ident -> Ident.name ident, None)
@@ -0,0 +1 @@
let nums = Sys.opaque_identity [1; 2; 3; 4; 5]
@@ -0,0 +1 @@
let () = List.iter (fun i -> print_endline (string_of_int i)) A.nums
@@ -0,0 +1,5 @@
1
2
3
4
5
@@ -0,0 +1,64 @@
(* TEST
include dynlink
libraries = ""
files = "a.ml b.ml loader.ml"
* shared-libraries
** setup-ocamlc.byte-build-env
*** ocamlc.byte
flags = "-for-pack Packed"
module = "a.ml"
*** ocamlc.byte
flags = "-for-pack Packed"
module = "b.ml"
*** ocamlc.byte
program = "packed.cmo"
flags = "-pack"
all_modules = "a.cmo b.cmo"
*** ocamlc.byte
program = "${test_build_directory}/loader.byte"
flags = "-linkall"
include ocamlcommon
libraries += "dynlink"
all_modules = "loader.ml"
**** run
arguments = "packed.cmo"
exit_status = "0"
***** check-program-output
reference = "${test_source_directory}/byte.reference"
** native-dynlink
*** setup-ocamlopt.byte-build-env
**** ocamlopt.byte
flags = "-for-pack Packed"
module = "a.ml"
**** ocamlopt.byte
flags = "-for-pack Packed"
module = "b.ml"
**** ocamlopt.byte
program = "packed.cmx"
flags = "-pack"
all_modules = "a.cmx b.cmx"
**** ocamlopt.byte
program = "plugin.cmxs"
flags = "-shared"
all_modules = "packed.cmx"
**** ocamlopt.byte
program = "${test_build_directory}/loader.exe"
flags = "-linkall"
include ocamlcommon
libraries += "dynlink"
all_modules = "loader.ml"
***** run
arguments = "plugin.cmxs"
exit_status = "0"
****** check-program-output
reference = "${test_source_directory}/native.reference"
*)
let () =
try
Dynlink.loadfile Sys.argv.(1)
with
| Dynlink.Error error ->
prerr_endline (Dynlink.error_message error)
@@ -0,0 +1,5 @@
1
2
3
4
5
@@ -0,0 +1 @@
loader.ml
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.