[PATCH 7/7] sysv: linux: Add 64-bit time_t variant for shmctl

[Alistair Francis]

On Wed, Jul 1, 2020 at 4:00 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jul 1, 2020 at 10:07 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> >
> >
> >
> > On 01/07/2020 13:35, Adhemerval Zanella wrote:
> > >
> > >
> > > On 01/07/2020 12:48, Alistair Francis wrote:
> > >> On Wed, Jul 1, 2020 at 5:19 AM Adhemerval Zanella
> > >> <adhemerval.zanella@linaro.org> wrote:
> > >>>
> > >>>
> > >>>
> > >>> On 30/06/2020 23:28, Alistair Francis wrote:
> > >>> deps/unix/sysv/linux/powerpc/struct_kernel_shmid64_ds.h b/sysdeps/unix/sysv/linux/powerpc/struct_kernel_shmid64_ds.h
> > >>>>> new file mode 100644
> > >>>>> index 0000000000..5533d44dee
> > >>>>> --- /dev/null
> > >>>>> +++ b/sysdeps/unix/sysv/linux/powerpc/struct_kernel_shmid64_ds.h
> > >>>>> @@ -0,0 +1,24 @@
> > >>>>> +/* Analogous to kernel struct shmid64_ds used on shmctl.  */
> > >>>>> +struct kernel_shmid64_ds
> > >>>>> +{
> > >>>>> +  struct ipc_perm shm_perm;
> > >>>>> +#if __TIMESIZE == 64
> > >>>>> +  long int shm_atime;
> > >>>>> +  long int shm_atime;
> > >>>>> +  long int shm_atime;
> > >>>>
> > >>>> This fails to compile as all three variables have the same name.
> > >>>
> > >>> In fact this code specifically is not used on powerpc since it will
> > >>> either use the __TIMESIZE == 32 (powerpc32) or not use this
> > >>> structure at all (powerpc64{le}).  But it is still an issue I will
> > >>> revise the patch for such issues and remove it (maybe adding a
> > >>> #error if it used on powerpc64).
> > >>
> > >> Ah whoops, I commented inline on the wrong file. The generic one
> > >> (sysdeps/unix/sysv/linux/struct_kernel_shmid64_ds.h) also has the same
> > >> problem which is why I saw the RISC-V build fail.
> > >>
> > >>>
> > >>> Besides it, does it work for riscv32 (which should use the default
> > >>> definition)?
> > >>
> > >> This patch doesn't work due to the same name variables in a few places
> > >> (one of them being the generic one).
> > >>
> > >
> > > I need to setup an 32-bit ABI with 64-bit time_t to catch these kind of
> > > issues.
> > >
> > >> I am testing all the other patches and should know by tonight (about 8
> > >> hours still), I'll let you know.
> > >
> > > Ack, you can also do a sniff test with the sysvipc tests to check if
> > > they are ok.
> >
> > Ok, the __TIMESIZE should not be used to define the kernel_* struct anyway,
> > they should be independent of the glibc time ABI selected or defined by
> > the ABI.
> >
> > I have updated my personal branch [1] with an updated version and I will
> > try to check if riscv32 at least built against. If you could also check
> > it would be helpful.
>
> Thanks!
>
> I have tested this series with patch 7 (this patch) dropped and it
> works for RV32.
>
> >
> > [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/semctl-y2038
>
> I am now running a full test of this branch, unfortunately it'll take
> about 24 hours.

I tested the above branch for RV32 and it passes all the tests, so works for me.

I have also reviewed the current patches on list.

Let me know if I can do anything else to help get this merged.

Alistair

>
> Alistair