Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Bug#1037136: dpkg-buildflags: 64-bit time_t by default

3 views
Skip to first unread message

Steve Langasek

unread,
Jun 6, 2023, 12:30:03 AM6/6/23
to
Package: dpkg-dev
Version: 1.21.22
Tags: patch
User: ubuntu...@lists.ubuntu.com
Usertags: origin-ubuntu mantic patch

Hi Guillem,

The discussion on debian-devel around 64-bit time_t has died down, so I
figure it's time to propose some patches to implement what's been discussed
there.

I'm not sure whether you were persuaded that i386 should stay with the
current ABI, but anyway thought I would propose the patches and we could
discuss further if necessary.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer https://www.debian.org/
slan...@ubuntu.com vor...@debian.org
0001-lfs-and-time64-are-no-longer-future-call-them-featur.patch
0002-Enable-time64-by-default-for-all-32-bit-archs-except.patch
0003-Also-emit-Werror-implicit-function-declaration-for-f.patch
signature.asc

Guillem Jover

unread,
Jun 8, 2023, 10:10:03 PM6/8/23
to
Hi!

On Mon, 2023-06-05 at 21:18:10 -0700, Steve Langasek wrote:
> Package: dpkg-dev
> Version: 1.21.22
> Tags: patch
> User: ubuntu...@lists.ubuntu.com
> Usertags: origin-ubuntu mantic patch

> The discussion on debian-devel around 64-bit time_t has died down, so I
> figure it's time to propose some patches to implement what's been discussed
> there.

Yeah, I'm not sure whether it has died off or it's just being slow.

> I'm not sure whether you were persuaded that i386 should stay with the
> current ABI, but anyway thought I would propose the patches and we could
> discuss further if necessary.

In any case I've tried to reply there. Also my impression was that
there was still some analysis pending (perhaps that was a wrong
impression though)?

Thanks for the patches!

> From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001
> From: Steve Langasek <steve.l...@ubuntu.com>
> Date: Fri, 2 Jun 2023 14:30:20 +0000
> Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
> "feature" instead
>

Ah, I actually had implemented locally the alias with your original
suggestion of "abi"! :) Using "feature" here seems would be rather
more confusing as these are called feature flags, and feature areas.

I'll try to push the stuff I've got queued locally during the weekend,
then I can rebase these patches on a branch or similar.

> From 7eff8f89b32b6921a0d86c50c6c62154c6ddc96e Mon Sep 17 00:00:00 2001
> From: Steve Langasek <steve.l...@ubuntu.com>
> Date: Fri, 2 Jun 2023 16:30:19 +0000
> Subject: [PATCH 3/3] Also emit -Werror=implicit-function-declaration for
> feature=+time64
>
> Per https://lists.debian.org/debian-devel/2023/05/msg00262.html et al.,
> missing glibc includes can cause packages to link to the wrong symbols,
> potentially causing crashes or misbehavior. Since functions that use
> time_t are fairly ubiquitous, there's a high risk of this happening for
> *some* package in Debian. Better to make all software with missing
> function declarations fail to build now, than to spend all cycle tracking
> down runtime bugs.
> ---
> scripts/Dpkg/Vendor/Debian.pm | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
> index 20d77fea1..803949024 100644
> --- a/scripts/Dpkg/Vendor/Debian.pm
> +++ b/scripts/Dpkg/Vendor/Debian.pm
> @@ -400,6 +400,8 @@ sub _add_build_flags {
>
> if ($flags->use_feature('feature', 'time64')) {
> $flags->append('CPPFLAGS', '-D_TIME_BITS=64');
> + $flags->append('CFLAGS', '-Werror=implicit-function-declaration');
> + $flags->append('CXXFLAGS', '-Werror=implicit-function-declaration');
> }

I'm not sure I like intermingling the different semantics here, if
necessary I'd rather make time64 forcibly enable another feature flag,
like it is done for lfs.

As I mentioned on the recent thread about the modern C stuff, I do
have a branch that adds these as part of a new qa=+c99 feature, but
that's too broad. :/

<https://git.hadrons.org/git/debian/dpkg/dpkg.git/commit/?h=next/modern-c&id=3316845bf415436299d61501db655fd2c1813436>

Maybe I could add a new feature area instead and add the flags
individually, and then make time64 also enable that other specific
feature. Hmmm.

Thanks,
Guillem

Steve Langasek

unread,
Jul 5, 2023, 5:40:04 PM7/5/23
to
Hi,

I wanted to check in on the status of this.

On Fri, Jun 09, 2023 at 03:56:28AM +0200, Guillem Jover wrote:
> On Mon, 2023-06-05 at 21:18:10 -0700, Steve Langasek wrote:
> > Package: dpkg-dev
> > Version: 1.21.22
> > Tags: patch
> > User: ubuntu...@lists.ubuntu.com
> > Usertags: origin-ubuntu mantic patch

> > The discussion on debian-devel around 64-bit time_t has died down, so I
> > figure it's time to propose some patches to implement what's been discussed
> > there.

> Yeah, I'm not sure whether it has died off or it's just being slow.

> > I'm not sure whether you were persuaded that i386 should stay with the
> > current ABI, but anyway thought I would propose the patches and we could
> > discuss further if necessary.

> In any case I've tried to reply there. Also my impression was that
> there was still some analysis pending (perhaps that was a wrong
> impression though)?

There is analysis pending, unfortunately 90% of the -dev packages have been
analyzed leaving 90% to go (~1600 -dev packages that require fixes to be
compilable and therefore analyzable). I don't have any answer yet from the
Release Team, so I'm not sure if we need this analysis to be completely done
before starting the transition or if we can leave the long tail of packages
with 1 or 2 reverse-build-dependencies to be figured out later.

> Thanks for the patches!

> > From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001
> > From: Steve Langasek <steve.l...@ubuntu.com>
> > Date: Fri, 2 Jun 2023 14:30:20 +0000
> > Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
> > "feature" instead

> Ah, I actually had implemented locally the alias with your original
> suggestion of "abi"! :) Using "feature" here seems would be rather
> more confusing as these are called feature flags, and feature areas.

