[CRIU] [PATCHv2] test: static,aio01 -- Predefine SIGNAL_H

[Andrei Vagin]

On Tue, Jun 19, 2018 at 10:09:34AM -0700, Andrei Vagin wrote:
> On Tue, Jun 19, 2018 at 08:11:32AM +0100, Radostin Stoyanov wrote:
> > Hi Andrei,
> > 
> > Thank you for the feedback!
> > 
> > On 18/06/18 19:22, Andrei Vagin wrote:
> > > On Fri, Jun 15, 2018 at 09:09:50AM +0100, Radostin Stoyanov wrote:
> > >> After commit the commit below, <linux/signal.h> is included
> > >> in <linux/aio_abi.h>
> > >>
> > >> https://github.com/torvalds/linux/commit/7a074e96
> > > I don't like this hack. As minimum, we need a comment before this
> > > define.
> > I agree. Do you have better idea(s)?
> 
> What do we need from this header? It may be simpler to move everything
> into criu headers.
> 
> > > Can you report this problem into lkml?
> > I created a Bugzilla report.
> > https://bugzilla.kernel.org/show_bug.cgi?id=200081
> 
> "Nobody" uses the kernel bugzilla. It is better to report such problems
> into lkml and glibc mailing lists. You can download mbox from here
> https://patchwork.kernel.org/patch/10341635/ and send a replay to this
> patch.

I've read the code again and now I think there may be nothing wrong in the
kernel. We can't include linux/aio_abi.h and libaio.h, we can't include
linux/signal.h and signal.h. The linux/ headers represent linux API,
libaio.h and signal.h represents glibc (poxis) API. We probably have to
split using different API-s to separate object files.


Your patch has a bad side effect. sigset_t is defined in signal.h and in
linux/signal.h and there is no guarantee that they are equal, so with
your patch, the definition of __aio_sigset may be wrong, because the
wrong definition of sigset_t will be used.

include/uapi/linux/aio_abi.h:

#include <linux/signal.h>
struct __aio_sigset {
	sigset_t __user	*sigmask;
	size_t		sigsetsize;
};

> 
> > 
> > Should we report it somewhere else?
> > >
> > > Why do you fix only static/aio01? I see that criu can't be compiled too
> > Perhaps I missed something because CRIU compiled on the fedora rawhide
> > VM I used for testing.
> > 
> > How can I create the same test environment used by Travis?
> 
> make -C scripts/build/ fedora-rawhide
> 
> > >   CC       criu/pie/parasite.o
> > >   CC       criu/arch/x86/restorer.o
> > > [91mIn file included from /usr/include/asm/signal.h:7,
> > >                  from /usr/include/linux/signal.h:5,
> > >                  from /usr/include/linux/aio_abi.h:32,
> > >                  from criu/include/aio.h:4,
> > >                  from criu/pie/parasite.c:22:
> > > /usr/include/linux/time.h:10:8: error: redefinition of 'struct timespec'
> > >  struct timespec {
> > >         ^~~~~~~~
> > > [0m[91mIn file included from /usr/include/signal.h:53,
> > >                  from criu/pie/parasite.c:3:
> > > /usr/include/bits/types/struct_timespec.h:8:8: note: originally defined
> > > here
> > >  struct timespec
> > >         ^~~~~~~~
> > >
> > >> This causes compilation the following errors:
> > >>
> > >> In file included from /usr/include/linux/signal.h:5,
> > >>                  from /usr/include/linux/aio_abi.h:32,
> > >>                  from aio01.c:1:
> > >> /usr/include/asm/signal.h:127:2: error: unknown type name ‘size_t’
> > >>   size_t ss_size;
> > >>   ^~~~~~
> > >> In file included from aio01.c:1:
> > >> /usr/include/linux/aio_abi.h:112:2: error: unknown type name ‘size_t’
> > >>   size_t  sigsetsize;
> > >>   ^~~~~~
> > >> In file included from /usr/include/sys/select.h:33,
> > >>                  from /usr/include/sys/types.h:196,
> > >>                  from aio01.c:3:
> > >> /usr/include/bits/types/sigset_t.h:7:20: error: conflicting types for ‘sigset_t’
> > >>  typedef __sigset_t sigset_t;
> > >>                     ^~~~~~~~
> > >> In file included from /usr/include/linux/signal.h:5,
> > >>                  from /usr/include/linux/aio_abi.h:32,
> > >>                  from aio01.c:1:
> > >> /usr/include/asm/signal.h:16:23: note: previous declaration of ‘sigset_t’ was here
> > >>  typedef unsigned long sigset_t;
> > >>                        ^~~~~~~~
> > >>
> > >> As a workaround, we can predefine SIGNAL_H to ignore this include.
> > >>
> > >> Signed-off-by: Radostin Stoyanov <rstoyanov1 at gmail.com>
> > >> ---
> > >>  criu/include/aio.h       | 1 +
> > >>  test/zdtm/static/aio01.c | 1 +
> > >>  2 files changed, 2 insertions(+)
> > >>
> > >> diff --git a/criu/include/aio.h b/criu/include/aio.h
> > >> index 9a58089b..2d3c3851 100644
> > >> --- a/criu/include/aio.h
> > >> +++ b/criu/include/aio.h
> > >> @@ -1,6 +1,7 @@
> > >>  #ifndef __CR_AIO_H__
> > >>  #define __CR_AIO_H__
> > >>  
> > >> +#define SIGNAL_H
> > >>  #include <linux/aio_abi.h>
> > >>  #include "images/mm.pb-c.h"
> > >>  unsigned int aio_estimate_nr_reqs(unsigned int size);
> > >> diff --git a/test/zdtm/static/aio01.c b/test/zdtm/static/aio01.c
> > >> index f84fd359..4b61fcc9 100644
> > >> --- a/test/zdtm/static/aio01.c
> > >> +++ b/test/zdtm/static/aio01.c
> > >> @@ -1,3 +1,4 @@
> > >> +#define SIGNAL_H
> > >>  #include <linux/aio_abi.h>
> > >>  #include <sys/syscall.h>
> > >>  #include <sys/types.h>
> > >> -- 
> > >> 2.17.1
> > >>
> > >> _______________________________________________
> > >> CRIU mailing list
> > >> CRIU at openvz.org
> > >> https://lists.openvz.org/mailman/listinfo/criu
> > 
> > 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu