[Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Laurent Vivier]

In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
defined if __NR_msgsnd is defined.

But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
__NR_semtimedop and it breaks the QEMU build.

__NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
but not in asm-mips/unistd_o32.h.

Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
It introduces __NR_semtimedop_time64 instead.

This patch fixes the problem by checking for each __NR_XXX symbol
before defining the corresponding syscall.

Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e311fcda0517..d316de25c9f2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -761,14 +761,7 @@ safe_syscall2(int, nanosleep, const struct timespec *, req,
 safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
               const struct timespec *, req, struct timespec *, rem)
 #endif
-#ifdef __NR_msgsnd
-safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
-              int, flags)
-safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
-              long, msgtype, int, flags)
-safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
-              unsigned, nsops, const struct timespec *, timeout)
-#else
+#if !defined(__NR_msgsnd) || !defined(__NR_msgrcv) || !defined(__NR_semtimedop)
 /* This host kernel architecture uses a single ipc syscall; fake up
  * wrappers for the sub-operations to hide this implementation detail.
  * Annoyingly we can't include linux/ipc.h to get the constant definitions
@@ -783,14 +776,29 @@ safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
 
 safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
               void *, ptr, long, fifth)
+#endif
+#ifdef __NR_msgsnd
+safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
+              int, flags)
+#else
 static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int flags)
 {
     return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void *)msgp, 0);
 }
+#endif
+#ifdef __NR_msgrcv
+safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
+              long, msgtype, int, flags)
+#else
 static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int flags)
 {
     return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
 }
+#endif
+#ifdef __NR_semtimedop
+safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
+              unsigned, nsops, const struct timespec *, timeout)
+#else
 static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned nsops,
                            const struct timespec *timeout)
 {
-- 
2.20.1

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Philippe Mathieu-Daudé]

On 5/23/19 7:54 PM, Laurent Vivier wrote:
> In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> defined if __NR_msgsnd is defined.
> 
> But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> __NR_semtimedop and it breaks the QEMU build.
> 
> __NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
> but not in asm-mips/unistd_o32.h.
> 
> Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
> It introduces __NR_semtimedop_time64 instead.
> 
> This patch fixes the problem by checking for each __NR_XXX symbol
> before defining the corresponding syscall.

Thanks for the quick fix Laurent.

> 
> Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  linux-user/syscall.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e311fcda0517..d316de25c9f2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -761,14 +761,7 @@ safe_syscall2(int, nanosleep, const struct timespec *, req,
>  safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
>                const struct timespec *, req, struct timespec *, rem)
>  #endif
> -#ifdef __NR_msgsnd
> -safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> -              int, flags)
> -safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> -              long, msgtype, int, flags)
> -safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> -              unsigned, nsops, const struct timespec *, timeout)
> -#else
> +#if !defined(__NR_msgsnd) || !defined(__NR_msgrcv) || !defined(__NR_semtimedop)
>  /* This host kernel architecture uses a single ipc syscall; fake up
>   * wrappers for the sub-operations to hide this implementation detail.
>   * Annoyingly we can't include linux/ipc.h to get the constant definitions
> @@ -783,14 +776,29 @@ safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
>  
>  safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
>                void *, ptr, long, fifth)
> +#endif
> +#ifdef __NR_msgsnd
> +safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> +              int, flags)
> +#else
>  static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int flags)
>  {
>      return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void *)msgp, 0);
>  }
> +#endif
> +#ifdef __NR_msgrcv
> +safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> +              long, msgtype, int, flags)
> +#else
>  static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int flags)
>  {
>      return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
>  }
> +#endif
> +#ifdef __NR_semtimedop
> +safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> +              unsigned, nsops, const struct timespec *, timeout)
> +#else
>  static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned nsops,
>                             const struct timespec *timeout)
>  {
> 

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Philippe Mathieu-Daudé]

On Thu, May 23, 2019 at 8:29 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
> On 5/23/19 7:54 PM, Laurent Vivier wrote:
> > In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> > defined if __NR_msgsnd is defined.
> >
> > But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> > __NR_semtimedop and it breaks the QEMU build.
> >
> > __NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
> > but not in asm-mips/unistd_o32.h.
> >
> > Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> > updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
> > It introduces __NR_semtimedop_time64 instead.
> >
> > This patch fixes the problem by checking for each __NR_XXX symbol
> > before defining the corresponding syscall.
>
> Thanks for the quick fix Laurent.
>
> >
> > Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Aleksandar, you have a pull request in preparation, if you agree with
this patch you might want to include it ;)

Regards,

Phil.

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Cornelia Huck]

On Thu, 23 May 2019 19:54:13 +0200
Laurent Vivier <laurent@vivier.eu> wrote:

> In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> defined if __NR_msgsnd is defined.
> 
> But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> __NR_semtimedop and it breaks the QEMU build.
> 
> __NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
> but not in asm-mips/unistd_o32.h.
> 
> Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
> It introduces __NR_semtimedop_time64 instead.

Thanks for looking into this! I still wonder whether something's b0rken
in the kernel, though. But that needs to be followed up by the mips
folks.

> 
> This patch fixes the problem by checking for each __NR_XXX symbol
> before defining the corresponding syscall.

This looks safe in any case.

> 
> Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/syscall.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Alex Bennée]

Laurent Vivier <laurent@vivier.eu> writes:

> In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> defined if __NR_msgsnd is defined.
>
> But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> __NR_semtimedop and it breaks the QEMU build.
>
> __NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
> but not in asm-mips/unistd_o32.h.
>
> Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
> It introduces __NR_semtimedop_time64 instead.
>
> This patch fixes the problem by checking for each __NR_XXX symbol
> before defining the corresponding syscall.
>
> Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  linux-user/syscall.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e311fcda0517..d316de25c9f2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -761,14 +761,7 @@ safe_syscall2(int, nanosleep, const struct timespec *, req,
>  safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
>                const struct timespec *, req, struct timespec *, rem)
>  #endif
> -#ifdef __NR_msgsnd
> -safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> -              int, flags)
> -safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> -              long, msgtype, int, flags)
> -safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> -              unsigned, nsops, const struct timespec *, timeout)
> -#else
> +#if !defined(__NR_msgsnd) || !defined(__NR_msgrcv) || !defined(__NR_semtimedop)
>  /* This host kernel architecture uses a single ipc syscall; fake up
>   * wrappers for the sub-operations to hide this implementation detail.
>   * Annoyingly we can't include linux/ipc.h to get the constant definitions
> @@ -783,14 +776,29 @@ safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
>
>  safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
>                void *, ptr, long, fifth)
> +#endif
> +#ifdef __NR_msgsnd
> +safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> +              int, flags)
> +#else
>  static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int flags)
>  {
>      return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void *)msgp, 0);
>  }
> +#endif
> +#ifdef __NR_msgrcv
> +safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> +              long, msgtype, int, flags)
> +#else
>  static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int flags)
>  {
>      return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
>  }
> +#endif
> +#ifdef __NR_semtimedop
> +safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> +              unsigned, nsops, const struct timespec *, timeout)
> +#else
>  static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned nsops,
>                             const struct timespec *timeout)
>  {


--
Alex Bennée

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Aleksandar Markovic]

On May 24, 2019 9:29 AM, "Alex Bennée" <alex.bennee@linaro.org> wrote:
>
>
> Laurent Vivier <laurent@vivier.eu> writes:
>
> > In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> > defined if __NR_msgsnd is defined.
> >
> > But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> > __NR_semtimedop and it breaks the QEMU build.
> >
> > __NR_semtimedop is defined in asm-mips/unistd_n64.h and
asm-mips/unistd_n32.h
> > but not in asm-mips/unistd_o32.h.
> >
> > Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> > updated asm-mips/unistd_o32.h and added __NR_msgsnd but not
__NR_semtimedop.
> > It introduces __NR_semtimedop_time64 instead.
> >
> > This patch fixes the problem by checking for each __NR_XXX symbol
> > before defining the corresponding syscall.
> >
> > Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
>

This patch was applied to mips queue, sent today.

Regards,
Aleksandar

> > ---
> >  linux-user/syscall.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index e311fcda0517..d316de25c9f2 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -761,14 +761,7 @@ safe_syscall2(int, nanosleep, const struct
timespec *, req,
> >  safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
> >                const struct timespec *, req, struct timespec *, rem)
> >  #endif
> > -#ifdef __NR_msgsnd
> > -safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> > -              int, flags)
> > -safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> > -              long, msgtype, int, flags)
> > -safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> > -              unsigned, nsops, const struct timespec *, timeout)
> > -#else
> > +#if !defined(__NR_msgsnd) || !defined(__NR_msgrcv) ||
!defined(__NR_semtimedop)
> >  /* This host kernel architecture uses a single ipc syscall; fake up
> >   * wrappers for the sub-operations to hide this implementation detail.
> >   * Annoyingly we can't include linux/ipc.h to get the constant
definitions
> > @@ -783,14 +776,29 @@ safe_syscall4(int, semtimedop, int, semid, struct
sembuf *, tsops,
> >
> >  safe_syscall6(int, ipc, int, call, long, first, long, second, long,
third,
> >                void *, ptr, long, fifth)
> > +#endif
> > +#ifdef __NR_msgsnd
> > +safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
> > +              int, flags)
> > +#else
> >  static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int
flags)
> >  {
> >      return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void
*)msgp, 0);
> >  }
> > +#endif
> > +#ifdef __NR_msgrcv
> > +safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
> > +              long, msgtype, int, flags)
> > +#else
> >  static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type,
int flags)
> >  {
> >      return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp,
type);
> >  }
> > +#endif
> > +#ifdef __NR_semtimedop
> > +safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
> > +              unsigned, nsops, const struct timespec *, timeout)
> > +#else
> >  static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned
nsops,
> >                             const struct timespec *timeout)
> >  {
>
>
> --
> Alex Bennée

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Laurent Vivier]

On 23/05/2019 19:54, Laurent Vivier wrote:
> In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
> defined if __NR_msgsnd is defined.
> 
> But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
> __NR_semtimedop and it breaks the QEMU build.
> 
> __NR_semtimedop is defined in asm-mips/unistd_n64.h and asm-mips/unistd_n32.h
> but not in asm-mips/unistd_o32.h.
> 
> Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
> updated asm-mips/unistd_o32.h and added __NR_msgsnd but not __NR_semtimedop.
> It introduces __NR_semtimedop_time64 instead.
> 
> This patch fixes the problem by checking for each __NR_XXX symbol
> before defining the corresponding syscall.
> 
> Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   linux-user/syscall.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)

This only fixes the problem at build time, but the changes in the kernel 
headers introduce also a regression at execution time:

if the host kernel doesn't implement the syscall, the syscall fails 
(ENOSYS) whereas it was working before because it was using ipc() 
instead. I have this problem with a Fedora 28 on ppc64 
(5.0.16-100.fc28.ppc64) (LTP test msgctl07).

I'm preparing a fix.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] linux-user: fix __NR_semtimedop undeclared error

[Aleksandar Markovic]

On May 28, 2019 6:25 PM, "Laurent Vivier" <laurent@vivier.eu> wrote:
>
> On 23/05/2019 19:54, Laurent Vivier wrote:
>>
>> In current code, __NR_msgrcv and__NR_semtimedop are supposed to be
>> defined if __NR_msgsnd is defined.
>>
>> But linux headers 5.2-rc1 for MIPS define __NR_msgsnd without defining
>> __NR_semtimedop and it breaks the QEMU build.
>>
>> __NR_semtimedop is defined in asm-mips/unistd_n64.h and
asm-mips/unistd_n32.h
>> but not in asm-mips/unistd_o32.h.
>>
>> Commit d9cb4336159a ("linux headers: update against Linux 5.2-rc1") has
>> updated asm-mips/unistd_o32.h and added __NR_msgsnd but not
__NR_semtimedop.
>> It introduces __NR_semtimedop_time64 instead.
>>
>> This patch fixes the problem by checking for each __NR_XXX symbol
>> before defining the corresponding syscall.
>>
>> Fixes: d9cb4336159a ("linux headers: update against Linux 5.2-rc1")
>> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>   linux-user/syscall.c | 24 ++++++++++++++++--------
>>   1 file changed, 16 insertions(+), 8 deletions(-)
>
>
> This only fixes the problem at build time, but the changes in the kernel
headers introduce also a regression at execution time:
>
> if the host kernel doesn't implement the syscall, the syscall fails
(ENOSYS) whereas it was working before because it was using ipc() instead.
I have this problem with a Fedora 28 on ppc64 (5.0.16-100.fc28.ppc64) (LTP
test msgctl07).
>
> I'm preparing a fix.
>

OK, great that you found this behavior, thanks. Just want to tell you that
meanwhile the original code of this patch got integrated into main source
tree.

Regards,
Aleksandar

> Thanks,
> Laurent