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 -big- files causes failure in ocamlc #5957

Closed
vicuna opened this Issue Mar 21, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

vicuna commented Mar 21, 2013

Original bug ID: 5957
Reporter: chetsky@gmail.com
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2015-12-11T18:18:37Z)
Resolution: fixed
Priority: normal
Severity: minor
Platform: x86
OS: Ubuntu Linux
OS Version: 12.04
Version: 4.00.1
Fixed in version: 4.01.0+dev
Category: back end (clambda to assembly)
Duplicate of: #5920
Related to: #7335

Bug description

When linking big .cmo files (like created by -pack in the Jane St core lib), ocamlc will fail with an Invalid_argument("String.create") -- it's trying to create a string of length 43Mbytes.

Steps to reproduce

(can repro with an opam installation pretty easily):

(1) ocaml4.00.1 + opam (1.0.0)
(2) opam install -y ocamlfind core

(3) compile the below file "yadda_main.ml" with:

% camlfind ocamlc -verbose -package core,threads.posix -linkpkg -thread -
custom -g -o yadda yadda_main.ml

================================================================
module Mutex = Core.Std.Mutex

let main() =
let m = Mutex.create() in
Mutex.lock m ;
(try
if Mutex.try_lock m then print_string "Free" else print_string "Held by
other"
with _ -> print_string "Held by me") ;
print_newline() ;
flush stdout ;
Mutex.unlock m
;;

main() ;;

Additional information

I built a version of ocamlc with "-g" and got a backtrace. I believe that the problem is with Misc.input_bytes, as it is used to read debug info. I am sure that this is not the right fix, but this -does- work (make world, make world.opt, make bootstrap, install, then rerun testcase).

================================================================
diff -Bwiu --recursive ../../Caml4.00/src.OLD/ocaml-4.00.1/bytecomp/bytelink.ml
ocaml-4.00.1/bytecomp/bytelink.ml
--- ../../Caml4.00/src.OLD/ocaml-4.00.1/bytecomp/bytelink.ml 2012-04-16
08:27:42.000000000 -0700
+++ ocaml-4.00.1/bytecomp/bytelink.ml 2013-03-20 20:59:48.459111534 -0700
@@ -188,7 +188,7 @@

(* Record compilation events *)

-let debug_info = ref ([] : (int * string) list)
+let debug_info = ref ([] : (int * string list) list)

(* Link in a compilation unit *)

@@ -199,7 +199,7 @@
Symtable.patch_object code_block compunit.cu_reloc;
if !Clflags.debug && compunit.cu_debug > 0 then begin
seek_in inchan compunit.cu_debug;

  • let buffer = input_bytes inchan compunit.cu_debugsize in
  • let buffer = input_big_bytes inchan compunit.cu_debugsize in
    debug_info := (currpos_fun(), buffer) :: !debug_info
    end;
    output_fun code_block;
    @@ -255,7 +255,7 @@
    let output_debug_info oc =
    output_binary_int oc (List.length !debug_info);
    List.iter
  • (fun (ofs, evl) -> output_binary_int oc ofs; output_string oc evl)
  • (fun (ofs, evll) -> output_binary_int oc ofs; (List.iter (output_string
    oc) evll))
    !debug_info;
    debug_info := []

diff -Bwiu --recursive ../../Caml4.00/src.OLD/ocaml-4.00.1/utils/misc.ml
ocaml-4.00.1/utils/misc.ml
--- ../../Caml4.00/src.OLD/ocaml-4.00.1/utils/misc.ml 2012-07-30
11:59:07.000000000 -0700
+++ ocaml-4.00.1/utils/misc.ml 2013-03-20 21:05:02.207099022 -0700
@@ -160,6 +160,25 @@
result
;;

+let input_big_bytes ic n =

  • let _blocksiz = 65536 in
  • let input0 n =
  • assert (n <> 0) ;
  • let toread = min n _blocksiz in
  • let result = String.create toread in
  • let nread = input ic result 0 toread in
  •  assert (nread <> 0) ;
    
  •  if nread <> toread then
    
  •   String.sub result 0 nread
    
  •  else result in
    
  • let rec inrec acc n =
  • if n = 0 then List.rev acc
  • else
  •  let buf = input0 n in
    
  •   inrec (buf::acc) (n-(String.length buf)) in
    
  • inrec [] n

(* Integer operations *)

let rec log2 n =
diff -Bwiu --recursive ../../Caml4.00/src.OLD/ocaml-4.00.1/utils/misc.mli
ocaml-4.00.1/utils/misc.mli
--- ../../Caml4.00/src.OLD/ocaml-4.00.1/utils/misc.mli 2012-05-30
06:29:48.000000000 -0700
+++ ocaml-4.00.1/utils/misc.mli 2013-03-20 20:47:46.707140317 -0700
@@ -72,6 +72,10 @@
(* [input_bytes ic n] reads [n] bytes from [ic] and returns them
in a new string. It raises [End_of_file] if EOF is encountered
before all the bytes are read. *)
+val input_big_bytes : in_channel -> int -> string list;;

  •    (* [input_bytes ic n] reads [n] bytes from [ic] and returns them
    
  •       in a new string.  It raises [End_of_file] if EOF is encountered
    
  •       before all the bytes are read. *)
    

val log2: int -> int
(* [log2 n] returns [s] such that [n = 1 lsl s]

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 21, 2013

Comment author: @gasche

This is the same bug report as #5920, but I'm keeping this one as your patch is a good way to start discussion.

As far as patches are concerned, it seems that you identified the right point to change for the debuginfo problem, but I must say I like Benoît's approach in http://caml.inria.fr/mantis/file_download.php?file_id=874&type=bug better. It might be interesting to merge the two things, that is extend the use of his LongString type to a input_long_bytes function and use it in the place you changed.

That said, the remark on 32bits users suffering from a lack of maintainer love still stands. Your best long-term workaround is to use a 64 bit architecture.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 22, 2013

Comment author: chetsky@gmail.com

Gasche, I looked at Benoit's changes, and

(a) to someone who doesn't understand the compiler much, yes his changes look analogous to what I tried to do

(b) his Longstring is much nicer than my little -hack- of "string list". Heh. I thought of writing something prettier (a Bigstring module) but decided that before going down that path, I'd log the report -- in case somebody else had already gone down this road. As, it turn out, somebody did.

So (c) I'll try out his Longstring and report back.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 25, 2013

Comment author: chetsky@gmail.com

gasche, I verified that Benoit's patch, with some minor additions to cover the case that my patch was for, builds, passes bootstrap, and also passes the testcase that I submitted above.

It's a pretty minor change, and I'm going to upload "longstring-2.diff", containing both benoit's patch, and my changes on top of his.

[Ugh: I included a file by mistake. "longstring-3.diff" should contain the right diff]

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Apr 2, 2013

Comment author: @damiendoligez

I think we should apply this patch. Should it go into trunk only, or also in the 4.00 branch?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Apr 17, 2013

Comment author: @gasche

I just committed the change (in trunk only) with minor cosmetic changes.

Thanks, chetsky, for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.