> I'll try to push the stuff I've got queued locally during the weekend,
> then I can rebase these patches on a branch or similar.

As far as I can tell, this hasn't been pushed anywhere yet?
Did you reach a decision here? Anything you'd like from me to move this
forward?

Thanks,
signature.asc

Guillem Jover

unread,
Jul 9, 2023, 2:10:03 PM7/9/23
to
Hi!

On Wed, 2023-07-05 at 14:24:02 -0700, Steve Langasek wrote:
> On Fri, Jun 09, 2023 at 03:56:28AM +0200, Guillem Jover wrote:
> > On Mon, 2023-06-05 at 21:18:10 -0700, Steve Langasek wrote:
> > > Package: dpkg-dev
> > > Version: 1.21.22
> > > Tags: patch
> > > User: ubuntu...@lists.ubuntu.com
> > > Usertags: origin-ubuntu mantic patch

> > > The discussion on debian-devel around 64-bit time_t has died down, so I
> > > figure it's time to propose some patches to implement what's been discussed
> > > there.
>
> > Yeah, I'm not sure whether it has died off or it's just being slow.
>
> > > I'm not sure whether you were persuaded that i386 should stay with the
> > > current ABI, but anyway thought I would propose the patches and we could
> > > discuss further if necessary.
>
> > In any case I've tried to reply there. Also my impression was that
> > there was still some analysis pending (perhaps that was a wrong
> > impression though)?
>
> There is analysis pending, unfortunately 90% of the -dev packages have been
> analyzed leaving 90% to go (~1600 -dev packages that require fixes to be

Ack, thanks. Is that last % supposed to be 10%? Otherwise I think I'm
missing something :).

> compilable and therefore analyzable). I don't have any answer yet from the
> Release Team, so I'm not sure if we need this analysis to be completely done
> before starting the transition or if we can leave the long tail of packages
> with 1 or 2 reverse-build-dependencies to be figured out later.

I don't know. Perhaps ask on d-d?

> > > From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001
> > > From: Steve Langasek <steve.l...@ubuntu.com>
> > > Date: Fri, 2 Jun 2023 14:30:20 +0000
> > > Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
> > > "feature" instead
>
> > Ah, I actually had implemented locally the alias with your original
> > suggestion of "abi"! :) Using "feature" here seems would be rather
> > more confusing as these are called feature flags, and feature areas.
>
> > I'll try to push the stuff I've got queued locally during the weekend,
> > then I can rebase these patches on a branch or similar.
>
> As far as I can tell, this hasn't been pushed anywhere yet?

Right, sorry got entangled in a local branch with random stuff, but
rebased that and added on top now several other fixes including these
changes or ones similar in intention. See at:

https://git.hadrons.org/git/debian/dpkg/dpkg.git/log/?h=next/time64-default

