[CRIU] [PATCH 2/2] x86: ia32 -- Add needed bits to be compilable for x86-32

[Christopher Covington]

Hi Cyrill,

On 04/13/2015 04:10 PM, Cyrill Gorcunov wrote:

> diff --git a/Makefile b/Makefile
> index 6e6efb290823..71398f73494a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -51,6 +51,8 @@ ifeq ($(ARCH),i386)
>  	SRCARCH      := x86-32
>  	DEFINES      := -DCONFIG_X86_32
>  	VDSO         := y
> +	PROTOUFIX    := y
> +	export PROTOUFIX
>  endif
>  ifeq ($(ARCH),x86_64)
>  	SRCARCH      := x86
> @@ -58,6 +60,16 @@ ifeq ($(ARCH),x86_64)
>  	LDARCH       := i386:x86-64
>  	VDSO         := y
>  endif
> +ifeq ($(ARCH),ia32)
> +	SRCARCH      := x86
> +	DEFINES      := -DCONFIG_X86_32
> +	LDARCH       := i386
> +	ldflags-y    += -m elf_i386
> +	VDSO         := y
> +	USERCFLAGS   += -m32

Do you have any thoughts on USERCFLAGS versus CFLAGS? Ideally to follow the
usual convention CFLAGS could be set on the command line, on either side of
make (right-hand clobbers makefile assignments while left-hand side gets
clobbered makefile assignments). But that would have been a very invasive
change so I just introduced USERCFLAGS.

It might be cleaner to have USERCFLAGS only ever set by the user and any
makefile manipulation would be done on CFLAGS. Alternatively we could try to
gradually change everything to append to USERCFLAGS so that eventually CFLAGS
is equal to USERCFLAGS, CFLAGS can be properly used from the command line, and
USERCFLAGS is a small backwards compatibility layer that could maybe
eventually be deprecated and dropped.

> +	PROTOUFIX    := y
> +	export PROTOUFIX ldflags-y
> +endif
>  
>  ifeq ($(shell echo $(ARCH) | sed -e 's/arm.*/arm/'),arm)
>  	ARMV         := $(shell echo $(ARCH) | sed -nr 's/armv([[:digit:]]).*/\1/p; t; i7')
> @@ -78,6 +90,11 @@ ifeq ($(ARCH),aarch64)
>  	VDSO         := y
>  endif
>  
> +ifeq ($(SRCARCH),arm)
> +	PROTOUFIX    := y
> +	export PROTOUFIX
> +endif
> +
>  SRCARCH		?= $(ARCH)
>  LDARCH		?= $(SRCARCH)
>  
> @@ -197,7 +214,7 @@ $(SYSCALL-LIB) $(ARCH-LIB) $(PROGRAM-BUILTINS): config
>  
>  $(PROGRAM): $(SYSCALL-LIB) $(ARCH-LIB) $(PROGRAM-BUILTINS)
>  	$(E) "  LINK    " $@
> -	$(Q) $(CC) $(CFLAGS) $^ $(LIBS) $(LDFLAGS) $(GMONLDOPT) -rdynamic -o $@
> +	$(Q) $(CC) $(CFLAGS) $^ $(LIBS) $(GMONLDOPT) -rdynamic -o $@

I notice you're dropping LDFLAGS here and adding ldflags-y to some other
rules. Why is that?

>  crit:
>  	$(Q) $(MAKE) -C pycriu all
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 6f7eb132168d..35ad6d2db836 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -29,7 +29,7 @@ syscalls-asm-y-asmflags	:= -Wstrict-prototypes -Wa,--noexecstack
>  endif
>  syscalls-asm-y-asmflags += -nostdlib -fomit-frame-pointer -I$(obj)
>  
> -ifeq ($(ARCH),i386)
> +ifneq ($(filter i386 ia32,$(ARCH)),)
>  syscalls-obj-y += syscalls/syscall32.o
>  $(obj)/syscalls/syscall32.o: $(obj)/$(SYS-CODES) $(obj)/$(SYS-PROTO)
>  endif
> diff --git a/arch/x86/crtools.c b/arch/x86/crtools.c
> index 02ce2e5bc2ce..f5bc371206c6 100644
> --- a/arch/x86/crtools.c
> +++ b/arch/x86/crtools.c
> @@ -361,9 +361,9 @@ static void show_rt_xsave_frame(struct xsave_struct *x)
>  		 (int)i387->fop, (int)i387->mxcsr, (int)i387->mxcsr_mask);
>  
>  	pr_debug("magic1:%x extended_size:%x xstate_bv:%lx xstate_size:%x\n",
> -		 fpx->magic1, fpx->extended_size, fpx->xstate_bv, fpx->xstate_size);
> +		 fpx->magic1, fpx->extended_size, (long)fpx->xstate_bv, fpx->xstate_size);
>  
> -	pr_debug("xstate_bv: %lx\n", xsave_hdr->xstate_bv);
> +	pr_debug("xstate_bv: %lx\n", (long)xsave_hdr->xstate_bv);
>  
>  	pr_debug("-----------------------\n");
>  }
> @@ -443,8 +443,12 @@ void *mmap_seized(struct parasite_ctl *ctl,
>  	unsigned long map;
>  	int err;
>  
> +#ifdef CONFIG_X86_64
>  	err = syscall_seized(ctl, __NR_mmap, &map,
> -			(unsigned long)addr, length, prot, flags, fd, offset);
> +#else
> +	err = syscall_seized(ctl, __NR_mmap2, &map,
> +#endif
> +			     (unsigned long)addr, length, prot, flags, fd, offset);
>  	if (err < 0 || map > TASK_SIZE)
>  		map = 0;

In my opinion it'd be easier to read and maintain something like #define
PARASITE_MMAP_NR __NR_mmap in an x86_64 header, #define PARASITE_MMAP_NR
__NR_mmap2 in an ia32 header, and have one syscall_seized that uses
PARASITE_MMAP_NR in crtools.c.

Regards,
Chris

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project