[RFC PATCH 0/3] check-tcg hacks for BSD

[RFC PATCH 0/3] check-tcg hacks for BSD

[Alex Bennée]

Hi Warner,

Here are some hacks I made to nominally get the check-tcg system
working on the BSD user builds. The first step was installing GCC as
we skip clang for x86 builds due to inline assembly issues:

  ../src/configure --disable-system --enable-user \
    --python=/usr/local/bin/python3.7 --cross-cc-x86_64=/usr/local/bin/gcc10

and then at least "gmake build-tcg" generates some binaries. You
should also be able to drop a simple helloworld.c in
tests/tcg/multiarch and have something simple to start with. At the
moment all apart from sha1 segfault when run native. None of them run
under the user mode emulation. I leave figuring that out to the BSD
experts.

Alex Bennée (3):
  configure: don't override the selected host test compiler if defined
  tests/tcg/sha1: remove endian include
  tests/tcg: commit Makefile atrocities in the name of portability

 configure                           | 7 +++++--
 tests/tcg/multiarch/sha1.c          | 1 -
 tests/tcg/multiarch/Makefile.target | 6 +++++-
 tests/tcg/x86_64/Makefile.target    | 4 ++++
 4 files changed, 14 insertions(+), 4 deletions(-)

-- 
2.30.2

[RFC PATCH 1/3] configure: don't override the selected host test compiler if defined

[Alex Bennée]

There are not many cases you would want to do this but one is if you
want to use a test friendly compiler like gcc instead of a system
compiler like clang. Either way we should honour the users choice if
they have made it.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Warner Losh <imp@bsdimp.com>
---
 configure | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 9a79a004d7..eadc434144 100755
--- a/configure
+++ b/configure
@@ -1691,8 +1691,11 @@ case "$cpu" in
     # No special flags required for other host CPUs
 esac
 
-eval "cross_cc_${cpu}=\$cc"
-cross_cc_vars="$cross_cc_vars cross_cc_${cpu}"
+if eval test -z "\${cross_cc_$cpu}"; then
+    eval "cross_cc_${cpu}=\$cc"
+    cross_cc_vars="$cross_cc_vars cross_cc_${cpu}"
+fi
+
 QEMU_CFLAGS="$CPU_CFLAGS $QEMU_CFLAGS"
 
 # For user-mode emulation the host arch has to be one we explicitly
-- 
2.30.2

[RFC PATCH 2/3] tests/tcg/sha1: remove endian include

[Alex Bennée]

This doesn't exist in BSD world and doesn't seem to be needed by
either.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Warner Losh <imp@bsdimp.com>
---
 tests/tcg/multiarch/sha1.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/tcg/multiarch/sha1.c b/tests/tcg/multiarch/sha1.c
index 87bfbcdf52..0081bd7657 100644
--- a/tests/tcg/multiarch/sha1.c
+++ b/tests/tcg/multiarch/sha1.c
@@ -43,7 +43,6 @@ void SHA1Init(SHA1_CTX* context);
 void SHA1Update(SHA1_CTX* context, const unsigned char* data, uint32_t len);
 void SHA1Final(unsigned char digest[20], SHA1_CTX* context);
 /* ================ end of sha1.h ================ */
-#include <endian.h>
 
 #define rol(value, bits) (((value) << (bits)) | ((value) >> (32 - (bits))))
 
-- 
2.30.2

[RFC PATCH 3/3] tests/tcg: commit Makefile atrocities in the name of portability

[Alex Bennée]

Not all of the multiarch tests are pure POSIX so elide over those
tests on a non-Linux system. This allows for at least some of the
tests to be nominally usable by *BSD user builds.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Warner Losh <imp@bsdimp.com>
---
 tests/tcg/multiarch/Makefile.target | 6 +++++-
 tests/tcg/x86_64/Makefile.target    | 4 ++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 85a6fb7a2e..38ee0f1dec 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -10,7 +10,11 @@ MULTIARCH_SRC=$(SRC_PATH)/tests/tcg/multiarch
 # Set search path for all sources
 VPATH 		+= $(MULTIARCH_SRC)
 MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
