RFC: test-in-container vs ld.so

RFC: test-in-container vs ld.so

[DJ Delorie]

The glibc test infrastructure goes to great lengths to run test
programs in the just-built environment, running env and ld.so with
lots of arguments to get things "just right".

The test-in-container infrastructure goes to... well, less great
lengths to provide a "correct native" environment for the test
programs, yet it still runs env and ld.so within the container.

There are a few of our tests that sometimes fail due to kernel address
space randomization when run under ld.so yet work correctly all the
time when run directly.

So... should we run containerized tests without ld.so?

I see two discussion points...

1. Are there tests that *need* the env/ld.so handling within the
container?  (not counting test-specific environment variables, I mean
the environment variables we pass to *every* test)

2. how complicated would it be to remove those from the makefile
rules?  Alternatively, I suppose test-container itself could remove
them, but better not to hide that logic.

Re: RFC: test-in-container vs ld.so

[Florian Weimer]

* DJ Delorie:

> So... should we run containerized tests without ld.so?

Yes, absolutely. It's one of the benefits of running things in a
container.

But I think we are already doing that?  test-container is run via
ld.so, but the actual test is not?

> 1. Are there tests that *need* the env/ld.so handling within the
> container?  (not counting test-specific environment variables, I mean
> the environment variables we pass to *every* test)

We need to set up a predictable locale configuration.  But GCONV_PATH
and LOCPATH should *not* be set.

> 2. how complicated would it be to remove those from the makefile
> rules?  Alternatively, I suppose test-container itself could remove
> them, but better not to hide that logic.

I suspect the removalof LC_*, LANG etc. variables should be done in
test-container, just for convenience.

Then we can just drop $(run-program-env) because it is defined as:

# $(run-program-env) is the default environment variable settings to
# use when running a program built with the newly built library.
run-program-env = GCONV_PATH=$(common-objpfx)iconvdata \
		  LOCPATH=$(common-objpfx)localedata LC_ALL=C

Re: RFC: test-in-container vs ld.so

[DJ Delorie]

Florian Weimer <fw@deneb.enyo.de> writes:
> But I think we are already doing that?  test-container is run via
> ld.so, but the actual test is not?
>
Here's nss/tst-nss-test3, with some newlines and tabs added for readability:

env GCONV_PATH=/envy/dj/tools/upstream/glibc.pristine.build/iconvdata
	LOCPATH=/envy/dj/tools/upstream/glibc.pristine.build/localedata LC_ALL=C
/envy/dj/tools/upstream/glibc.pristine.build/elf/ld-linux-x86-64.so.2
	--library-path
/envy/dj/tools/upstream/glibc.pristine.build:/envy/dj/tools/upstream/glibc.pristine.build/math:/envy/dj/tools/upstream/glibc.pristine.build/elf:/envy/dj/tools/upstream/glibc.pristine.build/dlfcn:/envy/dj/tools/upstream/glibc.pristine.build/nss:/envy/dj/tools/upstream/glibc.pristine.build/nis:/envy/dj/tools/upstream/glibc.pristine.build/rt:/envy/dj/tools/upstream/glibc.pristine.build/resolv:/envy/dj/tools/upstream/glibc.pristine.build/mathvec:/envy/dj/tools/upstream/glibc.pristine.build/support:/envy/dj/tools/upstream/glibc.pristine.build/crypt:/envy/dj/tools/upstream/glibc.pristine.build/nptl
\

/envy/dj/tools/upstream/glibc.pristine.build/support/test-container
env GCONV_PATH=/envy/dj/tools/upstream/glibc.pristine.build/iconvdata
	LOCPATH=/envy/dj/tools/upstream/glibc.pristine.build/localedata
	LC_ALL=C \

/envy/dj/tools/upstream/glibc.pristine.build/elf/ld-linux-x86-64.so.2
	--library-path
	/envy/dj/tools/upstream/glibc.pristine.build:/envy/dj/tools/upstream/glibc.pristine.build/math:/envy/dj/tools/upstream/glibc.pristine.build/elf:/envy/dj/tools/upstream/glibc.pristine.build/dlfcn:/envy/dj/tools/upstream/glibc.pristine.build/nss:/envy/dj/tools/upstream/glibc.pristine.build/nis:/envy/dj/tools/upstream/glibc.pristine.build/rt:/envy/dj/tools/upstream/glibc.pristine.build/resolv:/envy/dj/tools/upstream/glibc.pristine.build/mathvec:/envy/dj/tools/upstream/glibc.pristine.build/support:/envy/dj/tools/upstream/glibc.pristine.build/crypt:/envy/dj/tools/upstream/glibc.pristine.build/nptl \

   /envy/dj/tools/upstream/glibc.pristine.build/nss/tst-nss-test3 >
   /envy/dj/tools/upstream/glibc.pristine.build/nss/tst-nss-test3.out

Re: RFC: test-in-container vs ld.so

[Florian Weimer]

