Skip to content

Commit 32846d5

Browse files
authored
removes the old unit tracking module, fixes the modern one (#1310)
The old unit tracking module, which is used what compilation units are loaded to the core program, is no longer used since we phased out compilers that required it. However, the modern unit that relies on OCaml runtime to track the units was also broken (in all versions of OCaml since the introduction of this facility in 4.08.0 till the latest 4.12.0) as it wasn't properly initialized. This PR adds a kludge that ensures proper initialization. Note, the bug didn't manifest in BAP as we have several layers of protection, but it is still better to have it working properly.
1 parent acb26d6 commit 32846d5

File tree

6 files changed

+67
-156
lines changed

6 files changed

+67
-156
lines changed

lib/bap_plugins/bap_plugins_units.ml

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,39 @@
11
open Core_kernel
22

3-
open Bap_plugins_units_intf
4-
5-
[%%if ocaml_version < (4,08,0)]
6-
include Bap_plugins_units_fallback
7-
[%%else]
83
let name = "dynlink"
94

5+
type reason = [
6+
| `In_core
7+
| `Provided_by of string
8+
| `Requested_by of string
9+
]
10+
1011
let units : reason String.Table.t = String.Table.create ()
1112

13+
(* see https://github.com/ocaml/ocaml/issues/9338
14+
this ugly clutch could only be removed after we phase out
15+
OCaml 4.11, as the fix (https://github.com/ocaml/ocaml/pull/9790)
16+
was only merged in 4.12 *)
17+
let init_dynlink () =
18+
try Dynlink.loadfile "" with _ -> ()
19+
1220
let copy_units_from_dynlink () =
21+
init_dynlink ();
1322
Dynlink.all_units () |>
1423
List.iter ~f:(fun unit -> Hashtbl.add_exn units unit `In_core)
1524

1625
let init () = copy_units_from_dynlink ()
1726
let list () = Hashtbl.keys units
18-
let record name reason = Hashtbl.add_exn units name reason
27+
let record name reason = match Hashtbl.add units name reason with
28+
| `Ok -> ()
29+
| `Duplicate ->
30+
failwithf "bap-plugins: internal error - \
31+
the unit %s is already loaded" name ()
32+
1933
let lookup = Hashtbl.find units
2034
let handle_error name reason = function
2135
| Dynlink.Module_already_loaded _ ->
2236
Hashtbl.set units name reason;
2337
Ok ()
2438
| other ->
2539
Or_error.error_string (Dynlink.error_message other)
26-
[%%endif]

lib/bap_plugins/bap_plugins_units.mli

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,46 @@
1-
include Bap_plugins_units_intf.S
1+
(** Internal module. *)
2+
3+
open Core_kernel
4+
5+
type reason = [
6+
| `In_core
7+
| `Provided_by of string
8+
| `Requested_by of string
9+
]
10+
11+
12+
13+
(** the name of the selected backend.
14+
15+
Currently, it should be [findlib] or [dynlink], and is
16+
selected at configuration time via `./configure --plugins-backend`.
17+
*)
18+
val name : string
19+
20+
(** initializes the unit system.
21+
22+
May fail if the selected backend is unable to provide safe
23+
operation.
24+
25+
Could be only called once per program run.
26+
*)
27+
val init : unit -> unit
28+
29+
30+
(** [list ()] enumerates all currently linked modules. *)
31+
val list : unit -> string list
32+
33+
34+
(** [record name reason] records unit [name] as well as the
35+
reason, why it is linked.
36+
37+
pre: a unit with such name is not linked.
38+
*)
39+
val record : string -> reason -> unit
40+
41+
42+
(** [lookup name] checks if a unit with the given [name] is linked,
43+
and returns a reason why it was linked. *)
44+
val lookup : string -> reason option
45+
46+
val handle_error : string -> reason -> Dynlink.error -> unit Or_error.t

lib/bap_plugins/bap_plugins_units_fallback.ml

Lines changed: 0 additions & 63 deletions
This file was deleted.

lib/bap_plugins/bap_plugins_units_fallback.mli

Lines changed: 0 additions & 1 deletion
This file was deleted.

lib/bap_plugins/bap_plugins_units_intf.ml

Lines changed: 0 additions & 81 deletions
This file was deleted.

oasis/plugins

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ Library bap_plugins
88
FindLibName: bap-plugins
99
Modules: Bap_plugins
1010
InternalModules: Bap_plugins_config,
11-
Bap_plugins_units,
12-
Bap_plugins_units_fallback,
13-
Bap_plugins_units_intf
11+
Bap_plugins_units
1412
BuildDepends: core_kernel, dynlink, fileutils, findlib, bap-bundle, bap-future,
1513
uri, ppx_bap

0 commit comments

Comments
 (0)