(Beware that I might rebase that branch, before merging it, although I
think I'll start pushing some of the foundation work into main already.)
I realized now that this cannot be set for CXXFLAGS as at least g++
will warn about that. And I've gone for now by depending on qa=+bug,
but I'm not sure whether that would cause too many regressions. OTOH
and AFAIK any such problem should be considered a genuine bug anyway,
so…

But if this is too much I guess I could split that warning into its
own feature and make both the qa=+bug and abi=+time64 depend on it
instead.

Thanks,
Guillem

Steve Langasek

unread,
Dec 20, 2023, 2:50:05 AM12/20/23
to
Hi Guillem,

Coming back to this after a hiatus. (In the intervening time the focus has
been getting the library ABI analysis right; now coming back around to
looking at the toolchain.)

On Sun, Jul 09, 2023 at 08:06:56PM +0200, Guillem Jover wrote:
> > There is analysis pending, unfortunately 90% of the -dev packages have been
> > analyzed leaving 90% to go (~1600 -dev packages that require fixes to be

> Ack, thanks. Is that last % supposed to be 10%? Otherwise I think I'm
> missing something :).

A reference to https://en.wikipedia.org/wiki/Ninety%E2%80%93ninety_rule

> > > > From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001
> > > > From: Steve Langasek <steve.l...@ubuntu.com>
> > > > Date: Fri, 2 Jun 2023 14:30:20 +0000
> > > > Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
> > > > "feature" instead

> > > Ah, I actually had implemented locally the alias with your original
> > > suggestion of "abi"! :) Using "feature" here seems would be rather
> > > more confusing as these are called feature flags, and feature areas.

> > > I'll try to push the stuff I've got queued locally during the weekend,
> > > then I can rebase these patches on a branch or similar.

> > As far as I can tell, this hasn't been pushed anywhere yet?

> Right, sorry got entangled in a local branch with random stuff, but
> rebased that and added on top now several other fixes including these
> changes or ones similar in intention. See at:

> https://git.hadrons.org/git/debian/dpkg/dpkg.git/log/?h=next/time64-default

> (Beware that I might rebase that branch, before merging it, although I
> think I'll start pushing some of the foundation work into main already.)

> > Did you reach a decision here? Anything you'd like from me to move this
> > forward?

> I realized now that this cannot be set for CXXFLAGS as at least g++
> will warn about that. And I've gone for now by depending on qa=+bug,
> but I'm not sure whether that would cause too many regressions. OTOH
> and AFAIK any such problem should be considered a genuine bug anyway,
> so…

> But if this is too much I guess I could split that warning into its
> own feature and make both the qa=+bug and abi=+time64 depend on it
> instead.

Now that I've had a chance to look at the implementation, I am concerned
about abi=+time64 implying qa=+bug, because I see that this turns on
additional -Werror options beyond implicit-function-declaration:

my @cfamilyflags = qw(
array-bounds
clobbered
volatile-register-var
);
foreach my $warnflag (@cfamilyflags) {
$flags->append('CFLAGS', "-Werror=$warnflag");
$flags->append('CXXFLAGS', "-Werror=$warnflag");
}

While these may all be bugs, forcing the fixing of bugs unrelated to the
time_t transition during the transition will inevitably slow down Debian
development, so I am concerned about having such a dependency. We
unfortunately also haven't done any archive rebuild analysis on this to
understand how large the actual impact is (again, the focus has been on just
getting the analysis right of which packages do have an ABI change).

And while there's no specific timeline required on the Debian side yet, on
the Ubuntu side we have a tight timeline to get this all done in the next
couple of months so that it can be included in the 24.04 LTS release.

The idea of splitting it into a separate feature seems ok, to avoid turning
on unrelated -Werror options.

We still don't have a slot from the Release Team for when this can be
landed, but following up to debian-devel with a complete analysis of the
library ABIs is my next step before the end of year. Is this a change you
think could be uploaded to dpkg on short notice? Or would you be amenable
to an NMU, if you're unavailable for an upload?
signature.asc

Guillem Jover

unread,
Dec 20, 2023, 6:30:03 PM12/20/23
to
Hi!
Ok, I'll prepare a patch this week to split it then.

> We still don't have a slot from the Release Team for when this can be
> landed, but following up to debian-devel with a complete analysis of the
> library ABIs is my next step before the end of year. Is this a change you
> think could be uploaded to dpkg on short notice? Or would you be amenable
> to an NMU, if you're unavailable for an upload?

Once the discussion is settled and the plan agreed, I think that will
also include agreeing on a date for all the involved uploads. TBH, I
don't expect that to be on short notice given all the parties that
might need to coordinate this, but I have no problem planning and
preparing an upload for a specific pre-agreed date. (And in case that
for some weird reason it would end up being on short notice I should
be able to manage too, I guess. :)

I also assume that with "this change" you refer to flipping the default
plus the -Werror flag stuff and not just the latter.

Regards,
Guillem

Lukas Märdian

unread,
Feb 5, 2024, 10:00:04 AM2/5/24
to
Hi John, thanks for getting back!

On Mon, Feb 05, 2024 at 07:08:22AM -0600, John Goerzen wrote:
> Hi Lukas and Michael,
>
> Thank you for your work on this large transition.
>
> I am confused.

I'm sorry for the confusion, this time_t ABI transition is a special snowflake,
as timing is crucial. It all depends on the new default compiler flags
introduced through src:dpkg. See: https://bugs.debian.org/1037136

> The gensio bug was reported with severity serious, against the version
> of gensio in unstable, which prevents transition to testing.
>
> I don't understand what action is being requested. If the bug cannot be
> fixed, it should not be filed (or not filed as serious).

We cannot fix this bug right now. Only in experimental, where we have the new
dpkg already. As soon as the dpkg time_t changes land in unstable, we need to
fix affected libraries quickly, to minimize breakage. That's why we're
currently staging 1000+ NMUs.

> Additionally, there are other bugs impacting these packages and their
> symbol files, which I have addressed. They will create a conflict with
> the proposed NMU, so the NMU will need to be refreshed before being
> applied.
>
> Now the bug is reopened asking me to roll back the same patch that the
> bug was opened for. I could of course do that -- disentangling the
> conflicts -- and close the bug with the upload. But that would leave
> gensio without the t64 libraries -- the same state it was in when you
> reported the serious bug.

If you have other bugs to address you can do that in unstable. But please
revert the NMU patch for the time being. As stated above, we need to wait
for dpkg (Bug#1037136) to land in unstable, first.

The upload you have in unstable right now is still compiled using the old
time_t ABI, thus will be broken once the dpkg changes land.

> So which is it: is there a serious bug with gensio in unstable with the
> t64 libraries, or without? It cannot be both.
>
> I don't think I've ever received a bug report, severity serious, tagged
> patch, found in the version in unstable, where applying the patch is
> somehow the wrong thing to do.
>
> Please clarify what action I could take that would close this bug.

We don't have t64 libraries in unstable, yet. So it is not (yet) affecting
gensio in unstable, but will be in due course. We wanted to give maintainers
a bit of headroom, before uploading NMUs into unstable en masse.

Please roll back the NMU patch in unstable. But feel free to upload other
changes that are needed to fix gensio bugs. We'll then need to rebase the NMU.

After Bug#1037136 is resolved in unstable, we can upload the changes from the
NMU patch to unstable, which will pick up the new 64-bit time_t ABI and thus
close this bug. This should happen within the next few weeks.

> I may not be the only one confused here, and would be happy to have this
> conversation on debian-devel if that would be more broadly useful.

I understand this can be confusing, dicussion about it has been going on for
the past several weeks and months, feel free to jump into those threads for
further clarification:
- https://lwn.net/ml/debian-devel/ZGRSOnvO...@homer.dodds.net/
- https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1036884

Cheers,
Lukas
signature.asc

Dima Kogan

unread,
Feb 7, 2024, 3:50:05 AM2/7/24
to
Hi.

libmrcal-dev does not use time_t. I'm seeing the abi-compliance-checker
failure here:

https://adrien.dcln.fr/misc/armhf-time_t/2024-02-01T09%3A53%3A00/logs/libmrcal-dev/base/log.txt

The cause is that the tool takes all the headers in /usr/include/mrcal
in an arbitrary order, and tries to #include them. That does not work
here. "mrcal-internal.h" should not be #included explicitly since it is
already #included by mrcal.h. Removing that header from the command in
the error log above makes the errors disappear.

Can we do that, and remove libmrcal-dev from this transition?

Thanks.

Thorsten Glaser

unread,
Feb 20, 2024, 12:50:05 PM2/20/24
to
Hi *,

from my experience, adding -Werror=* flags causes breakage
because way too many configure scripts miss the appropriate
headers when probing for symbols, so this wouldn’t lead to
build failures but to changed feature sets or the programs
using older/fallback APIs (e.g. if probing openat() fails,
a program could unconditionally use open in an insecure way
as fallback, and the missing header would be there in the
site that actually uses it but autoconf generally misses
them).

In MirBSD, I’ve worked around this by appending a new flag
-Werror-maybe-reset to CFLAGS and CXXFLAGS which has no
effect in GCC unless an extra environment variable is set,
in which case it does -Wno-error. Then, we set that env
during the configure call but not during make all/install.
This was achieved with a trivial local GCC patch.

I know we already add -Werror=format-security in many
cases, but that has less chance to break the configue
stage (even so I’d look sceptical at it).

bye,
//mirabilos
--
<hecker> cool ein Ada Lovelace Google-Doodle. aber zum 197. Geburtstag? Hätten
die nicht noch 3 Jahre warten können? <mirabilos> bis dahin gibts google nicht
mehr <hecker> ja, könnte man meinen. wahrscheinlich ist der angekündigte welt-
untergang aus dem maya-kalender die globale abschaltung von google ☺ und darum
müssen die die doodles vorher noch raushauen
0 new messages