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

Linking bug with new pack-semantics for native code in OCaml 3.09 #3825

Closed
vicuna opened this issue Oct 28, 2005 · 7 comments
Closed

Linking bug with new pack-semantics for native code in OCaml 3.09 #3825

vicuna opened this issue Oct 28, 2005 · 7 comments
Labels

Comments

@vicuna
Copy link

vicuna commented Oct 28, 2005

Original bug ID: 3825
Reporter: administrator
Status: closed (set by @damiendoligez on 2005-12-02T10:19:15Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 3.09.0
Category: ~DO NOT USE (was: OCaml general)
Monitored by: cfloyd pzimmer ogunden @alainfrisch

Bug description

Hi,

I haven't yet managed to come up with a small example for the
following problem, but maybe the description can give you a hint of
what's going wrong.

I have built a module "Dfs_lib" using "-pack dfs_lib". The numerous
contained sub-modules (= files) have, of course, been compiled with
"-for-pack Dfs_lib", as required by the new packing semantics. The
creation of this library package went without problems.

Then I tried linking an application with this library. The compiler
complained that it could not find a particular one of the contained
modules ("No implementation provided for the following modules:
File_utils referenced from cli.cmx"). Interestingly, even though a
lot of modules are part of this library, all of which are used, it
only complained about this one.

Even stranger, I found out that it was only refusing to link if I made
use of a particular function contained in this module. Calling some
other functions (haven't tried all - too many) contained in the same
library module did not cause any linking errors. Even calling library
functions that again call the function in the same module did not
cause problems. The linking problem only pops up when the function is
called from the application rather than the library.

Here is a simplified scheme of the structures involved:

module (= packed library) Dfs_lib = struct
module File_utils = struct
val my_open_out : ... -> causes linking errors when used directly
val canonical_path : ... -> does not cause linking problems when called
val sync_my_open_out : ... -> calls "my_open_out" internally; no
problems when called
end

module Dfs_impl = struct
... -> no problems with anything
end
... -> lots of other modules; no problems
end

None of the observed problems occurred with byte-code, i.e. the
application would always link.

Platform: Linux (Fedora 3), i686.

Best regards,
Markus

P.S.:

Btw., thanks a lot for the new warning features in OCaml-3.09! They
have already been very helpful in finding bugs in my code.

--
Markus Mottl http://www.ocaml.info markus.mottl@gmail.com

@vicuna
Copy link
Author

vicuna commented Oct 31, 2005

Comment author: administrator

Je crois observer la même chose que Markus dans son rapport de bug
#3825. L'avantage, c'est que c'est en local:

cd ~frisch/ocaml-cduce309
make -f Makefile.ocamlduce opt

échoue sur:

No implementations provided for the following modules:
Compunit referenced from typing/env.cmx

Or typing/env.ml fait seulement référence à Cduce_types.Compunit
et Compunit a bien été packé dans Cduce_types.

-- Alain

@vicuna
Copy link
Author

vicuna commented Nov 3, 2005

Comment author: administrator

Voici un patch pour le bug 3825. Il s'agit de faire la traduction des
accès à des identificateurs globaux plus tôt, au moment de la traduction
vers le ulambda. Sinon, on stocke dans les .cmx des approximations avec
des Pgetglobal sur des modules qui ne sont plus disponibles, ce qui
empêche de faire la bonne traduction.

Avec le patch, le problème que j'avais est résolu.

diff -aur work/ocaml/asmcomp/clambda.ml /data/ocaml_patched/asmcomp/clambda.ml
--- work/ocaml/asmcomp/clambda.ml 2004-05-26 13:10:27.000000000 +0200
+++ /data/ocaml_patched/asmcomp/clambda.ml 2005-11-03 14:47:17.000000000 +0100
@@ -41,6 +41,7 @@
| Ufor of Ident.t * ulambda * ulambda * direction_flag * ulambda
| Uassign of Ident.t * ulambda
| Usend of meth_kind * ulambda * ulambda * ulambda list

  • | Ugetglobal of string

and ulambda_switch =
{ us_index_consts: int array;
diff -aur work/ocaml/asmcomp/clambda.mli /data/ocaml_patched/asmcomp/clambda.mli
--- work/ocaml/asmcomp/clambda.mli 2004-05-26 13:10:27.000000000 +0200
+++ /data/ocaml_patched/asmcomp/clambda.mli 2005-11-03 14:47:22.000000000 +0100
@@ -41,6 +41,7 @@
| Ufor of Ident.t * ulambda * ulambda * direction_flag * ulambda
| Uassign of Ident.t * ulambda
| Usend of meth_kind * ulambda * ulambda * ulambda list

  • | Ugetglobal of string

and ulambda_switch =
{ us_index_consts: int array;
diff -aur work/ocaml/asmcomp/closure.ml /data/ocaml_patched/asmcomp/closure.ml
--- work/ocaml/asmcomp/closure.ml 2005-10-24 11:05:27.000000000 +0200
+++ /data/ocaml_patched/asmcomp/closure.ml 2005-11-03 15:04:38.000000000 +0100
@@ -64,6 +64,7 @@
| Uassign(id, u) -> id = var || occurs u
| Usend(_, met, obj, args) ->
occurs met || occurs obj || List.exists occurs args

  • | Ugetglobal _ -> false
    and occurs_array a =
    try
    for i = 0 to Array.length a - 1 do
    @@ -130,6 +131,8 @@
    | Uprim(prim, args) ->
    size := !size + prim_size prim args;
    lambda_list_size args
  • | Ugetglobal _ ->
  •   incr size
    | Uswitch(lam, cases) ->
        if Array.length cases.us_actions_consts > 1 then size := !size + 5 ;
        if Array.length cases.us_actions_blocks > 1 then size := !size + 5 ;
    

@@ -172,6 +175,7 @@
Pccall _ | Praise | Poffsetref _ | Pstringsetu | Pstringsets |
Parraysetu _ | Parraysets _ | Pbigarrayset _), _) -> false
| Uprim(p, args) -> List.for_all is_pure_clambda args

  • | Ugetglobal _ -> true
    | _ -> false

(* Simplify primitive operations on integers *)
@@ -323,6 +327,7 @@
Uassign(id', substitute sb u)
| Usend(k, u1, u2, ul) ->
Usend(k, substitute sb u1, substitute sb u2, List.map (substitute sb) ul)

  • | Ugetglobal _ -> ulam

(* Perform an inline expansion *)

@@ -529,7 +534,8 @@
end
| Lprim(Pgetglobal id, []) as lam ->
check_constant_result lam

  •      (Uprim(Pgetglobal id, [])) (Compilenv.global_approx id)
    
  •      (Ugetglobal (Compilenv.symbol_for_global id))
    
  •   (Compilenv.global_approx id)
    
    | Lprim(Pmakeblock(tag, mut) as prim, lams) ->
    let (ulams, approxs) = List.split (List.map (close fenv cenv) lams) in
    (Uprim(prim, ulams),
    @@ -547,7 +553,8 @@
    | Lprim(Psetfield(n, _), [Lprim(Pgetglobal id, []); lam]) ->
    let (ulam, approx) = close fenv cenv lam in
    (!global_approx).(n) <- approx;
  •  (Uprim(Psetfield(n, false), [Uprim(Pgetglobal id, []); ulam]),
    
  •  (Uprim(Psetfield(n, false),
    
  •        [Ugetglobal (Compilenv.symbol_for_global id); ulam]),
       Value_unknown)
    

    | Lprim(p, args) ->
    simplif_prim p (close_list_approx fenv cenv args)
    diff -aur work/ocaml/asmcomp/cmmgen.ml /data/ocaml_patched/asmcomp/cmmgen.ml
    --- work/ocaml/asmcomp/cmmgen.ml 2005-08-01 17:51:09.000000000 +0200
    +++ /data/ocaml_patched/asmcomp/cmmgen.ml 2005-11-03 15:20:40.000000000 +0100
    @@ -850,10 +850,12 @@
    transl_letrec bindings (transl body)

    (* Primitives *)

  • | Ugetglobal id ->

  •   Cconst_symbol id
    

    | Uprim(prim, args) ->
    begin match (simplif_primitive prim, args) with

  •    (Pgetglobal id, []) ->
    
  •      Cconst_symbol (Compilenv.symbol_for_global id)
    
  •    (Pgetglobal id, []) ->
    
  •     assert false
      | (Pmakeblock(tag, mut), []) ->
          transl_constant(Const_block(tag, []))
      | (Pmakeblock(tag, mut), args) ->
    

@vicuna
Copy link
Author

vicuna commented Nov 16, 2005

Comment author: administrator

see also #3833

is this bug fixed?

@vicuna
Copy link
Author

vicuna commented Nov 21, 2005

Comment author: @alainfrisch

As far as I can tell, the bug is still in the current CVS.

-- Alain

@vicuna
Copy link
Author

vicuna commented Dec 1, 2005

Comment author: ogunden

Should this really be marked 'minor'? It's basically a blocker for 3.09 if you use -pack.

@vicuna
Copy link
Author

vicuna commented Dec 2, 2005

Comment author: @damiendoligez

fixed by applying Alain's patch

@vicuna vicuna closed this as completed Dec 2, 2005
@vicuna
Copy link
Author

vicuna commented Dec 11, 2005

Comment author: @xavierleroy

Did some cosmetic changes in the implementation of Alain's patch, but
should still behave the same, i.e. correct interaction between -for-pack
and static approximations. Feel free to re-test and let me know of any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant