m4: Enable 64-bit time_t on 32-bit systems with glibc-2.34

[swegener]

glibc-2.34 includes the user-facing part of the 64-bit time_t support for 32-bit systems.

Checklist

I have:

• [x] read the CONTRIBUTING.md document
• [x] compiled this code
• [x] tested this code
• [ ] included documentation (including possible behaviour changes)
• [ ] documented the code
• [ ] added or modified regression test(s)
• [ ] added or modified unit test(s)

[Habbie]

Nice, I didn't know that was possible! Just for my curiosity, what platform are you on where glibc is new enough but a full distro recompile hasn't happened?

[swegener]

I'm the maintainer of Gentoo's PowerDNs packages and we're having glibc-2.34 available. It's not going to require a recompile, but behaves like -D_FILE_OFFSET_BITS=64 in that it is opt-in und uses defines to alias time_t, time() etc. to a 64-bit definition during compilation.

[swegener]

I'm still in process of testing, so consider this a draft for now.

[swegener]

I built pdns-recursor-4.5.6 with these changes in a 32-bit container. It compiles and runs fine, but I think further testing would be good.

[omoerbeek]

@swegener This is still marked as Draft PR. Did you have any chance to look at this further? I'm getting the tree in shape for rec-4.6.0-beta1, and would like to include this.

[swegener]

From my end this should be it. I've tested both pdns and pdns-recursor in an 32-bit container with glibc-2.34 and both compile and run fine.

[Habbie]

Was the 32 bit container running on a 32 bit kernel or a 64 bit kernel?

[swegener]

The kernel was 64-bit.

[omoerbeek]

From my end this should be it. I've tested both pdns and pdns-recursor in an 32-bit container with glibc-2.34 and both compile and run fine.

Did you build & run the unit tests?

If not, please run configure with --enable-unit-tests, make and run testrunner for both pdns and pdns-recursor.

[omoerbeek]

I have not tested this on a recent enough glibc but the approach makes sense to me. I wonder if we could get away with defining _FILE_OFFSET_BITS=64 and D_TIME_BITS=64 unconditionally.

I remarked this earlier:

I do not like this unconditional approach much as it assumes that the whole world is glibc. Please add these defines only if needed.

[rgacogne]

Right, I guess that prevents polluting the environment with unneeded constants.

[Habbie]

I like this, but we should indeed get full test results (3 products, unit and regress) on a 32 bit kernel before merging.

[swegener]

8a420bd880b1 ~/pdns/pdns # git describe
rec-4.6.0-beta1-8-g989963770
8a420bd880b1 ~/pdns/pdns # uname -r -m
5.14.17 i686
8a420bd880b1 ~/pdns/pdns # /lib/libc.so.6
GNU C Library (Gentoo 2.34 p4) stable release version 2.34.
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR PURPOSE.
Compiled by GNU CC version 11.2.0.
libc ABIs: UNIQUE IFUNC ABSOLUTE
For bug reporting instructions, please see:
<https://bugs.gentoo.org/>.
8a420bd880b1 ~/pdns/pdns # PDNS_TEST_NO_IPV6=1 ./testrunner
Running 217 test cases...
Domain example.com lives in file './zones/example.com'
Domain test.com lives in file './zones/test.com'
Domain test.dyndns lives in file './zones/test.dyndns'
Domain wtest.com lives in file './zones/wtest.com'
Domain nztest.com lives in file './zones/nztest.com'
Domain dnssec-parent.com lives in file './zones/dnssec-parent.com'
Domain delegated.dnssec-parent.com lives in file './zones/delegated.dnssec-parent.com'
Domain secure-delegated.dnssec-parent.com lives in file './zones/secure-delegated.dnssec-parent.com'
Domain minimal.com lives in file './zones/minimal.com'
Domain tsig.com lives in file './zones/tsig.com'
Domain stest.com lives in file './zones/stest.com'
Nov 09 21:22:45 1001 questions waiting for database/backend attention. Limit is 1000, respawning
Queued: 0
Received: 100

