Re: [Qemu-devel] [PATCH] Get tests/tcg building, fix unused variable warning due to wrong extended asm operand, fix the 'test' make target.

[Peter Maydell]

On 13 July 2012 07:55, Catalin Patulea <address@hidden> wrote:
> Not all tests pass, but at least they can be run using 'make test'.
>
> To build individual tests:
> $ cd $BUILD_PATH/tests/tcg
> $ SRC_PATH=path/to/qemu make <target>

I don't know enough about our test makefile structure to
know if this is right (try Anthony) but some comments below.

(It looks like some of this is fixing bustage from Anthony's
commit c09015dd.)

> Signed-off-by: Catalin Patulea <address@hidden>
> ---
>  tests/tcg/Makefile    |   16 ++++++++++------
>  tests/tcg/test-i386.c |    3 ++-
>  tests/tcg/test_path.c |   13 +++++++------
>  3 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
> index 15e36a2..3256a98 100644
> --- a/tests/tcg/Makefile
> +++ b/tests/tcg/Makefile
> @@ -1,13 +1,13 @@
> --include ../config-host.mak
> +-include ../../config-host.mak
>  -include $(SRC_PATH)/rules.mak
>
> -$(call set-vpath, $(SRC_PATH)/tests)
> +$(call set-vpath, $(SRC_PATH)/tests/tcg)
>
> -QEMU=../i386-linux-user/qemu-i386
> -QEMU_X86_64=../x86_64-linux-user/qemu-x86_64
> +QEMU=../../i386-linux-user/qemu-i386
> +QEMU_X86_64=../../x86_64-linux-user/qemu-x86_64
>  CC_X86_64=$(CC_I386) -m64
>
> -QEMU_INCLUDES += -I..
> +QEMU_INCLUDES += -I../..
>  CFLAGS=-Wall -O2 -g -fno-strict-aliasing
>  #CFLAGS+=-msse2
>  LDFLAGS=
> @@ -36,6 +36,7 @@ TESTS += $(I386_TESTS)
>  endif
>
>  all: $(patsubst %,run-%,$(TESTS))
> +test: all
>
>  # rules to run tests
>
> @@ -74,7 +75,10 @@ run-test_path: test_path
>  # rules to compile tests
>
>  test_path: test_path.o
> +       $(CC_I386) $(LDFLAGS) $(LIBS) -o $@ $^

This won't compile:
ccache gcc -m32  -lrt -pthread -lgthread-2.0 -lrt -lglib-2.0    -o
test_path test_path.o
test_path.o: In function `qemu_iovec_init':
/home/pm215/src/qemu/qemu/cutils.c:142: undefined reference to `g_malloc'
test_path.o: In function `qemu_iovec_add':
/home/pm215/src/qemu/qemu/cutils.c:166: undefined reference to `g_realloc'
test_path.o: In function `qemu_iovec_destroy':
/home/pm215/src/qemu/qemu/cutils.c:210: undefined reference to `g_free'
collect2: ld returned 1 exit status

You need to have the $(LIBS) after the $^.

> +
>  test_path.o: test_path.c
> +       $(CC_I386) $(QEMU_INCLUDES) $(GLIB_CFLAGS) $(CFLAGS) $(LDFLAGS) -c -o 
> $@ $^

Ditto.

>  hello-i386: hello-i386.c
>         $(CC_I386) -nostdlib $(CFLAGS) -static $(LDFLAGS) -o $@ $<
> @@ -86,7 +90,7 @@ testthread: testthread.c
>  # i386/x86_64 emulation test (test various opcodes) */
>  test-i386: test-i386.c test-i386-code16.S test-i386-vm86.S \
>             test-i386.h test-i386-shift.h test-i386-muldiv.h
> -       $(CC_I386) $(CFLAGS) $(LDFLAGS) -o $@ \
> +       $(CC_I386) $(QEMU_INCLUDES) $(CFLAGS) $(LDFLAGS) -o $@ \
>                $(<D)/test-i386.c $(<D)/test-i386-code16.S 
> $(<D)/test-i386-vm86.S -lm
>
>  test-x86_64: test-i386.c \
> diff --git a/tests/tcg/test-i386.c b/tests/tcg/test-i386.c
> index 8e64bba..acbc9c9 100644
> --- a/tests/tcg/test-i386.c
> +++ b/tests/tcg/test-i386.c
> @@ -17,6 +17,7 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>  #define _GNU_SOURCE
> +#include "compiler.h"  // QEMU_PACKED

checkpatch complains about this //-syntax comment;
might as well just drop it.

>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <string.h>
> @@ -784,7 +785,7 @@ void fpu_clear_exceptions(void)
>          long double fpregs[8];
>      } float_env32;
>
> -    asm volatile ("fnstenv %0\n" : : "m" (float_env32));
> +    asm volatile ("fnstenv %0\n" : "=m" (float_env32));
>      float_env32.fpus &= ~0x7f;
>      asm volatile ("fldenv %0\n" : : "m" (float_env32));
>  }

This doesn't seem to be sufficient for test-i386 to compile
for me:
ccache gcc -m32 -I/home/pm215/src/qemu/qemu/slirp -I.
-I/home/pm215/src/qemu/qemu -I/home/pm215/src/qemu/qemu/fpu -I../..
-Wall -O2 -g -fno-strict-aliasing  -o test-i386 \
              ./test-i386.c ./test-i386-code16.S ./test-i386-vm86.S -lm
./test-i386.c: Assembler messages:
./test-i386.c:1831: Error: expecting lockable instruction after `lock'

Since 'lock nop' isn't valid we probably need to do this with
a .byte directive to emit the sequence we require.
(this is gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1 on x86.)

> diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
> index 7265a94..a064eea 100644
> --- a/tests/tcg/test_path.c
> +++ b/tests/tcg/test_path.c
> @@ -1,11 +1,12 @@
>  /* Test path override code */
> -#include "../config-host.h"
> -#include "../qemu-malloc.c"
> -#include "../cutils.c"
> -#include "../path.c"
> -#include "../trace.c"
> +#define _GNU_SOURCE
> +#include "config-host.h"
> +#include "iov.c"
> +#include "cutils.c"
> +#include "path.c"
> +#include "trace.c"
>  #ifdef CONFIG_TRACE_SIMPLE
> -#include "../simpletrace.c"
> +#include "../trace/simple.c"
>  #endif
>
>  #include <stdarg.h>

The whole way this test works looks pretty foul to me, so yeah, whatever :-)

-- PMM