[PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

[Adhemerval Zanella]

On 27/10/2020 19:10, Lukasz Majewski wrote:
> After the commit SHA1: 3283f711132eaadc4f04bd8c1d84c910c29ba
> "sysv: linux: Add 64-bit time_t variant for msgctl"
> 
> The glibc internal, 64 bit supporting version of struct __msqid64_ds has
> been exported.
> 
> This approach has issue when one would like to add Y2038 support to glibc.
> The problem is that both struct msqid_ds and __msqid64_ds rely on exported
> implementation of the msg.h header (./sysvipc/sys/msg.h) which
> unconditionally includes ./sysdeps/unix/sysv/linux/bits/msq.h which in
> turn includes ./sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h
> 
> As a result the externally exported struct msqid_ds.h is used internally
> by glibc. Problem starts when one wants to extend this struct with 64 bit
> time support:
> 
> | In file included from ../sysdeps/unix/sysv/linux/bits/msq.h:28,
> |                  from ../sysvipc/sys/msg.h:30,
> |                  from ../sysdeps/unix/sysv/linux/include/sys/msg.h:2,
> |                  from ../include/sys/msg.h:1,
> |                  from ../sysdeps/unix/sysv/linux/msgctl.c:19:
> | ../sysdeps/unix/sysv/linux/bits/types/struct_msqid_ds.h:31:6: error:
>             "__USE_TIME_BITS64" is not defined, evaluates to 0 [-Werror=undef]
> |  # if __USE_TIME_BITS64
> |       ^~~~~~~~~~~~~~~~~
> 
> 
> To fix this issue - the internal,for glibc, copy of struct __msqid64_ds
> has been re-introduced. This will allow clear separation between exported
> and internal glibc types.
> 
> Moreover, the __TIMESIZE based alias shall be internal to glibc.

I don think the definition duplication is the correct approach, it is not
required for other ABIs which vary depending of the compiler flags (such
as LFS).  Also, __TIMESIZE is already defined on an installed header 
(timesize.h) and it defines a macro which within the glibc namespace.

The redirection is different than LFS since we don't have the symbol64 
for all ABIs, so we will need a different way to redirect the implementation.  

What I think it should be done it to move the __TIMESIZE definition to
exported header that actually includes and use the redirect macros we
have for LFS support.  Something as:

  1. On sysdeps/unix/sysv/linux/bits/msq.h

    #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
    # include <bits/types/struct_msqid_ds.h>
    #else
    # include <bits/types/struct_msqid_ds64.h>
    #endif

    This make currently 64-bit time ABIs or 32-bit ABIs using 32-bit flags
    to include the architecture defined struct_msqid_ds.h

    I used this approach instead of moving the logic to bits/msq.h to avoid
    the need to add this boilerplate and on each architecture define
    bits/msq.h.

  2. On the installed sysvipc/sys/msg.h header do a similar thing:

    #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
    # define msqid_ds __msqid64_ds
    #endif

    #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
    extern int msgctl (int __msqid, int __cmd, struct msqid_ds *__buf) __THROW;
    #else
    # ifdef __REDIRECT_NTH
    extern int __REDIRECT_NTH (msgctl (int __msqid, 
				       int __cmd,
				       struct msqid64_ds *__buf),
				       __msgctl64) __THROW;
    # else
    #  define msgctl __msgctl64
    # endif
    #endif
    
    Different than LFS, I don't think we need to support the symbol64
    usage (enabled by _LARGEFILE64_SOURCE); just the build to use the
    64-bit time_t ABI.

    Unfortunately I don't know a easy way to redirect the 'struct msqid'
    instantiation to the 64-bit time_t type without a '#define'.

  3. Install the struct_msqid_ds64.h header.


You might need to adjust something on the build itself, but I think this
is the general idea of exporting the 64-bit time interfaces.  What I am
not sure if how we gonna export the 64-bit ABI: whether to use current
name with double underscore of use a similar name convention used for
LFS (without the underscore).

> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> ---
>  include/bits/types/struct_msqid64_ds.h        | 36 +++++++++++++++++++
>  .../sysv/linux/bits/types/struct_msqid64_ds.h |  4 ---
>  2 files changed, 36 insertions(+), 4 deletions(-)
>  create mode 100644 include/bits/types/struct_msqid64_ds.h
> 
> diff --git a/include/bits/types/struct_msqid64_ds.h b/include/bits/types/struct_msqid64_ds.h
> new file mode 100644
> index 0000000000..8c39a2a872
> --- /dev/null
> +++ b/include/bits/types/struct_msqid64_ds.h
> @@ -0,0 +1,36 @@
> +/* Internal for glibc implementation of the SysV message struct msqid64_ds.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <time.h>
> +
> +#if __TIMESIZE == 64
> +# define __msqid64_ds msqid_ds
> +#else
> +struct __msqid64_ds
> +{
> +  struct ipc_perm msg_perm;	/* structure describing operation permission */
> +  __time64_t msg_stime;		/* time of last msgsnd command */
> +  __time64_t msg_rtime;		/* time of last msgsnd command */
> +  __time64_t msg_ctime;		/* time of last change */
> +  __syscall_ulong_t __msg_cbytes; /* current number of bytes on queue */
> +  msgqnum_t msg_qnum;		/* number of messages currently on queue */
> +  msglen_t msg_qbytes;		/* max number of bytes allowed on queue */
> +  __pid_t msg_lspid;		/* pid of last msgsnd() */
> +  __pid_t msg_lrpid;		/* pid of last msgrcv() */
> +};
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> index 3536c8ea62..07cc1036ab 100644
> --- a/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> +++ b/sysdeps/unix/sysv/linux/bits/types/struct_msqid64_ds.h
> @@ -20,9 +20,6 @@
>  # error "Never use <bits/msq.h> directly; include <sys/msg.h> instead."
>  #endif
>  
> -#if __TIMESIZE == 64
> -# define __msqid64_ds msqid_ds
> -#else
>  struct __msqid64_ds
>  {
>    struct ipc_perm msg_perm;	/* structure describing operation permission */
> @@ -35,4 +32,3 @@ struct __msqid64_ds
>    __pid_t msg_lspid;		/* pid of last msgsnd() */
>    __pid_t msg_lrpid;		/* pid of last msgrcv() */
>  };
> -#endif
>