[Qemu-trivial] [PATCH] tests/tcg: Fix compilation of test_path

[Qemu-trivial] [PATCH] tests/tcg: Fix compilation of test_path

[Peter Maydell]

The test_path binary is (unlike the other test binaries in tests/tcg)
actually intended to be compiled with the same compiler used to build
the main QEMU executables. It actually #includes a number of the
QEMU source files in an attempt to unit-test the util/path.c functions,
and so if it is not compiled with the same compiler used by configure
to set CONFIG_ settings then it is liable to fail to build.
Fix the makefile to build it with CC, not CC_I386, and fix the test
itself not to include a lot of unnecessary trace related source
files which cause the build to fail if the trace backend is anything
other than 'simple'.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The particular build failure you usually hit is that CC is an
x86-64 compiler supporting __int128_t whereas CC_I386 does not,
and then the code in the headers using those types blows up.

The whole attempt to unit test by compiling bits of the .c files
seems terribly fragile to me; this is the only test we have that
does it, so if anybody has a better approach to testing these path.c
functions I'd like to know.

Stefan, you added the trace .c files in commit 6d4adef48dd6bb but
I can't work out why they're needed -- the test builds fine for
me without them whether we configured using the default or simple
backends, and it definitely doesn't compile if we used the default
backend and the test includes the trace .c files...

 tests/tcg/Makefile    | 4 ++--
 tests/tcg/test_path.c | 6 ------
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
index 24e3154..2ffa067 100644
--- a/tests/tcg/Makefile
+++ b/tests/tcg/Makefile
@@ -81,10 +81,10 @@ run-test_path: test_path
 # rules to compile tests
 
 test_path: test_path.o
-	$(CC_I386) $(LDFLAGS) -o $@ $^ $(LIBS)
+	$(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
 
 test_path.o: test_path.c
-	$(CC_I386) $(QEMU_INCLUDES) $(GLIB_CFLAGS) $(CFLAGS) -c -o $@ $^
+	$(CC) $(QEMU_INCLUDES) $(GLIB_CFLAGS) $(CFLAGS) -c -o $@ $^
 
 hello-i386: hello-i386.c
 	$(CC_I386) -nostdlib $(CFLAGS) -static $(LDFLAGS) -o $@ $<
diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
index f8dd36a..c77fdbf 100644
--- a/tests/tcg/test_path.c
+++ b/tests/tcg/test_path.c
@@ -6,12 +6,6 @@
 #include "util/iov.c"
 #include "util/path.c"
 #include "util/qemu-timer-common.c"
-#include "trace/control.c"
-#include "../trace/generated-events.c"
-#ifdef CONFIG_TRACE_SIMPLE
-#include "trace/simple.c"
-#endif
-
 #include <stdarg.h>
 #include <sys/stat.h>
 #include <fcntl.h>
-- 
1.9.2

Re: [Qemu-trivial] [Qemu-devel] [PATCH] tests/tcg: Fix compilation of test_path

[Peter Maydell]

On 28 April 2014 20:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> --- a/tests/tcg/Makefile
> +++ b/tests/tcg/Makefile
> @@ -81,10 +81,10 @@ run-test_path: test_path
>  # rules to compile tests
>
>  test_path: test_path.o
> -       $(CC_I386) $(LDFLAGS) -o $@ $^ $(LIBS)
> +       $(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
>
>  test_path.o: test_path.c
> -       $(CC_I386) $(QEMU_INCLUDES) $(GLIB_CFLAGS) $(CFLAGS) -c -o $@ $^
> +       $(CC) $(QEMU_INCLUDES) $(GLIB_CFLAGS) $(CFLAGS) -c -o $@ $^

In fact we can drop these CC lines altogether (going back
to the situation before commit f62cb1b6d) -- we failed to
notice then that test_path is an oddball, being a host
binary rather than a target binary, unlike all the other
files being compiled in this directory.

(Also means _GNU_SOURCE needs to be removed from the
.c file as otherwise the compiler complains that it's
defined on the command line and in the c file.)

thanks
-- PMM

Re: [Qemu-trivial] [PATCH] tests/tcg: Fix compilation of test_path

[Stefan Weil]

Am 28.04.2014 21:35, schrieb Peter Maydell:
> The test_path binary is (unlike the other test binaries in tests/tcg)
> actually intended to be compiled with the same compiler used to build
> the main QEMU executables. It actually #includes a number of the
> QEMU source files in an attempt to unit-test the util/path.c functions,
> and so if it is not compiled with the same compiler used by configure
> to set CONFIG_ settings then it is liable to fail to build.
> Fix the makefile to build it with CC, not CC_I386, and fix the test
> itself not to include a lot of unnecessary trace related source
> files which cause the build to fail if the trace backend is anything
> other than 'simple'.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The particular build failure you usually hit is that CC is an
> x86-64 compiler supporting __int128_t whereas CC_I386 does not,
> and then the code in the headers using those types blows up.
> 
> The whole attempt to unit test by compiling bits of the .c files
> seems terribly fragile to me; this is the only test we have that
> does it, so if anybody has a better approach to testing these path.c
> functions I'd like to know.
> 
> Stefan, you added the trace .c files in commit 6d4adef48dd6bb but
> I can't work out why they're needed -- the test builds fine for
> me without them whether we configured using the default or simple
> backends, and it definitely doesn't compile if we used the default
> backend and the test includes the trace .c files...

Before my commit, a trace.c was included which was no longer available,
so I simply replaced it by its successor (which needed additional trace
.c files). I can confirm that removing those trace files also works.
That's of course a better solution.

Stefan