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

Bug with the serialization of double for custom value #4486

Closed
vicuna opened this issue Jan 22, 2008 · 3 comments
Closed

Bug with the serialization of double for custom value #4486

vicuna opened this issue Jan 22, 2008 · 3 comments
Assignees
Labels

Comments

@vicuna
Copy link

vicuna commented Jan 22, 2008

Original bug ID: 4486
Reporter: Mathias Kende
Assigned to: @xavierleroy
Status: closed (set by @xavierleroy on 2013-08-31T10:44:00Z)
Resolution: not a bug
Priority: normal
Severity: minor
Version: 3.10.0
Category: ~DO NOT USE (was: OCaml general)
Monitored by: "Mathias Kende" jm

Bug description

The serialization and deserialization function for double (needed to handle custom value when writing a FFI) in byterun/intern.c and byterun/extern.c are incoherent:

  • The function caml_serialize_float_8 in extern.c calls the function caml_serialize_block_8 which depends on the ARCH_BIG_ENDIAN variable.
  • The function caml_deserialize_float_8 in intern.c calls the function caml_deserialize_float_8 which depends on the ARCH_FLOAT_ENDIANNESS variable and performs more work.

Therefore double variables can not be marshalled with the *serialize_float_8 functions when ARCH_FLOAT_ENDIANNESS is neither 0x01234567 nor 0x76543210 which seems to be my case (INTEL64 processor with 64 bits Linux).

@vicuna
Copy link
Author

vicuna commented Aug 4, 2008

Comment author: @xavierleroy

Fixed in working sources, will be in release 3.11. Note however that the bug affects only architectures where floats are neither big-endian nor little-endian, namely ARM with the old ABI. On IA64 or x86-64, the buggy implementation of caml_serialize_float_8 should still produce correct output.

@vicuna
Copy link
Author

vicuna commented Sep 23, 2008

Comment author: Mathias Kende

I'm reoppening the thread because I think that the issue is worse than that (I ran into it on x86-64 platform).

Reading config.h I see that ARCH_BIG_ENDIAN imply that ARCH_FLOAT_ENDIANNESS = 0x76543210

But, in both intern.c and extern.c the *serialize_block_8 and *serialize_float_8 functions use different conventions: the *block_8 functions perform a simple copy of their data when ARCH_BIG_ENDIAN is defined, whereas the *float_8 functions perform a simple copy when ARCH_FLOAT_ENDIANNESS is 0x01234567 (and therefor ARCH_BIG_ENDIAN is not defined).

So, maybe, double and other data are dealt with differently. In this case the issue is already solved, but it was triggered on any platforms. Or, there is an other typo in this code and the behaving of two of these functions should be exchanged between big and little endian (even though, it should now work consistently on one arch and even between big and little endian: only the old arm arch will be affected if the *_float_8 functions are faulty).

@vicuna
Copy link
Author

vicuna commented Dec 18, 2011

Comment author: @xavierleroy

Let me try to resolve this old issue. It could be that (by mistake) the serialize_block_8 and serialize_float_8 functions use different conventions. However, as noted by Mathias Kende, they correctly read back data, even if it was written on a platform with a different endianness, which is the important property. Moreover, changing the conventions could be problematic for users who store persistent data in files or databases using output_value. So, I propose to leave the code as it is today and close this PR.

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

2 participants