Return types varing between different target platforms should be unified

[y8]

Bug Report

Different types returned on different arch's for File#pos

Linux rpi 5.10.17+ #1403 Mon Feb 22 11:26:13 GMT 2021 armv6l GNU/Linux
File#pos = Int32, File#size = Int64

Darwin intelmac 20.3.0 Darwin Kernel Version 20.3.0: Thu Jan 21 00:07:06 PST 2021; root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64
File#pos = Int64, File#size = Int64

Minimal test case:

tempfile = File.tempfile(".bar")
tempfile << "Foobar"

puts `uname -a`
puts "File#pos = #{typeof(tempfile.pos)}, File#size = #{typeof(tempfile.size)}"

For 64-bit Darwin it's built without any options withshards build
For 32-bit Linux it's cross-compiled on Darwin and linked inside armv6 docker container:

shards build -p -s -t --cross-compile --target="armv6-unknown-linux-gnueabihf"
docker run --name build-crystal --rm -it -v "$PWD":/build  yopp/crystal-cross-armv6:1.0 bin/file_type_check

It boils down to LibC.lseek return type:

• crystal/src/crystal/system/unix/file_descriptor.cr

  Line 83 in dd40a24

  pos = LibC.lseek(fd, 0, IO::Seek::Current)

• crystal/src/io/file_descriptor.cr

  Line 119 in dd40a24

  system_pos - @in_buffer_rem.size

And it seems like there is other places where arch-dependent types are exposed from LibC:
.pread

crystal/src/file/preader.cr

Line 22 in dd40a24

bytes_read = Crystal::System::FileDescriptor.pread(@file.fd, slice[0, count], @offset + @pos)

crystal/src/crystal/system/unix/file_descriptor.cr

Line 156 in dd40a24

bytes_read = LibC.pread(fd, buffer, buffer.size, offset)

Possibly #unbuffered_read & #unbuffured_write (implicit return value with bytes read/written?)

crystal/src/crystal/system/unix/file_descriptor.cr

Line 12 in dd40a24

LibC.read(fd, slice, slice.size).tap do |return_code|

[straight-shoota]

Since #10580, the return type of IO::FileDescriptor#pos is restricted to Int64. This means it's now actually broken on armv6. Similarly for pread.

The unbuffered methods are not directly affected, because IO::Evented casts the return values to Int64.

[straight-shoota]

There may be similar cases like these, though.

@oprypin Do you think we can use the instrumentation from #10575 to find return type discrepancies between targets?

[oprypin]

@straight-shoota Good idea!

Just running it without modifications produces this:

(x86_64-pc-linux-gnu vs armv6-unknown-linux-gnueabihf)

+++ b/src/big/big_float.cr
@@ -85,7 +85,7 @@ struct BigFloat < Float
   # TODO: improve this
   def_hash to_f64
 
-  def self.default_precision : UInt64
+  def self.default_precision : UInt32
     LibGMP.mpf_get_default_prec
   end
 
@@ -193,7 +193,7 @@ struct BigFloat < Float
     BigInt.new { |mpz| LibGMP.set_f(mpz, mpf) }
   end
 
-  def to_i64 : Int64
+  def to_i64 : Int32
     LibGMP.mpf_get_si(self)
   end
 
@@ -233,7 +233,7 @@ struct BigFloat < Float
     LibGMP.mpf_get_si(self)
   end
 
-  def to_u64 : UInt64
+  def to_u64 : UInt32
     raise OverflowError.new if self < 0
     LibGMP.mpf_get_ui(self)
   end
diff --git a/src/pointer.cr b/src/pointer.cr
index d65811a69..eaf4e8eb5 100644
--- a/src/pointer.cr
+++ b/src/pointer.cr
@@ -452,7 +452,7 @@ struct Pointer(T)
   # ptr[0] # => 42
   # ptr[1] # => 42
   # ```
-  def self.malloc(size : Int, value : T) : Int64*
+  def self.malloc(size : Int, value : T) : UInt32*
     ptr = Pointer(T).malloc(size)
     size.times { |i| ptr[i] = value }
     ptr
diff --git a/src/yaml/pull_parser.cr b/src/yaml/pull_parser.cr
index 630352869..b9e190dea 100644
--- a/src/yaml/pull_parser.cr
+++ b/src/yaml/pull_parser.cr
@@ -260,7 +260,7 @@ class YAML::PullParser
 
   # Note: YAML starts counting from 0, we want to count from 1
 
-  def location : {UInt64, UInt64}
+  def location : {UInt32, UInt32}
     {start_line, start_column}
   end
 

(the malloc one definitely seems bogus)

[oprypin]

This means it's now actually broken on armv6

I can confirm that it's broken. But actually that would be totally preventable! Why don't we just add such a build in CI: --cross-compile --target=armv6-unknown-linux-gnueabihf optionally --no-codegen

[straight-shoota]

Yeah, #7587 got kind of burried 🙈

[straight-shoota]

The return types of BigFloat#to_u64 and #to_i64 seem to be really broken on armv6. The whole point of these methods is to cast to 64-bit integer (not 32-bit).

YAML::PullParser location numbers are Int because of SizeT in LibYAML::Mark properties.

I found some others:

• Number#popcount has different return types based on implementations
• Math.frexp(BigDecimal) has return type based on SizeT