* DJ Delorie:

> Florian Weimer <fw@deneb.enyo.de> writes:
>> But I think we are already doing that?  test-container is run via
>> ld.so, but the actual test is not?
>>
> Here's nss/tst-nss-test3, with some newlines and tabs added for readability:

Ah, then it's just a matter of replacing $(host-test-program-cmd) with
$(built-program-file), I think.

Re: RFC: test-in-container vs ld.so

[Carlos O'Donell]

On 11/7/19 5:31 PM, DJ Delorie wrote:
> 
> The glibc test infrastructure goes to great lengths to run test
> programs in the just-built environment, running env and ld.so with
> lots of arguments to get things "just right".
> 
> The test-in-container infrastructure goes to... well, less great
> lengths to provide a "correct native" environment for the test
> programs, yet it still runs env and ld.so within the container.
> 
> There are a few of our tests that sometimes fail due to kernel address
> space randomization when run under ld.so yet work correctly all the
> time when run directly.
> 
> So... should we run containerized tests without ld.so?
> 
> I see two discussion points...
> 
> 1. Are there tests that *need* the env/ld.so handling within the
> container?  (not counting test-specific environment variables, I mean
> the environment variables we pass to *every* test)

I don't think so.

I think everything that test-in-container does should emulate an
installed system.

If we have a *need* to run an ld.so test, that verifies running ld.so
directly works, we can do it outside of the test-in-container
infrastructure.

> 2. how complicated would it be to remove those from the makefile
> rules?  Alternatively, I suppose test-container itself could remove
> them, but better not to hide that logic.
 
test-in-container is already a distinct target. You would have to alter
the dependencies, and likely create a cloned rule that doesn't run ld.so
directly. I don't know how difficult this would be.

I think this is the right directly though. Test in container should not
run ld.so, it should run the binary directly.

You and I discussed this on the list while we were reviewing test-in-container,
but at the time the goal was to get the framework in place and emulating
exactly what the previous framework did except in a container.

The next logical step is to make test-in-container *more* like the installed
tree testing we want it to be.

-- 
Cheers,
Carlos.

Re: RFC: test-in-container vs ld.so

[DJ Delorie]

This patch seems to work, but we don't have a lot of containerized
tests, and my setup is "normal" (esp compared to Joseph's ;)

diff --git a/Rules b/Rules
index 8dbac56ce6..9ca5e14092 100644
--- a/Rules
+++ b/Rules
@@ -275,8 +275,8 @@ $(objpfx)%.out: /dev/null $(objpfx)%	# Make it 2nd arg for canned sequence.
 # tests-container.
 $(tests-container:%=$(objpfx)%.out): $(objpfx)%.out : $(if $(wildcard $(objpfx)%.files),$(objpfx)%.files,/dev/null) $(objpfx)%
 	$(test-wrapper-env) $(run-program-env) $(run-via-rtld-prefix) \
-	  $(common-objpfx)support/test-container env $(run-program-env) $($*-ENV) \
-	  $(host-test-program-cmd) $($*-ARGS) > $@; \
+	  $(common-objpfx)support/test-container env $($*-ENV) \
+	  $(built-program-file) $($*-ARGS) > $@; \
 	$(evaluate-test)
 
 

Re: RFC: test-in-container vs ld.so

[Carlos O'Donell]

On 11/8/19 6:54 PM, DJ Delorie wrote:
> 
> This patch seems to work, but we don't have a lot of containerized
> tests, and my setup is "normal" (esp compared to Joseph's ;)
> 
> diff --git a/Rules b/Rules
> index 8dbac56ce6..9ca5e14092 100644
> --- a/Rules
> +++ b/Rules
> @@ -275,8 +275,8 @@ $(objpfx)%.out: /dev/null $(objpfx)%	# Make it 2nd arg for canned sequence.
>  # tests-container.
>  $(tests-container:%=$(objpfx)%.out): $(objpfx)%.out : $(if $(wildcard $(objpfx)%.files),$(objpfx)%.files,/dev/null) $(objpfx)%
>  	$(test-wrapper-env) $(run-program-env) $(run-via-rtld-prefix) \
> -	  $(common-objpfx)support/test-container env $(run-program-env) $($*-ENV) \
> -	  $(host-test-program-cmd) $($*-ARGS) > $@; \
> +	  $(common-objpfx)support/test-container env $($*-ENV) \
> +	  $(built-program-file) $($*-ARGS) > $@; \
>  	$(evaluate-test)
>  
>  
> 

FWIW, that looks good to me.

You could just propose this as a patch and we could try to run with it
and see what happens.

We don't have a multi-target tester, except for the build bots but they
are not well maintained at this point (we'd like to fix this, but that
will take time).

-- 
Cheers,
Carlos.

Re: RFC: test-in-container vs ld.so

[Florian Weimer]

* DJ Delorie:

> This patch seems to work, but we don't have a lot of containerized
> tests, and my setup is "normal" (esp compared to Joseph's ;)
>
> diff --git a/Rules b/Rules
> index 8dbac56ce6..9ca5e14092 100644
> --- a/Rules
> +++ b/Rules
> @@ -275,8 +275,8 @@ $(objpfx)%.out: /dev/null $(objpfx)%	# Make it 2nd arg for canned sequence.
>  # tests-container.
>  $(tests-container:%=$(objpfx)%.out): $(objpfx)%.out : $(if $(wildcard $(objpfx)%.files),$(objpfx)%.files,/dev/null) $(objpfx)%
>  	$(test-wrapper-env) $(run-program-env) $(run-via-rtld-prefix) \
> -	  $(common-objpfx)support/test-container env $(run-program-env) $($*-ENV) \
> -	  $(host-test-program-cmd) $($*-ARGS) > $@; \
> +	  $(common-objpfx)support/test-container env $($*-ENV) \
> +	  $(built-program-file) $($*-ARGS) > $@; \
>  	$(evaluate-test)

I think it's a step in the right direction.  The outer
$(run-program-env) is still inherited by the test, and that will need
fixing, too.

Re: RFC: test-in-container vs ld.so

[DJ Delorie]

Florian Weimer <fw@deneb.enyo.de> writes:
> I suspect the removalof LC_*, LANG etc. variables should be done in
> test-container, just for convenience.

This is what I ended up with.  All tests pass, but as I mentioned in a
previous email, very few tests run in the container.  This blocks the
outer variables from coming in, while letting the test itself set them
again if needed.

I'm not sure what kind of test we could run in the container to make
sure we're *not* accidentally using the build-tree file, though.

diff --git a/Rules b/Rules
index 8dbac56ce6..9ca5e14092 100644
--- a/Rules
+++ b/Rules
@@ -275,8 +275,8 @@ $(objpfx)%.out: /dev/null $(objpfx)%	# Make it 2nd arg for canned sequence.
 # tests-container.
 $(tests-container:%=$(objpfx)%.out): $(objpfx)%.out : $(if $(wildcard $(objpfx)%.files),$(objpfx)%.files,/dev/null) $(objpfx)%
 	$(test-wrapper-env) $(run-program-env) $(run-via-rtld-prefix) \
-	  $(common-objpfx)support/test-container env $(run-program-env) $($*-ENV) \
-	  $(host-test-program-cmd) $($*-ARGS) > $@; \
+	  $(common-objpfx)support/test-container env $($*-ENV) \
+	  $(built-program-file) $($*-ARGS) > $@; \
 	$(evaluate-test)
 
 
diff --git a/support/test-container.c b/support/test-container.c
index 5d08979df3..9b0b1c7631 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -712,6 +712,12 @@ main (int argc, char **argv)
       --argc;
     }
 
+  /* We don't want to inherit these from the ld.so wrapper we were
+     invoked with, but the test can still provide a test-specific
+     value via the "env" we pretend to run.  */
+  unsetenv ("GCONV_PATH");
+  unsetenv ("LOCPATH");
+
   if (strcmp (argv[1], "env") == 0)
     {
       ++argv;

Re: RFC: test-in-container vs ld.so

[DJ Delorie]

Amusingly, test-container.c has for a long time already had this code:


  if (strcmp (argv[1], support_objdir_elf_ldso) == 0)
    {
      ++argv;
      --argc;
      while (argv[1][0] == '-')
	{
	  if (strcmp (argv[1], "--library-path") == 0)
	    {
	      ++argv;
	      --argc;
	    }
	  ++argv;
	  --argc;
	}
    }


So, we're already not running ld.so in the container anyway :-P

IIRC we made a point of running the test program as PID 1 in the
container, which precludes running under other programs.

Re: RFC: test-in-container vs ld.so

[Carlos O'Donell]

On 12/6/19 3:09 PM, DJ Delorie wrote:
> 
> Amusingly, test-container.c has for a long time already had this code:
> 
> 
>   if (strcmp (argv[1], support_objdir_elf_ldso) == 0)
>     {
>       ++argv;
>       --argc;
>       while (argv[1][0] == '-')
> 	{
> 	  if (strcmp (argv[1], "--library-path") == 0)
> 	    {
> 	      ++argv;
> 	      --argc;
> 	    }
> 	  ++argv;
> 	  --argc;
> 	}
>     }
> 
> 
> So, we're already not running ld.so in the container anyway :-P
> 
> IIRC we made a point of running the test program as PID 1 in the
> container, which precludes running under other programs.
 
Yay! Does that mean we don't have a bug?

-- 
Cheers,
Carlos.

Re: RFC: test-in-container vs ld.so

[DJ Delorie]

"Carlos O'Donell" <codonell@redhat.com> writes:
>> So, we're already not running ld.so in the container anyway :-P
>  
> Yay! Does that mean we don't have a bug?

I think we should still adjust the Makefiles so that what the user sees
in the logs more accurately reflects reality.