From 0df27122243b9414e4f0c994966c380ee345c86c Mon Sep 17 00:00:00 2001 From: Xavier Leroy Date: Tue, 19 Nov 2024 14:09:27 +0100 Subject: [PATCH 1/3] Replace `C2C.atom_is_extern` by more robust criterion `C2C.atom_is_external` 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 --- aarch64/extractionMachdep.v | 2 +- cfrontend/C2C.ml | 17 ++++++++++++----- x86/extractionMachdep.v | 4 ++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/aarch64/extractionMachdep.v b/aarch64/extractionMachdep.v index ae0006bc38..78eb363fba 100644 --- a/aarch64/extractionMachdep.v +++ b/aarch64/extractionMachdep.v @@ -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 *) diff --git a/cfrontend/C2C.ml b/cfrontend/C2C.ml index f44eb63812..9591b6b7ab 100644 --- a/cfrontend/C2C.ml +++ b/cfrontend/C2C.ml @@ -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 *) @@ -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 @@ -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]; @@ -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))); @@ -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; @@ -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]; diff --git a/x86/extractionMachdep.v b/x86/extractionMachdep.v index 26a3f0a74e..d1a29c729c 100644 --- a/x86/extractionMachdep.v +++ b/x86/extractionMachdep.v @@ -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)". From 70e85ccc242d589e774be16787aef18c13e4be50 Mon Sep 17 00:00:00 2001 From: Xavier Leroy Date: Thu, 21 Nov 2024 11:54:02 +0100 Subject: [PATCH 2/3] Further refine the `C2C.atom_is_external` criterion in "no-common" mode In `-fno-common` mode, a global variable declaration such as `int x;`, with default storage class, acts like a definition: `x` cannot be defined in a DLL. --- cfrontend/C2C.ml | 1 + 1 file changed, 1 insertion(+) diff --git a/cfrontend/C2C.ml b/cfrontend/C2C.ml index 9591b6b7ab..4f7363b01c 100644 --- a/cfrontend/C2C.ml +++ b/cfrontend/C2C.ml @@ -57,6 +57,7 @@ let atom_is_external a = match Hashtbl.find decl_atom a with | { a_defined = true } -> false | { a_storage = C.Storage_static } -> false + | { a_storage = C.Storage_default; a_size = Some _ } -> !Clflags.option_fcommon | _ -> true | exception Not_found -> true From ff0875d62350192945e28b6ef1e3a8e2be810a35 Mon Sep 17 00:00:00 2001 From: Xavier Leroy Date: Fri, 22 Nov 2024 10:25:37 +0100 Subject: [PATCH 3/3] Regression test for #537 --- test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test b/test index bbfd6c8f3d..3bd105123f 160000 --- a/test +++ b/test @@ -1 +1 @@ -Subproject commit bbfd6c8f3de5d8e38988bd7f27af3a151e239bb4 +Subproject commit 3bd105123f38de72ea0455b7c06e71612d5df657