*** No errors detected
8a420bd880b1 ~/pdns/pdns # cd recursordist/
8a420bd880b1 ~/pdns/pdns/recursordist # ./testrunner 
Running 457 test cases...

*** No errors detected
8a420bd880b1 ~/pdns/pdns/recursordist # cd ../dnsdistdist/
8a420bd880b1 ~/pdns/pdns/dnsdistdist # ./testrunner 
Running 123 test cases...

*** No errors detected

[mnalis]

(ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058643)

[Habbie]

(ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058643)

which warns that this change might not be safe, so we can't merge this without way more testing

[mnalis]

(ref: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058643)

which warns that this change might not be safe, so we can't merge this without way more testing

Note that it warns for specific case when dnsdist is on 32-bit system compiled with 64bit time_t, but:

• libraries that dnsdist uses are not also compiled with 64bit time_t, and
• dnsdist actually passes raw time_t values to those libraries

If either of those is not the case, there is no problem at all.

Only when both are true, those libraries would receive corrupted time_t values, which would be a problem (although arguably not as a big a problem as not being able to run dnsdist at all on 32bit platforms, as is the case currently).

So, what tests did you have in mind? I think the second point would be easier to test; i.e. what new libraries has dnsdist introduced between 1.5.x (when it worked and was supported on 32bit platforms) and current series? And which of those libraries actually take time_t as an argument that dnsdist send them?

---

I personally have been running dnsdist with PR on several such mixed 32/64bit locations for more than a month now, and have not noticed any ill effects so far, so it was somewhat tested. Of course, I'm unlikely to have exercised all of the codebase, but then again, if not having automated tests for 100% of the functionality is a release-blocker, does dnsdist qualify for 64bit platforms either?

Alternatively, would wrapping this support in --enable-beta-32bit-support be sufficient enough to address those concerns ("it should work, but it is beta, so not being thoroughly tested"), and then rely on people using dnsdist in mixed 32/64bit environments to report (potential) bugs related to time_t?

[Habbie]

wrapping this support in --enable-beta-32bit-support

I like this. Perhaps even add 'glibc' to the flag?

[mnalis]

wrapping this support in --enable-beta-32bit-support

I like this. Perhaps even add 'glibc' to the flag?

@swegener would you like to add wrapping of those changes in --enable-beta-glibc-32bit-support so the PR can be merged? Or would you prefer someone else to take it over?

[swegener]

I've decided to name the option --enable-experimental-64bit-time_t-support-on-glibc. experimental is used for other options. It's rather long, but it describes best what it does.

[coveralls]

Pull Request Test Coverage Report for Build 8421524387

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 17 unchanged lines in 6 files lost coverage.
• Overall coverage increased (+1.0%) to 59.429%

---

Files with Coverage Reduction	New Missed Lines	%
pdns/backends/gsql/gsqlbackend.hh	2	97.71%
modules/lmdbbackend/lmdbbackend.cc	2	73.5%
pdns/dnsdistdist/dnsdist.cc	2	68.09%
pdns/axfr-retriever.cc	3	65.96%
pdns/iputils.hh	3	74.8%
pdns/signingpipe.cc	5	85.48%
<!--	Total:	17

Totals
Change from base Build 8420603627:	1.0%
Covered Lines:	113637
Relevant Lines:	158752

---

💛 - Coveralls

[Habbie]

Approved, thank you for your patience! We'll merge this soon.

[rgacogne]

I was looking into backporting this to rel/dnsdist-1.9.x but I realized that dnsdist does't call PDNS_CHECK_TIME_T anymore, so I'm not sure the backport is needed?

[zeha]

I was looking into backporting this to rel/dnsdist-1.9.x but I realized that dnsdist does't call PDNS_CHECK_TIME_T anymore, so I'm not sure the backport is needed?

ISTM distros with actual 64bit time_t support turn on the flags without packages needing to do anything. Distros without the full support don't do this, and there its really then "something experimental". I'm thinking users wanting "something experimental" can run master?