[Qemu-devel] [PATCH 0/2] build fix for clang 3.4

[Qemu-devel] [PATCH 0/2] build fix for clang 3.4

[Richard Henderson]

Alex pointed out that my last attempt to get things all working
falls afoul of old clang, in use by our Travis CI.

This is tested with clang 3.7, but with examination of the build
logs to see that the system assembler is used for the optionroms.


r~


Richard Henderson (2):
  linux-user: Fix i386 safe-syscall.S
  build: Use $(AS) for optionrom explicitly

 configure                               | 10 +++-------
 linux-user/host/i386/safe-syscall.inc.S | 24 ++++++------------------
 pc-bios/optionrom/Makefile              |  3 +++
 rules.mak                               |  2 +-
 4 files changed, 13 insertions(+), 26 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/2] linux-user: Fix i386 safe-syscall.S

[Richard Henderson]

Clang insists that "cmp" is ambiguous with a memory destination,
requiring an explicit size suffix.

There was a true error in the use of .cfi_def_cfa_offset in the
epilogue, but changing to use the proper .cfi_adjust_cfa_offset
runs afoul of a clang bug wrt .cfi_restore_state.  Better to
fold the two epilogues so that we don't trigger the bug.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/host/i386/safe-syscall.inc.S | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/linux-user/host/i386/safe-syscall.inc.S b/linux-user/host/i386/safe-syscall.inc.S
index 766d0de..9e58fc6 100644
--- a/linux-user/host/i386/safe-syscall.inc.S
+++ b/linux-user/host/i386/safe-syscall.inc.S
@@ -69,7 +69,7 @@ safe_syscall_base:
 safe_syscall_start:
 	/* if signal_pending is non-zero, don't do the call */
 	mov	4+16(%esp), %eax	/* signal_pending */
-	cmp	$0, (%eax)
+	cmpl	$0, (%eax)
 	jnz	1f
 	mov	8+16(%esp), %eax	/* syscall number */
 	int	$0x80
@@ -77,16 +77,16 @@ safe_syscall_end:
 	/* code path for having successfully executed the syscall */
 	pop	%ebx
 	.cfi_remember_state
-	.cfi_def_cfa_offset -4
+	.cfi_adjust_cfa_offset -4
 	.cfi_restore ebx
 	pop	%edi
-	.cfi_def_cfa_offset -4
+	.cfi_adjust_cfa_offset -4
 	.cfi_restore edi
 	pop	%esi
-	.cfi_def_cfa_offset -4
+	.cfi_adjust_cfa_offset -4
 	.cfi_restore esi
 	pop	%ebp
-	.cfi_def_cfa_offset -4
+	.cfi_adjust_cfa_offset -4
 	.cfi_restore ebp
 	ret
 
@@ -94,19 +94,7 @@ safe_syscall_end:
 	/* code path when we didn't execute the syscall */
 	.cfi_restore_state
 	mov	$-TARGET_ERESTARTSYS, %eax
-	pop	%ebx
-	.cfi_def_cfa_offset -4
-	.cfi_restore ebx
-	pop	%edi
-	.cfi_def_cfa_offset -4
-	.cfi_restore edi
-	pop	%esi
-	.cfi_def_cfa_offset -4
-	.cfi_restore esi
-	pop	%ebp
-	.cfi_def_cfa_offset -4
-	.cfi_restore ebp
-	ret
+	jmp	safe_syscall_end
 	.cfi_endproc
 
 	.size	safe_syscall_base, .-safe_syscall_base
-- 
2.7.4

[Qemu-devel] [PATCH 2/2] build: Use $(AS) for optionrom explicitly

[Richard Henderson]

For clang before 3.5, -fno-integrated-as does not exist,
so the workaround in 5f6f0e27fb24 fails to build.

Use clang's default assembler for linux-user/safe-syscall.S,
and explicitly change to use the system assembler for the
option roms.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 configure                  | 10 +++-------
 pc-bios/optionrom/Makefile |  3 +++
 rules.mak                  |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index e9090a0..5ada56d 100755
--- a/configure
+++ b/configure
@@ -368,6 +368,7 @@ else
 fi
 
 ar="${AR-${cross_prefix}ar}"
+as="${AS-${cross_prefix}as}"
 ccas="${CCAS-$cc}"
 cpp="${CPP-$cc -E}"
 objcopy="${OBJCOPY-${cross_prefix}objcopy}"
@@ -4490,13 +4491,6 @@ if test "$fortify_source" != "no"; then
   fi
 fi
 
-#################################################
-# clang does not support the 16-bit assembly for roms
-
-if echo | $ccas -dM -E - | grep __clang__ > /dev/null 2>&1 ; then
-  ccas="$ccas -fno-integrated-as"
-fi
-
 ##########################################
 # check if struct fsxattr is available via linux/fs.h
 
@@ -5515,6 +5509,7 @@ echo "CXX=$cxx" >> $config_host_mak
 echo "OBJCC=$objcc" >> $config_host_mak
 echo "AR=$ar" >> $config_host_mak
 echo "ARFLAGS=$ARFLAGS" >> $config_host_mak
+echo "AS=$as" >> $config_host_mak
 echo "CCAS=$ccas" >> $config_host_mak
 echo "CPP=$cpp" >> $config_host_mak
 echo "OBJCOPY=$objcopy" >> $config_host_mak
