Skip to content

Commit

Permalink
Replace C2C.atom_is_extern by more robust criterion `C2C.atom_is_ex…
Browse files Browse the repository at this point in the history
…ternal`

This predicate is used in macOS and Cygwin ports to determine when a global symbol may be defined in a shared library, and therefore must be referenced through the GOT.

The previous criterion was just "is it declared `extern`"?  However, this misses some cases, e.g. "common" declaration.

The new criterion is "it is not declared `static` and not defined in the current compilation unit", which should be a lot more robust.

Fixes: #537
  • Loading branch information
xavierleroy committed Nov 19, 2024
1 parent 044cfbc commit 0df2712
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
2 changes: 1 addition & 1 deletion aarch64/extractionMachdep.v
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Extract Constant Archi.abi =>

Extract Constant SelectOp.symbol_is_relocatable =>
"match Configuration.system with
| ""macos"" -> C2C.atom_is_extern
| ""macos"" -> C2C.atom_is_external
| _ -> (fun _ -> false)".

(* Asm *)
Expand Down
17 changes: 12 additions & 5 deletions cfrontend/C2C.ml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type inline_status =

type atom_info =
{ a_storage: C.storage; (* storage class *)
a_defined: bool; (* defined in the current comp. unit? *)
a_size: int64 option; (* size in bytes *)
a_alignment: int option; (* alignment *)
a_sections: Sections.section_name list; (* in which section to put it *)
Expand All @@ -51,11 +52,13 @@ let atom_is_static a =
with Not_found ->
false

let atom_is_extern a =
try
(Hashtbl.find decl_atom a).a_storage = C.Storage_extern
with Not_found ->
false
(* Is it possible for symbol [a] to be defined in a DLL? *)
let atom_is_external a =
match Hashtbl.find decl_atom a with
| { a_defined = true } -> false
| { a_storage = C.Storage_static } -> false
| _ -> true
| exception Not_found -> true

let atom_alignof a =
try
Expand Down Expand Up @@ -588,6 +591,7 @@ let name_for_string_literal s =
let mergeable = if is_C_string s then 1 else 0 in
Hashtbl.add decl_atom id
{ a_storage = C.Storage_static;
a_defined = true;
a_alignment = Some 1;
a_size = Some (Int64.of_int (String.length s + 1));
a_sections = [Sections.for_stringlit mergeable];
Expand Down Expand Up @@ -623,6 +627,7 @@ let name_for_wide_string_literal s ik =
let mergeable = if is_C_wide_string s then wchar_size else 0 in
Hashtbl.add decl_atom id
{ a_storage = C.Storage_static;
a_defined = true;
a_alignment = Some wchar_size;
a_size = Some (Int64.(mul (of_int (List.length s + 1))
(of_int wchar_size)));
Expand Down Expand Up @@ -1156,6 +1161,7 @@ let convertFundef loc env fd =
Debug.atom_global fd.fd_name id';
Hashtbl.add decl_atom id'
{ a_storage = fd.fd_storage;
a_defined = true;
a_alignment = None;
a_size = None;
a_sections = Sections.for_function env loc id' fd.fd_attrib;
Expand Down Expand Up @@ -1247,6 +1253,7 @@ let convertGlobvar loc env (sto, id, ty, optinit) =
error "'%s' has incomplete type" id.name;
Hashtbl.add decl_atom id'
{ a_storage = sto;
a_defined = optinit <> None;
a_alignment = Some (Z.to_int al);
a_size = Some (Z.to_int64 sz);
a_sections = [section];
Expand Down
4 changes: 2 additions & 2 deletions x86/extractionMachdep.v
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ Extract Constant Archi.win64 =>

Extract Constant SelectOp.symbol_is_external =>
"match Configuration.system with
| ""macos"" -> C2C.atom_is_extern
| ""cygwin"" when Archi.ptr64 -> C2C.atom_is_extern
| ""macos"" -> C2C.atom_is_external
| ""cygwin"" when Archi.ptr64 -> C2C.atom_is_external
| _ -> (fun _ -> false)".

0 comments on commit 0df2712

Please sign in to comment.