-MULTIARCH_TESTS  =$(filter-out float_helpers, $(MULTIARCH_SRCS:.c=))
+MULTIARCH_SKIP=float_helpers
+ifeq ($(CONFIG_LINUX),)
+MULTIARCH_SKIP+=linux-test
+endif
+MULTIARCH_TESTS  =$(filter-out $(MULTIARCH_SKIP),$(MULTIARCH_SRCS:.c=))
 
 #
 # The following are any additional rules needed to build things
diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
index 2151ea6302..d7a7385583 100644
--- a/tests/tcg/x86_64/Makefile.target
+++ b/tests/tcg/x86_64/Makefile.target
@@ -8,8 +8,12 @@
 
 include $(SRC_PATH)/tests/tcg/i386/Makefile.target
 
+ifneq ($(CONFIG_LINUX),)
 X86_64_TESTS += vsyscall
 TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
+else
+TESTS=$(MULTIARCH_TESTS)
+endif
 QEMU_OPTS += -cpu max
 
 test-x86_64: LDFLAGS+=-lm -lc
-- 
2.30.2

Re: [RFC PATCH 2/3] tests/tcg/sha1: remove endian include

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

On Tue, Aug 3, 2021, 5:02 AM Alex Bennée <alex.bennee@linaro.org> wrote:

> This doesn't exist in BSD world and doesn't seem to be needed by
> either.
>

Sys/endian.h is common. FreeBSD has endian.h, but others don't.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Warner Losh <imp@bsdimp.com>
>

Acked by: Warner Losh <imp@bsdimp.com>

> ---
>  tests/tcg/multiarch/sha1.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/tests/tcg/multiarch/sha1.c b/tests/tcg/multiarch/sha1.c
> index 87bfbcdf52..0081bd7657 100644
> --- a/tests/tcg/multiarch/sha1.c
> +++ b/tests/tcg/multiarch/sha1.c
> @@ -43,7 +43,6 @@ void SHA1Init(SHA1_CTX* context);
>  void SHA1Update(SHA1_CTX* context, const unsigned char* data, uint32_t
> len);
>  void SHA1Final(unsigned char digest[20], SHA1_CTX* context);
>  /* ================ end of sha1.h ================ */
> -#include <endian.h>
>
>  #define rol(value, bits) (((value) << (bits)) | ((value) >> (32 -
> (bits))))
>
> --
> 2.30.2
>
>

[-- Attachment #2: Type: text/html, Size: 2027 bytes --]

Re: [RFC PATCH 2/3] tests/tcg/sha1: remove endian include

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

On Tue, Aug 3, 2021 at 7:55 AM Warner Losh <imp@bsdimp.com> wrote:

>
>
> On Tue, Aug 3, 2021, 5:02 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> This doesn't exist in BSD world and doesn't seem to be needed by
>> either.
>>
>
> Sys/endian.h is common. FreeBSD has endian.h, but others don't.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Warner Losh <imp@bsdimp.com>
>>
>
> Acked by: Warner Losh <imp@bsdimp.com>
>

On second thought, this is

Reviewed by: Warner Losh <imp@bsdimp.com>

since I know the change is good.

Warner


> ---
>>  tests/tcg/multiarch/sha1.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/tests/tcg/multiarch/sha1.c b/tests/tcg/multiarch/sha1.c
>> index 87bfbcdf52..0081bd7657 100644
>> --- a/tests/tcg/multiarch/sha1.c
>> +++ b/tests/tcg/multiarch/sha1.c
>> @@ -43,7 +43,6 @@ void SHA1Init(SHA1_CTX* context);
>>  void SHA1Update(SHA1_CTX* context, const unsigned char* data, uint32_t
>> len);
>>  void SHA1Final(unsigned char digest[20], SHA1_CTX* context);
>>  /* ================ end of sha1.h ================ */
>> -#include <endian.h>
>>
>>  #define rol(value, bits) (((value) << (bits)) | ((value) >> (32 -
>> (bits))))
>>
>> --
>> 2.30.2
>>
>>

[-- Attachment #2: Type: text/html, Size: 2905 bytes --]

Re: [RFC PATCH 3/3] tests/tcg: commit Makefile atrocities in the name of portability

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 4136 bytes --]

On Tue, Aug 3, 2021 at 5:02 AM Alex Bennée <alex.bennee@linaro.org> wrote:

> Not all of the multiarch tests are pure POSIX so elide over those
> tests on a non-Linux system. This allows for at least some of the
> tests to be nominally usable by *BSD user builds.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Warner Losh <imp@bsdimp.com>
> ---
>  tests/tcg/multiarch/Makefile.target | 6 +++++-
>  tests/tcg/x86_64/Makefile.target    | 4 ++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
>

Acked-by: Warner Losh <imp@bsdimp.com>

To do this with gcc10, however, I had to add -Wno-error=overflow
otherwise I got a lot of warnings about constants being truncated to
0.

It also fails the sha1 test, but when I run it by hand it works. It turns
out that I have a sha1 in my path, and at least in the bsd-user edition
of qemu-i386 tries to run that and fails.

Also, the hello world program needed tweaking

So with this applied and the following patch

diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 63cf1b2573..39420631a8 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -155,7 +155,7 @@ RUN_TESTS+=$(EXTRA_RUNS)

 ifdef CONFIG_USER_ONLY
 run-%: %
-       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) $<, "$< on
$(TARGET_NAME)")
+       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) ./$<, "$< on
$(TARGET_NAME)")

 run-plugin-%:
        $(call run-test, $@, $(QEMU) $(QEMU_OPTS) \
@@ -168,7 +168,7 @@ run-%: %
        $(call run-test, $<, \
          $(QEMU) -monitor none -display none \
                  -chardev file$(COMMA)path=$<.out$(COMMA)id=output \
-                 $(QEMU_OPTS) $<, \
+                 $(QEMU_OPTS) ./$<, \
          "$< on $(TARGET_NAME)")

 run-plugin-%:
diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
index a053ca3f15..ae258c47f0 100644
--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -21,6 +21,7 @@ run-plugin-test-i386-pcmpistri-%: QEMU_OPTS += -cpu max
 run-test-i386-bmi2: QEMU_OPTS += -cpu max
 run-plugin-test-i386-bmi2-%: QEMU_OPTS += -cpu max

+CFLAGS +=  -Wno-error=overflow
 #
 # hello-i386 is a barebones app
 #
diff --git a/tests/tcg/i386/hello-i386.c b/tests/tcg/i386/hello-i386.c
index 59196dd0b7..4a5a25211c 100644
--- a/tests/tcg/i386/hello-i386.c
+++ b/tests/tcg/i386/hello-i386.c
@@ -1,4 +1,10 @@
+#ifdef __FreeBSD__
+#include <sys/syscall.h>
+#define __NR_exit SYS_exit
+#define __NR_write SYS_write
+#else
 #include <asm/unistd.h>
+#endif

 static inline void exit(int status)
 {

I get down to a failure i the mmap test.... and that's all I have time to
plumb the depths
of this morning... Investigating the mmap test failure will have to wait
for another day.

Warner


> diff --git a/tests/tcg/multiarch/Makefile.target
> b/tests/tcg/multiarch/Makefile.target
> index 85a6fb7a2e..38ee0f1dec 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -10,7 +10,11 @@ MULTIARCH_SRC=$(SRC_PATH)/tests/tcg/multiarch
>  # Set search path for all sources
>  VPATH          += $(MULTIARCH_SRC)
>  MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
> -MULTIARCH_TESTS  =$(filter-out float_helpers, $(MULTIARCH_SRCS:.c=))
> +MULTIARCH_SKIP=float_helpers
> +ifeq ($(CONFIG_LINUX),)
> +MULTIARCH_SKIP+=linux-test
> +endif
> +MULTIARCH_TESTS  =$(filter-out $(MULTIARCH_SKIP),$(MULTIARCH_SRCS:.c=))
>
>  #
>  # The following are any additional rules needed to build things
> diff --git a/tests/tcg/x86_64/Makefile.target
> b/tests/tcg/x86_64/Makefile.target
> index 2151ea6302..d7a7385583 100644
> --- a/tests/tcg/x86_64/Makefile.target
> +++ b/tests/tcg/x86_64/Makefile.target
> @@ -8,8 +8,12 @@
>
>  include $(SRC_PATH)/tests/tcg/i386/Makefile.target
>
> +ifneq ($(CONFIG_LINUX),)
>  X86_64_TESTS += vsyscall
>  TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
> +else
> +TESTS=$(MULTIARCH_TESTS)
> +endif
>  QEMU_OPTS += -cpu max
>
>  test-x86_64: LDFLAGS+=-lm -lc
> --
> 2.30.2
>
>

[-- Attachment #2: Type: text/html, Size: 5307 bytes --]

Re: [RFC PATCH 3/3] tests/tcg: commit Makefile atrocities in the name of portability

[Alex Bennée]

Warner Losh <imp@bsdimp.com> writes:

> On Tue, Aug 3, 2021 at 5:02 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>  Not all of the multiarch tests are pure POSIX so elide over those
>  tests on a non-Linux system. This allows for at least some of the
>  tests to be nominally usable by *BSD user builds.
>
>  Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>  Cc: Warner Losh <imp@bsdimp.com>
>  ---
>   tests/tcg/multiarch/Makefile.target | 6 +++++-
>   tests/tcg/x86_64/Makefile.target    | 4 ++++
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> Acked-by: Warner Losh <imp@bsdimp.com>
>
> To do this with gcc10, however, I had to add -Wno-error=overflow
> otherwise I got a lot of warnings about constants being truncated to
> 0.
>
> It also fails the sha1 test, but when I run it by hand it works. It turns
> out that I have a sha1 in my path, and at least in the bsd-user edition
> of qemu-i386 tries to run that and fails.
>
> Also, the hello world program needed tweaking
>
> So with this applied and the following patch
>
> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> index 63cf1b2573..39420631a8 100644
> --- a/tests/tcg/Makefile.target
> +++ b/tests/tcg/Makefile.target
> @@ -155,7 +155,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
>
>  ifdef CONFIG_USER_ONLY
>  run-%: %
> -       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) $<, "$< on $(TARGET_NAME)")
> +       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) ./$<, "$< on $(TARGET_NAME)")
>
>  run-plugin-%:
>         $(call run-test, $@, $(QEMU) $(QEMU_OPTS) \
> @@ -168,7 +168,7 @@ run-%: %
>         $(call run-test, $<, \
>           $(QEMU) -monitor none -display none \
>                   -chardev file$(COMMA)path=$<.out$(COMMA)id=output \
> -                 $(QEMU_OPTS) $<, \
> +                 $(QEMU_OPTS) ./$<, \
>           "$< on $(TARGET_NAME)")

That's weird. I'm not super keen to merge this because it's incomplete
(we have a large number of manual run-FOO stanzas). AFAICT neither of
the loaders attempt to enumerate and search path so I wonder if this is
a function of the shell?

>
>  run-plugin-%:
> diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
> index a053ca3f15..ae258c47f0 100644
> --- a/tests/tcg/i386/Makefile.target
> +++ b/tests/tcg/i386/Makefile.target
> @@ -21,6 +21,7 @@ run-plugin-test-i386-pcmpistri-%: QEMU_OPTS += -cpu max
>  run-test-i386-bmi2: QEMU_OPTS += -cpu max
>  run-plugin-test-i386-bmi2-%: QEMU_OPTS += -cpu max
>
> +CFLAGS +=  -Wno-error=overflow

I'd apply this direct to the test in question rather than a global change.

>  #
>  # hello-i386 is a barebones app
>  #
> diff --git a/tests/tcg/i386/hello-i386.c b/tests/tcg/i386/hello-i386.c
> index 59196dd0b7..4a5a25211c 100644
> --- a/tests/tcg/i386/hello-i386.c
> +++ b/tests/tcg/i386/hello-i386.c
> @@ -1,4 +1,10 @@
> +#ifdef __FreeBSD__
> +#include <sys/syscall.h>
> +#define __NR_exit SYS_exit
> +#define __NR_write SYS_write
> +#else
>  #include <asm/unistd.h>
> +#endif
>
>  static inline void exit(int status)
>  {
>
> I get down to a failure i the mmap test.... and that's all I have time to plumb the depths
> of this morning... Investigating the mmap test failure will have to wait for another day.
>
> Warner
>  
>  diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
>  index 85a6fb7a2e..38ee0f1dec 100644
>  --- a/tests/tcg/multiarch/Makefile.target
>  +++ b/tests/tcg/multiarch/Makefile.target
>  @@ -10,7 +10,11 @@ MULTIARCH_SRC=$(SRC_PATH)/tests/tcg/multiarch
>   # Set search path for all sources
>   VPATH          += $(MULTIARCH_SRC)
>   MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
>  -MULTIARCH_TESTS  =$(filter-out float_helpers, $(MULTIARCH_SRCS:.c=))
>  +MULTIARCH_SKIP=float_helpers
>  +ifeq ($(CONFIG_LINUX),)
>  +MULTIARCH_SKIP+=linux-test
>  +endif
>  +MULTIARCH_TESTS  =$(filter-out $(MULTIARCH_SKIP),$(MULTIARCH_SRCS:.c=))
>
>   #
>   # The following are any additional rules needed to build things
>  diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
>  index 2151ea6302..d7a7385583 100644
>  --- a/tests/tcg/x86_64/Makefile.target
>  +++ b/tests/tcg/x86_64/Makefile.target
>  @@ -8,8 +8,12 @@
>
>   include $(SRC_PATH)/tests/tcg/i386/Makefile.target
>
>  +ifneq ($(CONFIG_LINUX),)
>   X86_64_TESTS += vsyscall
>   TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
>  +else
>  +TESTS=$(MULTIARCH_TESTS)
>  +endif
>   QEMU_OPTS += -cpu max
>
>   test-x86_64: LDFLAGS+=-lm -lc
>  -- 
>  2.30.2


-- 
Alex Bennée

Re: [RFC PATCH 3/3] tests/tcg: commit Makefile atrocities in the name of portability

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 5516 bytes --]

On Fri, Sep 17, 2021 at 8:39 AM Alex Bennée <alex.bennee@linaro.org> wrote:

>
> Warner Losh <imp@bsdimp.com> writes:
>
> > On Tue, Aug 3, 2021 at 5:02 AM Alex Bennée <alex.bennee@linaro.org>
> wrote:
> >
> >  Not all of the multiarch tests are pure POSIX so elide over those
> >  tests on a non-Linux system. This allows for at least some of the
> >  tests to be nominally usable by *BSD user builds.
> >
> >  Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >  Cc: Warner Losh <imp@bsdimp.com>
> >  ---
> >   tests/tcg/multiarch/Makefile.target | 6 +++++-
> >   tests/tcg/x86_64/Makefile.target    | 4 ++++
> >   2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > Acked-by: Warner Losh <imp@bsdimp.com>
> >
> > To do this with gcc10, however, I had to add -Wno-error=overflow
> > otherwise I got a lot of warnings about constants being truncated to
> > 0.
> >
> > It also fails the sha1 test, but when I run it by hand it works. It turns
> > out that I have a sha1 in my path, and at least in the bsd-user edition
> > of qemu-i386 tries to run that and fails.
> >
> > Also, the hello world program needed tweaking
> >
> > So with this applied and the following patch
> >
> > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> > index 63cf1b2573..39420631a8 100644
> > --- a/tests/tcg/Makefile.target
> > +++ b/tests/tcg/Makefile.target
> > @@ -155,7 +155,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
> >
> >  ifdef CONFIG_USER_ONLY
> >  run-%: %
> > -       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) $<, "$< on
> $(TARGET_NAME)")
> > +       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) ./$<, "$< on
> $(TARGET_NAME)")
> >
> >  run-plugin-%:
> >         $(call run-test, $@, $(QEMU) $(QEMU_OPTS) \
> > @@ -168,7 +168,7 @@ run-%: %
> >         $(call run-test, $<, \
> >           $(QEMU) -monitor none -display none \
> >                   -chardev file$(COMMA)path=$<.out$(COMMA)id=output \
> > -                 $(QEMU_OPTS) $<, \
> > +                 $(QEMU_OPTS) ./$<, \
> >           "$< on $(TARGET_NAME)")
>
> That's weird. I'm not super keen to merge this because it's incomplete
> (we have a large number of manual run-FOO stanzas). AFAICT neither of
> the loaders attempt to enumerate and search path so I wonder if this is
> a function of the shell?
>

bsd-user does, in fact, search the path. It does so in loader_exec. It does
this,
I believe, to support execing native binaries, but I'll need to check on
that.

It's fine if we don't merge this just yet.

>  run-plugin-%:
> > diff --git a/tests/tcg/i386/Makefile.target
> b/tests/tcg/i386/Makefile.target
> > index a053ca3f15..ae258c47f0 100644
> > --- a/tests/tcg/i386/Makefile.target
> > +++ b/tests/tcg/i386/Makefile.target
> > @@ -21,6 +21,7 @@ run-plugin-test-i386-pcmpistri-%: QEMU_OPTS += -cpu max
> >  run-test-i386-bmi2: QEMU_OPTS += -cpu max
> >  run-plugin-test-i386-bmi2-%: QEMU_OPTS += -cpu max
> >
> > +CFLAGS +=  -Wno-error=overflow
>
> I'd apply this direct to the test in question rather than a global change.
>

I think we should not apply it at all. I was so happy to get things
compiling
that I'd raced ahead of myself: when this is active, the tests fail to run.


> >  #
> >  # hello-i386 is a barebones app
> >  #
> > diff --git a/tests/tcg/i386/hello-i386.c b/tests/tcg/i386/hello-i386.c
> > index 59196dd0b7..4a5a25211c 100644
> > --- a/tests/tcg/i386/hello-i386.c
> > +++ b/tests/tcg/i386/hello-i386.c
> > @@ -1,4 +1,10 @@
> > +#ifdef __FreeBSD__
> > +#include <sys/syscall.h>
> > +#define __NR_exit SYS_exit
> > +#define __NR_write SYS_write
> > +#else
> >  #include <asm/unistd.h>
> > +#endif
> >
> >  static inline void exit(int status)
> >  {
>

I can submit this as a separate thing, but it's the only patch I'm
completely sure of.

Warner


> > I get down to a failure i the mmap test.... and that's all I have time
> to plumb the depths
> > of this morning... Investigating the mmap test failure will have to wait
> for another day.
> >
> > Warner
> >
> >  diff --git a/tests/tcg/multiarch/Makefile.target
> b/tests/tcg/multiarch/Makefile.target
> >  index 85a6fb7a2e..38ee0f1dec 100644
> >  --- a/tests/tcg/multiarch/Makefile.target
> >  +++ b/tests/tcg/multiarch/Makefile.target
> >  @@ -10,7 +10,11 @@ MULTIARCH_SRC=$(SRC_PATH)/tests/tcg/multiarch
> >   # Set search path for all sources
> >   VPATH          += $(MULTIARCH_SRC)
> >   MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
> >  -MULTIARCH_TESTS  =$(filter-out float_helpers, $(MULTIARCH_SRCS:.c=))
> >  +MULTIARCH_SKIP=float_helpers
> >  +ifeq ($(CONFIG_LINUX),)
> >  +MULTIARCH_SKIP+=linux-test
> >  +endif
> >  +MULTIARCH_TESTS  =$(filter-out $(MULTIARCH_SKIP),$(MULTIARCH_SRCS:.c=))
> >
> >   #
> >   # The following are any additional rules needed to build things
> >  diff --git a/tests/tcg/x86_64/Makefile.target
> b/tests/tcg/x86_64/Makefile.target
> >  index 2151ea6302..d7a7385583 100644
> >  --- a/tests/tcg/x86_64/Makefile.target
> >  +++ b/tests/tcg/x86_64/Makefile.target
> >  @@ -8,8 +8,12 @@
> >
> >   include $(SRC_PATH)/tests/tcg/i386/Makefile.target
> >
> >  +ifneq ($(CONFIG_LINUX),)
> >   X86_64_TESTS += vsyscall
> >   TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
> >  +else
> >  +TESTS=$(MULTIARCH_TESTS)
> >  +endif
> >   QEMU_OPTS += -cpu max
> >
> >   test-x86_64: LDFLAGS+=-lm -lc
> >  --
> >  2.30.2
>
>
> --
> Alex Bennée
>

[-- Attachment #2: Type: text/html, Size: 7526 bytes --]

Re: [RFC PATCH 3/3] tests/tcg: commit Makefile atrocities in the name of portability

[Alex Bennée]

Warner Losh <imp@bsdimp.com> writes:

> On Fri, Sep 17, 2021 at 8:39 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>  Warner Losh <imp@bsdimp.com> writes:
>
>  > On Tue, Aug 3, 2021 at 5:02 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>  >
>  >  Not all of the multiarch tests are pure POSIX so elide over those
>  >  tests on a non-Linux system. This allows for at least some of the
>  >  tests to be nominally usable by *BSD user builds.
>  >
>  >  Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>  >  Cc: Warner Losh <imp@bsdimp.com>
>  >  ---
>  >   tests/tcg/multiarch/Makefile.target | 6 +++++-
>  >   tests/tcg/x86_64/Makefile.target    | 4 ++++
>  >   2 files changed, 9 insertions(+), 1 deletion(-)
>  >
>  > Acked-by: Warner Losh <imp@bsdimp.com>
>  >
>  > To do this with gcc10, however, I had to add -Wno-error=overflow
>  > otherwise I got a lot of warnings about constants being truncated to
>  > 0.
>  >
>  > It also fails the sha1 test, but when I run it by hand it works. It turns
>  > out that I have a sha1 in my path, and at least in the bsd-user edition
>  > of qemu-i386 tries to run that and fails.
>  >
>  > Also, the hello world program needed tweaking
>  >
>  > So with this applied and the following patch
>  >
>  > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
>  > index 63cf1b2573..39420631a8 100644
>  > --- a/tests/tcg/Makefile.target
>  > +++ b/tests/tcg/Makefile.target
>  > @@ -155,7 +155,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
>  >
>  >  ifdef CONFIG_USER_ONLY
>  >  run-%: %
>  > -       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) $<, "$< on $(TARGET_NAME)")
>  > +       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) ./$<, "$< on $(TARGET_NAME)")
>  >
>  >  run-plugin-%:
>  >         $(call run-test, $@, $(QEMU) $(QEMU_OPTS) \
>  > @@ -168,7 +168,7 @@ run-%: %
>  >         $(call run-test, $<, \
>  >           $(QEMU) -monitor none -display none \
>  >                   -chardev file$(COMMA)path=$<.out$(COMMA)id=output \
>  > -                 $(QEMU_OPTS) $<, \
>  > +                 $(QEMU_OPTS) ./$<, \
>  >           "$< on $(TARGET_NAME)")
>
>  That's weird. I'm not super keen to merge this because it's incomplete
>  (we have a large number of manual run-FOO stanzas). AFAICT neither of
>  the loaders attempt to enumerate and search path so I wonder if this is
>  a function of the shell?
>
> bsd-user does, in fact, search the path. It does so in loader_exec. It does this,
> I believe, to support execing native binaries, but I'll need to check
> on that.

It's certainly different from what linux-user does. The execing of
native binaries seems a bit niche given you can always pass an explicit
path. Maybe you could tweak loader_exec to check for the local binary
first. It seems to skip straight to searching the path if there are no
/'s in the filename.

This is unrelated to how you handle foreign binaries on the BSDs? Is
there an equivalent to binfmt_misc?

-- 
Alex Bennée

Re: [RFC PATCH 3/3] tests/tcg: commit Makefile atrocities in the name of portability

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 3877 bytes --]

On Fri, Sep 17, 2021 at 10:45 AM Alex Bennée <alex.bennee@linaro.org> wrote:

>
> Warner Losh <imp@bsdimp.com> writes:
>
> > On Fri, Sep 17, 2021 at 8:39 AM Alex Bennée <alex.bennee@linaro.org>
> wrote:
> >
> >  Warner Losh <imp@bsdimp.com> writes:
> >
> >  > On Tue, Aug 3, 2021 at 5:02 AM Alex Bennée <alex.bennee@linaro.org>
> wrote:
> >  >
> >  >  Not all of the multiarch tests are pure POSIX so elide over those
> >  >  tests on a non-Linux system. This allows for at least some of the
> >  >  tests to be nominally usable by *BSD user builds.
> >  >
> >  >  Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >  >  Cc: Warner Losh <imp@bsdimp.com>
> >  >  ---
> >  >   tests/tcg/multiarch/Makefile.target | 6 +++++-
> >  >   tests/tcg/x86_64/Makefile.target    | 4 ++++
> >  >   2 files changed, 9 insertions(+), 1 deletion(-)
> >  >
> >  > Acked-by: Warner Losh <imp@bsdimp.com>
> >  >
> >  > To do this with gcc10, however, I had to add -Wno-error=overflow
> >  > otherwise I got a lot of warnings about constants being truncated to
> >  > 0.
> >  >
> >  > It also fails the sha1 test, but when I run it by hand it works. It
> turns
> >  > out that I have a sha1 in my path, and at least in the bsd-user
> edition
> >  > of qemu-i386 tries to run that and fails.
> >  >
> >  > Also, the hello world program needed tweaking
> >  >
> >  > So with this applied and the following patch
> >  >
> >  > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> >  > index 63cf1b2573..39420631a8 100644
> >  > --- a/tests/tcg/Makefile.target
> >  > +++ b/tests/tcg/Makefile.target
> >  > @@ -155,7 +155,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
> >  >
> >  >  ifdef CONFIG_USER_ONLY
> >  >  run-%: %
> >  > -       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) $<, "$< on
> $(TARGET_NAME)")
> >  > +       $(call run-test, $<, $(QEMU) $(QEMU_OPTS) ./$<, "$< on
> $(TARGET_NAME)")
> >  >
> >  >  run-plugin-%:
> >  >         $(call run-test, $@, $(QEMU) $(QEMU_OPTS) \
> >  > @@ -168,7 +168,7 @@ run-%: %
> >  >         $(call run-test, $<, \
> >  >           $(QEMU) -monitor none -display none \
> >  >                   -chardev file$(COMMA)path=$<.out$(COMMA)id=output \
> >  > -                 $(QEMU_OPTS) $<, \
> >  > +                 $(QEMU_OPTS) ./$<, \
> >  >           "$< on $(TARGET_NAME)")
> >
> >  That's weird. I'm not super keen to merge this because it's incomplete
> >  (we have a large number of manual run-FOO stanzas). AFAICT neither of
> >  the loaders attempt to enumerate and search path so I wonder if this is
> >  a function of the shell?
> >
> > bsd-user does, in fact, search the path. It does so in loader_exec. It
> does this,
> > I believe, to support execing native binaries, but I'll need to check
> > on that.
>
> It's certainly different from what linux-user does. The execing of
> native binaries seems a bit niche given you can always pass an explicit
> path. Maybe you could tweak loader_exec to check for the local binary
> first. It seems to skip straight to searching the path if there are no
> /'s in the filename.
>
> This is unrelated to how you handle foreign binaries on the BSDs? Is
> there an equivalent to binfmt_misc?
>

It's a difference in the presentation of args in BSD. When reviewing the
code,
Kyle Evans said:

>> IIRC imgact_binmisc will have the resolved path but preserve argv as
>> it should have been were it not emulated, so we have to re-evaluate
>> the PATH search here because we try to be faithful to the context.

At the time, I confirmed that behavior. Thinking about it, if '.' isn't in
the
path, and we can't find it, then we're not in this condition. So that means
looking for it in '.' also won't break anything and will fix this issue.
I'll
rework bsd-user's bsdload.c to improve the logic.

Warner

[-- Attachment #2: Type: text/html, Size: 5458 bytes --]