@@ -5988,6 +5983,7 @@ for rom in seabios vgabios ; do
     config_mak=roms/$rom/config.mak
     echo "# Automatically generated by configure - do not modify" > $config_mak
     echo "SRC_PATH=$source_path/roms/$rom" >> $config_mak
+    echo "AS=$as" >> $config_mak
     echo "CCAS=$ccas" >> $config_mak
     echo "CC=$cc" >> $config_mak
     echo "BCC=bcc" >> $config_mak
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index ce4852a..2cdda87 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -20,6 +20,9 @@ build-all: multiboot.bin linuxboot.bin kvmvapic.bin
 # suppress auto-removal of intermediate files
 .SECONDARY:
 
+%.o: %.S
+	$(call quiet-command,$(CPP) $(QEMU_INCLUDES) $(QEMU_DGFLAGS) $(CFLAGS) -c -o - $< | $(AS) $(ASFLAGS) -o $@,"  AS    $(TARGET_DIR)$@")
+
 %.img: %.o
 	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
 
diff --git a/rules.mak b/rules.mak
index 7d7d83b..ed8e482 100644
--- a/rules.mak
+++ b/rules.mak
@@ -69,7 +69,7 @@ LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o
        $(version-obj-y) $(call extract-libs,$1) $(LIBS),"  LINK  $(TARGET_DIR)$@")
 
 %.o: %.S
-	$(call quiet-command,$(CCAS) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  AS    $(TARGET_DIR)$@")
+	$(call quiet-command,$(CCAS) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CCAS  $(TARGET_DIR)$@")
 
 %.o: %.cc
 	$(call quiet-command,$(CXX) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  CXX   $(TARGET_DIR)$@")
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/2] build: Use $(AS) for optionrom explicitly

[Alex Bennée]

Richard Henderson <rth@twiddle.net> writes:

> For clang before 3.5, -fno-integrated-as does not exist,
> so the workaround in 5f6f0e27fb24 fails to build.
>
> Use clang's default assembler for linux-user/safe-syscall.S,
> and explicitly change to use the system assembler for the
> option roms.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  configure                  | 10 +++-------
>  pc-bios/optionrom/Makefile |  3 +++
>  rules.mak                  |  2 +-
>  3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/configure b/configure
> index e9090a0..5ada56d 100755
> --- a/configure
> +++ b/configure
> @@ -368,6 +368,7 @@ else
>  fi
>  
>  ar="${AR-${cross_prefix}ar}"
> +as="${AS-${cross_prefix}as}"
>  ccas="${CCAS-$cc}"
>  cpp="${CPP-$cc -E}"
>  objcopy="${OBJCOPY-${cross_prefix}objcopy}"
> @@ -4490,13 +4491,6 @@ if test "$fortify_source" != "no"; then
>    fi
>  fi
>  
> -#################################################
> -# clang does not support the 16-bit assembly for roms
> -
> -if echo | $ccas -dM -E - | grep __clang__ > /dev/null 2>&1 ; then
> -  ccas="$ccas -fno-integrated-as"
> -fi
> -
>  ##########################################
>  # check if struct fsxattr is available via linux/fs.h
>  
> @@ -5515,6 +5509,7 @@ echo "CXX=$cxx" >> $config_host_mak
>  echo "OBJCC=$objcc" >> $config_host_mak
>  echo "AR=$ar" >> $config_host_mak
>  echo "ARFLAGS=$ARFLAGS" >> $config_host_mak
> +echo "AS=$as" >> $config_host_mak
>  echo "CCAS=$ccas" >> $config_host_mak
>  echo "CPP=$cpp" >> $config_host_mak
>  echo "OBJCOPY=$objcopy" >> $config_host_mak
> @@ -5988,6 +5983,7 @@ for rom in seabios vgabios ; do
>      config_mak=roms/$rom/config.mak
>      echo "# Automatically generated by configure - do not modify" > $config_mak
>      echo "SRC_PATH=$source_path/roms/$rom" >> $config_mak
> +    echo "AS=$as" >> $config_mak
>      echo "CCAS=$ccas" >> $config_mak
>      echo "CC=$cc" >> $config_mak
>      echo "BCC=bcc" >> $config_mak
> diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> index ce4852a..2cdda87 100644
> --- a/pc-bios/optionrom/Makefile
> +++ b/pc-bios/optionrom/Makefile
> @@ -20,6 +20,9 @@ build-all: multiboot.bin linuxboot.bin kvmvapic.bin
>  # suppress auto-removal of intermediate files
>  .SECONDARY:
>  
> +%.o: %.S
> +	$(call quiet-command,$(CPP) $(QEMU_INCLUDES) $(QEMU_DGFLAGS) $(CFLAGS) -c -o - $< | $(AS) $(ASFLAGS) -o $@,"  AS    $(TARGET_DIR)$@")
> +
>  %.img: %.o
>  	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
>  
> diff --git a/rules.mak b/rules.mak
> index 7d7d83b..ed8e482 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -69,7 +69,7 @@ LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o
>         $(version-obj-y) $(call extract-libs,$1) $(LIBS),"  LINK  $(TARGET_DIR)$@")
>  
>  %.o: %.S
> -	$(call quiet-command,$(CCAS) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  AS    $(TARGET_DIR)$@")
> +	$(call quiet-command,$(CCAS) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CCAS  $(TARGET_DIR)$@")
>  
>  %.o: %.cc
>  	$(call quiet-command,$(CXX) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  CXX   $(TARGET_DIR)$@")

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

-- 
Alex Bennée