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

Use CApiFFI for syscalls that break with glibc's handling of 64-bit time_t #62

Closed
iliastsi opened this issue Apr 20, 2024 · 3 comments
Closed
Assignees

Comments

@iliastsi
Copy link
Contributor

As part of the effort to support 64-bit time_t on 32-bit platforms, glibc uses macros to define functions like clock_gettime(), gettimeofday(), mktime(), localtime() etc. This breaks unix-time which uses the ccall calling convention.

As an example, the following code breaks on a 32-bit system with 64-bit time_t:

import Data.UnixTime

main = getUnixTime >>= print

output:

UnixTime {utSeconds = 2274169716966502, utMicroSeconds = 0}

Here are some related fixes on other Haskell libraries:

@kazu-yamamoto kazu-yamamoto self-assigned this Apr 21, 2024
@kazu-yamamoto
Copy link
Owner

I don't have 32bit machines anymore.
Could you create a PR?

@iliastsi
Copy link
Contributor Author

iliastsi commented May 19, 2024

Hi @kazu-yamamoto, I can send a PR yes.

Looking at the code, I also noticed that utMicroSeconds is defined as Int32 (see https://github.com/kazu-yamamoto/unix-time/blob/a0e38a4a9688a0ac340e0069478a7e9d5210ac3b/Data/UnixTime/Types.hsc#L37C5-L37C19), but tv_usec from struct timeval is of type suseconds_t, which on 64-bit systems is 8 bytes long.

This means that unix-time is broken on big endian systems, and always sets utMicroSeconds to 0.

One solutions would be to change utMicroSeconds to be of type CSUSeconds instead (https://hackage.haskell.org/package/base-4.20.0.0/docs/Foreign-C-Types.html#t:CSUSeconds), but this will change the API for this package.

Another solution is to peek the value as CSUSeconds and then convert it to Int32 (given that tv_usec is in the [0, 999,999] range). Let me know how you want to handle this and I can include it in this PR as welll.

@kazu-yamamoto
Copy link
Owner

Nice catch!
To maintain backward compatibility, I like the latter approach.
PR is highly welcome!

iliastsi added a commit to iliastsi/unix-time that referenced this issue May 26, 2024
UnixTime defines 'utMicroSeconds' as 'Int32', but 'tv_usec' (in 'struct
timeval') is of type 'suseconds_t'.

The size of 'suseconds_t' is platform specific, and it is 8 bytes long
on 64-bit platforms. This happens to give us the correct result on small
endian architectures, but results in a value of 0 for 'utMicroSeconds'
on 64-bit big-endian architectures.

In order to avoid changing the type of 'utMicroSeconds' (which will
result in an API change) we peek 'tv_usec' using the 'CSUSeconds' type
and then convert it to 'Int32' (the type of 'utMicroSeconds'). We rely
on the fact that 'tv_usec' is no bigger than '1000000' and hence we will
not overflow during the above conversion.

Refs kazu-yamamoto#62
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants