unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: Joseph Myers <joseph@codesourcery.com>
Cc: libc-alpha@sourceware.org
Subject: Re: Accelerating Y2038 glibc fixes
Date: Mon, 29 Jul 2019 18:00:25 -0300	[thread overview]
Message-ID: <b7952e93-94f4-dc06-11d2-33c7c951f55d@linaro.org> (raw)
In-Reply-To: <alpine.DEB.2.21.1907291950350.1468@digraph.polyomino.org.uk>



On 29/07/2019 16:58, Joseph Myers wrote:
> On Mon, 29 Jul 2019, Adhemerval Zanella wrote:
> 
>>> I think duplicating ABIs like this is a very bad idea - the ABI supported 
>>> by glibc for a configuration that currently has 32-bit time_t should not 
>>> change to have two different, incompatible variants depending on how glibc 
>>> is configured.  The default API provided by glibc should also not vary 
>>> like that depending on how glibc is configured.
>>
>> Later on the thread [1] I did state I would prefer switch based on release
>> rather than a configure option, the suggestion was initially as a way to
>> easier the transition (at the cost of complexity I give you).
> 
> I'm not clear what "switch based on release" means.

To make the time_t change its type on a new glibc release rather than the
configure option.

> 
>> Since we require to have both time32 and time64 implementation for the
>> 'legacy' 32-bit architectures, the change to implement (c) is mainly to
>> make the symbol compat ones.  And since we will need to internal logic
> 
> No, it's mainly (for a large number of functions) finding some way to 
> avoid the unconditional (for platforms with __TIMESIZE == 32) header 
> redirects from <func> to __<func>_time64 applying to the definitions of 
> those compat symbols (and, likewise, for all the functions that have 
> variants for _FILE_OFFSET_BITS=32, because we don't want to support the 
> combination of 32-bit offsets with 64-bit times, and requiring 64-bit 
> times implies first requiring 64-bit offsets).  You can do that with a 
> suitable #define before including the header and #undef after, but there 
> are many functions, and different implementations of those functions to 
> deal with - and then there is the testing issue, where the changes are 
> probably even more involved.
> 
> Just adding new function variants with new names and header redirection to 
> provide optional support for using them is much simpler than anything that 
> also obsoletes the old functions.

I think it would be easier than what you described because we won't need
to actually to add any header redefinition, all symbol affect will just
use the new time_t regardless of the ABI.  The compat implementation will
use an internal-only type to use the old one.

What it would require is to add compat implementations with a different
type, time32_t for instance.  Something like:

---
* sysdeps/unix/sysv/linux/generic/bits/typesizes.h
[...]
#define __TIME_T_TYPE           __UQUAD_TYPE
[...]

* time/time.h
[...]
extern time_t time (time_t *__timer) __THROW;
[...]

* sysdeps/unix/sysv/linux/time.c
[...]
time_t
__time (time_t *t)
{ 
#ifdef __ASSUME_TIME64_SYSCALLS
  return INLINE_SYSCALL_CALL (time64, t);
#else  
  int ret;
# ifdef __NR_time64
  /* Maybe we can use 'sets' of 'supported' to enable/disable
     multiple syscalls.  */
  static int time64_supported = 1;
  if (atomic_read_relaxed (&time64_supported) == 1)
    {
      ret = INLINE_SYSCALL_CALL (time64, t);
      if (ret == 0 || errno != ENOSYS)
	return ret;

      atomic_store_relaxed (&time64_supported, 0)
    }
# endif /* __NR_syscall64  */
  
  ret = INLINE_SYSCALL_CALL (time, t);
  /* I assume kernel will return EOVERFLOW if the case.  Other symbol
     will require further handling.  */
  return ret;
#endif
}
libc_hidden_def (time)

/* At first it would be a arch-specific definition, on kernel-features.h.
   Maybe there is a clever way to accomplish it.  */
#ifdef __REQUIRE_TIME32_COMPAT
/* Define time32_t internally somewhere.  */
time32_t
__time32 (time32_t *t)
{
  return INLINE_SYSCALL_CALL (time, t);
}
compat_symbol (libc, __time32, time, GLIBC_2_0);
versioned_symbol (libc, __time, time, GLIBC_2_SOMETHING);
#else
weak_alias (__time, time)
#endif
[...]
---

I used the simplest symbol I can think off, other symbol that use time_t embedded
inside more complex struct will need to replicate it on some compat header (as
we do for some internal definition such sigaction).

Testing will require more boilerplate to link against the compat symbol, but
we already have some support on libsupport and I think we can improve to
make it simpler.

> 
>> The question I have is what is the real gain of still supporting _TIME_BITS=32
>> as a build option, if the idea is default to _TIME_BITS=64.  It open a 
> 
> The gain is supporting building glibc itself for such configurations 
> without a large amount of complicated work to build and test compat 
> symbols needing to be done up front - allowing the transition to be broken 
> down into more reasonably sized pieces.
> 

I don't have a strong opinion if a patch proposal use the _TIME_BITS=32
as a initial transition to enable time64 support, however I see no point
in make it available either on a release point neither in long term. 

In fact I see that support_TIME_BITS=32 on long term is a detriment for 
the ecosystem as whole. It is error prone, it will be glibc specific 
(since other system that used to have the same issue already did that
transition without similar complexity, such as BSD), and it will lead 
to subtle bugs with the multiple build systems that we have in Linux 
ecosystem.

That's why I still think that an initial rough transition now to move
forward definitely is better than still support broken interfaces 
indefinitely (not considering the required compat implementations).

  reply	other threads:[~2019-07-29 21:00 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12  7:21 Accelerating Y2038 glibc fixes Wolfgang Denk
2019-07-16  9:32 ` Wolfgang Denk
2019-07-16 11:50 ` Siddhesh Poyarekar
2019-07-16 12:40   ` Wolfgang Denk
2019-07-16 12:44 ` Florian Weimer
2019-07-16 14:52   ` Wolfgang Denk
2019-07-16 15:09     ` Florian Weimer
2019-07-16 15:19       ` Andrew Pinski
2019-07-17 14:15     ` Arnd Bergmann
2019-07-17 14:41       ` Florian Weimer
2019-07-17 16:00         ` Wolfgang Denk
2019-07-17 16:04           ` Florian Weimer
2019-07-17 16:18             ` Lukasz Majewski
2019-07-18 18:53               ` Adhemerval Zanella
2019-07-18 19:13                 ` Florian Weimer
2019-07-18 20:31                   ` Adhemerval Zanella
2019-07-18 21:20                     ` Florian Weimer
2019-07-18 22:32                     ` Paul Eggert
2019-07-19  7:21                       ` Andreas Schwab
2019-07-19  3:06                     ` Rich Felker
2019-07-19 17:44                       ` Adhemerval Zanella
2019-07-19 19:03                         ` Alistair Francis
2019-07-25 20:40                 ` Joseph Myers
2019-07-29 17:47                   ` Adhemerval Zanella
2019-07-29 19:58                     ` Joseph Myers
2019-07-29 21:00                       ` Adhemerval Zanella [this message]
2019-07-29 21:08                         ` Joseph Myers
2019-07-29 23:12                           ` Paul Eggert
2019-07-29 23:30                             ` Joseph Myers
2019-07-17 17:50       ` Rich Felker
2019-07-17 21:57         ` Lukasz Majewski
2019-07-17 22:37           ` Rich Felker
2019-07-18  7:20             ` Lukasz Majewski
2019-07-18 13:35               ` Rich Felker
2019-07-18 14:47           ` Rich Felker
2019-07-18 14:49             ` Florian Weimer
2019-07-18 15:46               ` Rich Felker
2019-07-18 16:43                 ` Adhemerval Zanella
2019-07-20  4:43             ` Rich Felker
2019-07-25 19:54 ` Joseph Myers
2019-07-26 10:39   ` Lukasz Majewski
2019-07-29 18:55     ` Zack Weinberg
2019-07-29 20:12       ` Joseph Myers
2019-07-30 11:02         ` Lukasz Majewski
2019-07-30 12:24           ` Joseph Myers
2019-07-30 14:04       ` Lukasz Majewski
2019-08-09  7:25         ` Lukasz Majewski
     [not found]           ` <CAKCAbMhOMQ9yTFpy+OQkDvZPPFf_fFn6oSxjvLTaUwC4jpPRag@mail.gmail.com>
2019-08-09 12:32             ` Fwd: " Zack Weinberg
2019-07-30 19:58       ` Joseph Myers
2019-07-30 20:28         ` Florian Weimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b7952e93-94f4-dc06-11d2-33c7c951f55d@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).