read_int incorrectly reads (-1)

[db4]

This function

lwt/src/unix/lwt_io.ml

Lines 1058 to 1069 in d791b9b

	let read_int ic =
	read_block_unsafe ic 4
	(fun buffer ptr ->
	let v0 = get buffer (ptr + pos32_0)
	and v1 = get buffer (ptr + pos32_1)
	and v2 = get buffer (ptr + pos32_2)
	and v3 = get buffer (ptr + pos32_3) in
	let v = v0 lor (v1 lsl 8) lor (v2 lsl 16) lor (v3 lsl 24) in
	if v3 land 0x80 = 0 then
	Lwt.return v
	else
	Lwt.return (v - (1 lsl 32)))

reads -1 as -2 in 32-bit OCaml distribution. Isn't it a bug?

[aantron]

Yes. On a 32-bit platform, Sys.int_size is 31. The result of n lsl m is "unspecified," since m > 31 here (https://caml.inria.fr/pub/docs/manual-ocaml/libref/Pervasives.html#VAL(lsl)). It happens that 1 lsl 32 is 1, hence the result.

[db4]

Why not just use OCaml compiler approach:

let input_binary_int ic =
  let b1 = input_byte ic in
  let n1 = if b1 >= 128 then b1 - 256 else b1 in
  let b2 = input_byte ic in
  let b3 = input_byte ic in
  let b4 = input_byte ic in
  (n1 lsl 24) + (b2 lsl 16) + (b3 lsl 8) + b4

[aantron]

Thanks. I adapted this code, and the bug should be fixed by the attached commit.

We are waiting on #658 (4.08 compatibility) for a release. In the meantime, would you mind testing whether Lwt master works for you? You should be able to install it with

opam pin add lwt --dev-repo

[db4]

@aantron Dev lwt doesn't compile in my environment:

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] The compilation of lwt failed at "C:\\Users\\dbely\\.opam\\4.06.1+vc15+x86\\bin\\dune.exe build -p lwt -j 3".

#=== ERROR while compiling lwt.dev ============================================#
# context     2.0.2 | win32/x86_64 | ocaml-variants.4.06.1+win32 | pinned(git+https://github.com/ocsigen/lwt.git#33a44df4)
# path        ~/.opam/4.06.1+vc15+x86/.opam-switch/build/lwt.dev
# command     C:\Users\dbely\.opam\4.06.1+vc15+x86\bin\dune.exe build -p lwt -j 3
# exit-code   1
# env-file    ~/.opam/log/lwt-376-5f1023.env
# output-file ~/.opam/log/lwt-376-5f1023.out
### output ###
# [...]
# not checking for netdb_reentrant
# not checking for reentrant gethost*
# testing for nanosecond stat support: ........... unavailable
# not checking for BSD mincore
# File "_build/default/src/unix/unix_c_library_flags.sexp", line 1, characters 1-11:
# 1 | (ws2_32.lib)
#      ^^^^^^^^^^
# Error: This atom must be quoted because it is the first element of a list and doesn't start with - or :
#       ocamlc src/unix/lwt_libev_stubs.obj
# c:\users\dbely\.opam\4.06.1+vc15+x86\.opam-switch\build\lwt.dev\_build\default\src\unix\lwt_libev_stubs.c(221) : warning C4716: 'lwt_libev_stop': must return a valuec:\users\dbely\.opam\4.06.1+vc15+x86\.opam-switch\build\lwt.dev\_build\default\src\unix\lwt_libev_stubs.c(222) : warning C4716: 'lwt_libev_loop': must return a valuec:\users\dbely\.opam\4.06.1+vc15+x86\.opam-switch\build\lwt.dev\_[...]
#       ocamlc src/unix/windows_not_available.obj
# c:\users\dbely\.opam\4.06.1+vc15+x86\.opam-switch\build\lwt.dev\_build\default\src\unix\windows_not_available.c(14) : warning C4716: 'lwt_unix_madvise': must return a valuec:\users\dbely\.opam\4.06.1+vc15+x86\.opam-switch\build\lwt.dev\_build\default\src\unix\windows_not_available.c(15) : warning C4716: 'lwt_unix_mincore': must return a valuec:\users\dbely\.opam\4.06.1+vc15+x86\.opam-switch\b[...]

BTW, It would be great if you apply the following small fix needed for the recent Microsoft compilers:

--- a/src/unix/config/discover.ml
+++ b/src/unix/config/discover.ml
@@ -324,14 +324,16 @@ let compile (opt, lib) stub_file =
   let cmd fmt = ksprintf (fun s ->
     dprintf "RUN: %s" s;
     Sys.command s = 0) fmt in
+  let o_flag = if !ccomp_type = "msvc" then "-Fo:" else "-o" in
   let obj_file = Filename.chop_suffix stub_file ".c" ^ !ext_obj
   in
   (* First compile the .c file using -ocamlc and CFLAGS (opt) *)
   (cmd
-    "%s -c %s %s -ccopt -o -ccopt %s >> %s 2>&1"
+    "%s -c %s %s -ccopt %s -ccopt %s >> %s 2>&1"
     !ocamlc
     (String.concat " " (List.map (fun x -> "-ccopt " ^ x) (List.map Filename.quote opt)))
     (Filename.quote stub_file)
+    o_flag
     (Filename.quote obj_file)
     (Filename.quote !log_file))
   &&

[aantron]

Thanks for all that info, @db4! I committed what I think is a fix for the S-expression problem, and also applied your patch (I double-checked -Fo: is valid for MSVC all the way back to VS2005). See the commits linked above. Could you try Lwt master again?

[db4]

@aantron Lwt master works for me. Thanks a lot!