Re: [PATCH v3] Define in_int32_t_range to check if the 64 bit time_t syscall should be used

[Adhemerval Zanella Netto via Libc-alpha <libc-alpha@sourceware.org>]

On 08/11/22 11:27, Adhemerval Zanella Netto wrote:
> 
> 
> On 08/11/22 11:07, Arnd Bergmann wrote:
>> On Tue, Nov 8, 2022, at 14:49, Adhemerval Zanella Netto wrote:
>>> On 08/11/22 10:27, Arnd Bergmann wrote:
>>>
>>> Yes, the 32 bit fallback assumes that you either use the default minimum
>>> kernel or configure with --enable-kernel with a value lower than 5.1.
>>> And the optimization such as ecf2661281c was added on the basis that for
>>> such configuration the 32 time_t is always present.  
>>>
>>> For __ASSUME_TIME64_SYSCALLS (default fro 64 bit time_t ABI and for 32
>>> bit time_t with --enable-kernel=5.1) the 32 bit syscall should not be
>>> issued.
>>
>> Ok, good. What is the amount of testing that this combination
>> (--enable-kernel=5.1, CONFIG_COMPAT_32_BIT_TIME=n) has seen? Is
>> this something that is already part of some regression test setup,
>> or rather something that is supposed to work but isn't in
>> widespread use?
> 
> I would say not much I am aware off, I usually check only against stock 
> kernels from usual distributions (ubuntu, fedora, etc) and our patchwork 
> instance also uses default kernels.
> 
>>
>>> There are still the issue for a default configured glibc when running
>>> on kernels with CONFIG_COMPAT_32BIT_TIME=y, this would require to remove
>>> the fallback optimizations for !__ASSUME_TIME64_SYSCALLS.
>>
>> You mean CONFIG_COMPAT_32BIT_TIME=n with !__ASSUME_TIME64_SYSCALLS,
>> right? As I said, I'm not too worried about this corner case, as
>> long as there is some kind of use feedback that tells users to
>> change either glibc or kernel configuration.
>>
>> My impression is that the optimizing for running on old kernels
>> is not overly helpful, and that changing the fallback logic
>> would be better, but this does not feel like a correctness
>> issue when general-purpose distros always enable
>> CONFIG_COMPAT_32BIT_TIME, and embedded users that want it
>> disabled always build a matching glibc as well.
> 
> I added the optimization mainly because to disable the use of 32 bit
> time_t support requires a configure switch.  It is worries me a bit
> the combination of CONFIG_COMPAT_32BIT_TIME=n with 
> !__ASSUME_TIME64_SYSCALLS as potential source of obscures bugs, but
> I think that it would be unlikely in future when newer kernels will
> be more present.

I am ccing Rich because he agrees that CONFIG_COMPAT_32BIT_TIME is a 
userspace breakage and it will most likely break musl tim64 support.
He suggested to move this option behind EXPERT [1] and fix the 
documentation.

[1] http://ix.io/4bse