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

misc fixes from building on 32-bit architectures with 64-bit time_t #5009

Merged
merged 5 commits into from
Sep 10, 2024

Conversation

elliefm
Copy link
Contributor

@elliefm elliefm commented Aug 28, 2024

I spent some time a few months ago trying to track down a variety of build failures on armel, armhf, and some others platforms affected by Debian's transition to providing a 64-bit time_t on these 32-bit platforms (#4876, #4887), but eventually stalled out on a stack smashing problem that seemed to be coming from within libical. @rjbs and I decided that it wasn't worth spending more time chasing the problem around since it makes no sense to run an IMAP service on these sort of devices anyway.

This PR is a distillation of the useful changes I made while investigating.

@dilyanpalauzov
Copy link
Contributor

As mentioned at #4887 (comment) , I think configure.ac should call AC_SYS_LARGEFILE.

Copy link
Contributor

@ksmurchison ksmurchison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial testing:

  • compiles on Fedora 39 with GCC 13.3.1and passes cunit and Cass tests
  • does NOT compile on Fedora 40 with GCC 14.2.1
  CCLD     cunit/unit
/usr/bin/ld: cunit/timeofday.o: in function `real_gettimeofday':
/home/ken/cyrus-imapd-upstream/cunit/timeofday.c:203:(.text+0x44d): undefined reference to `__gettimeofday64'

I tried #including <sys/time.h> to solve this but then it complains that gettimeofday() is defined twice

@elliefm
Copy link
Contributor Author

elliefm commented Aug 29, 2024

Huh, curious. I wonder what that's about

@elliefm
Copy link
Contributor Author

elliefm commented Sep 1, 2024

@ksmurchison can you do something like ack __gettimeofday64 /usr/include/ on Fedora 40 to see which (if any) of the system headers mention __gettimeofday64 anywhere, and then email me those header files? I want to study the defines and stuff.

@rsto Can you try building this PR too please? I'm curious whether it works ok for you, has the same issues, or has different ones.

@rsto
Copy link
Member

rsto commented Sep 2, 2024

@elliefm it builds fine for me on Debian 12.6 (bookworm)

gcc (Debian 12.2.0-14) 12.2.0
aarch64

CFLAGS="-g -Og -fPIC -W -Wall -Werror -fstack-protector-all -fno-semantic-interposition -Bsymbolic -fno-omit-frame-pointer"
CXXFLAGS="-g -O0 -Wunused -Wall -Wextra -Werror"

@elliefm
Copy link
Contributor Author

elliefm commented Sep 6, 2024

@ksmurchison also, what version of libc does Fedora 40 have? You can find the version with ldd --version

@ksmurchison
Copy link
Contributor

@ksmurchison also, what version of libc does Fedora 40 have? You can find the version with ldd --version

ldd (GNU libc) 2.39

@ksmurchison
Copy link
Contributor

@ksmurchison can you do something like ack __gettimeofday64 /usr/include/ on Fedora 40 to see which (if any) of the system headers mention __gettimeofday64 anywhere, and then email me those header files? I want to study the defines and stuff.

Emailed

@elliefm
Copy link
Contributor Author

elliefm commented Sep 9, 2024

@ksmurchison Thanks, that was helpful. Looks like this is a libc change rather than a Debian/Fedora thing, turns out the macros for dealing with 64-bit time on 32-bit hardware were changed in libc 2.39.

I've updated cunit/timeofday.c again to accommodate this... hopefully! It still works for me with libc 2.36, so I don't think I've broken anything that already worked, but I can't test whether I've actually fixed it for 2.39 or not. Can you try it out again please? Thanks

@ksmurchison
Copy link
Contributor

@elliefm Looks good on my end

@elliefm elliefm merged commit 2512750 into cyrusimap:master Sep 10, 2024
1 check passed
@elliefm elliefm added backport-to-3.8 for PRs that are to be backported to 3.8 backport-to-3.10 for PRs that are to be backported to 3.10 labels Sep 10, 2024
@elliefm elliefm deleted the v311/64-on-32-fixes branch September 13, 2024 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-3.8 for PRs that are to be backported to 3.8 backport-to-3.10 for PRs that are to be backported to 3.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants