diff mbox series

Fix WAIT_FOR_DEBUGGER for container tests.

Message ID 20230928105040.2266853-1-stli@linux.ibm.com
State New
Headers show
Series Fix WAIT_FOR_DEBUGGER for container tests. | expand

Commit Message

Stefan Liebler Sept. 28, 2023, 10:50 a.m. UTC 
For container tests, gdb needs to set the sysroot to the corresponding
testroot.root directory.  The assumption was that PIDs < 3 means that
we are running within a container.

Starting with commit 2fe64148a81f0d78050c302f34a6853d21f7cae4
"Allow for unpriviledged nested containers", the default is to use
the PID namespace of the parent.  Thus support_test_main.c does not
recognize our container anymore.

This patch now assumes that we are running inside a container if
test-container.c has set PID_OUTSIDE_CONTAINER and always uses this
PID independent of having a new PID namespace or not.
---
 support/support_test_main.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Adhemerval Zanella Netto Oct. 3, 2023, 6:12 p.m. UTC | #1 
On 28/09/23 07:50, Stefan Liebler wrote:
> For container tests, gdb needs to set the sysroot to the corresponding
> testroot.root directory.  The assumption was that PIDs < 3 means that
> we are running within a container.
> 
> Starting with commit 2fe64148a81f0d78050c302f34a6853d21f7cae4
> "Allow for unpriviledged nested containers", the default is to use
> the PID namespace of the parent.  Thus support_test_main.c does not
> recognize our container anymore.
> 
> This patch now assumes that we are running inside a container if
> test-container.c has set PID_OUTSIDE_CONTAINER and always uses this
> PID independent of having a new PID namespace or not.

Do you have a scenario where debugglibc is failing to attach to a container
test? I am trying to see if something it broken, but it does seems to be
working.

> ---
>  support/support_test_main.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/support/support_test_main.c b/support/support_test_main.c
> index c20c19e774..f19fce5644 100644
> --- a/support/support_test_main.c
> +++ b/support/support_test_main.c
> @@ -195,16 +195,14 @@ run_test_function (int argc, char **argv, const struct test_config *config)
>        char *gdb_script_name;
>        int inside_container = 0;
>  
> -      mypid = getpid();
> -      if (mypid < 3)
> +      const char *outside_pid = getenv("PID_OUTSIDE_CONTAINER");
> +      if (outside_pid)
>  	{
> -	  const char *outside_pid = getenv("PID_OUTSIDE_CONTAINER");
> -	  if (outside_pid)
> -	    {
> -	      mypid = atoi (outside_pid);
> -	      inside_container = 1;
> -	    }
> +	  mypid = atoi (outside_pid);
> +	  inside_container = 1;
>  	}
> +      else
> +	mypid = getpid();
>  
>        gdb_script_name = (char *) xmalloc (strlen (argv[0]) + strlen (".gdb") + 1);
>        sprintf (gdb_script_name, "%s.gdb", argv[0]);
Stefan Liebler Oct. 4, 2023, 8:03 a.m. UTC | #2 
On 03.10.23 20:12, Adhemerval Zanella Netto wrote:
> 
> 
> On 28/09/23 07:50, Stefan Liebler wrote:
>> For container tests, gdb needs to set the sysroot to the corresponding
>> testroot.root directory.  The assumption was that PIDs < 3 means that
>> we are running within a container.
>>
>> Starting with commit 2fe64148a81f0d78050c302f34a6853d21f7cae4
>> "Allow for unpriviledged nested containers", the default is to use
>> the PID namespace of the parent.  Thus support_test_main.c does not
>> recognize our container anymore.
>>
>> This patch now assumes that we are running inside a container if
>> test-container.c has set PID_OUTSIDE_CONTAINER and always uses this
>> PID independent of having a new PID namespace or not.
> 
> Do you have a scenario where debugglibc is failing to attach to a container
> test? I am trying to see if something it broken, but it does seems to be
> working.
> 
For me, it failed with "WAIT_FOR_DEBUGGER=1 make test
t=nss/tst-nss-gai-hv2-canonname". After gdb attached to the process, gdb
has not found a suitable binary and thus can't set wait_for_debugger.

The same seems also to happen for "./debugglibc.sh -c --
./nss/tst-nss-gai-hv2-canonname".

Can you please check if your ./nss/tst-nss-gai-hv2-canonname.gdb
contains "set sysroot"?
For me it does not work on s390x / x86_64. In both cases, I'm using
Fedora 38.
Stefan Liebler Oct. 12, 2023, 8:04 a.m. UTC | #3 
On 28.09.23 12:50, Stefan Liebler wrote:
> For container tests, gdb needs to set the sysroot to the corresponding
> testroot.root directory.  The assumption was that PIDs < 3 means that
> we are running within a container.
> 
> Starting with commit 2fe64148a81f0d78050c302f34a6853d21f7cae4
> "Allow for unpriviledged nested containers", the default is to use
> the PID namespace of the parent.  Thus support_test_main.c does not
> recognize our container anymore.
> 
> This patch now assumes that we are running inside a container if
> test-container.c has set PID_OUTSIDE_CONTAINER and always uses this
> PID independent of having a new PID namespace or not.
> ---
>  support/support_test_main.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/support/support_test_main.c b/support/support_test_main.c
> index c20c19e774..f19fce5644 100644
> --- a/support/support_test_main.c
> +++ b/support/support_test_main.c
> @@ -195,16 +195,14 @@ run_test_function (int argc, char **argv, const struct test_config *config)
>        char *gdb_script_name;
>        int inside_container = 0;
>  
> -      mypid = getpid();
> -      if (mypid < 3)
> +      const char *outside_pid = getenv("PID_OUTSIDE_CONTAINER");
> +      if (outside_pid)
>  	{
> -	  const char *outside_pid = getenv("PID_OUTSIDE_CONTAINER");
> -	  if (outside_pid)
> -	    {
> -	      mypid = atoi (outside_pid);
> -	      inside_container = 1;
> -	    }
> +	  mypid = atoi (outside_pid);
> +	  inside_container = 1;
>  	}
> +      else
> +	mypid = getpid();
>  
>        gdb_script_name = (char *) xmalloc (strlen (argv[0]) + strlen (".gdb") + 1);
>        sprintf (gdb_script_name, "%s.gdb", argv[0]);

ping
Adhemerval Zanella Netto Oct. 13, 2023, 1:20 p.m. UTC | #4 
On 04/10/23 05:03, Stefan Liebler wrote:
> On 03.10.23 20:12, Adhemerval Zanella Netto wrote:
>>
>>
>> On 28/09/23 07:50, Stefan Liebler wrote:
>>> For container tests, gdb needs to set the sysroot to the corresponding
>>> testroot.root directory.  The assumption was that PIDs < 3 means that
>>> we are running within a container.
>>>
>>> Starting with commit 2fe64148a81f0d78050c302f34a6853d21f7cae4
>>> "Allow for unpriviledged nested containers", the default is to use
>>> the PID namespace of the parent.  Thus support_test_main.c does not
>>> recognize our container anymore.
>>>
>>> This patch now assumes that we are running inside a container if
>>> test-container.c has set PID_OUTSIDE_CONTAINER and always uses this
>>> PID independent of having a new PID namespace or not.
>>
>> Do you have a scenario where debugglibc is failing to attach to a container
>> test? I am trying to see if something it broken, but it does seems to be
>> working.
>>
> For me, it failed with "WAIT_FOR_DEBUGGER=1 make test
> t=nss/tst-nss-gai-hv2-canonname". After gdb attached to the process, gdb
> has not found a suitable binary and thus can't set wait_for_debugger.

Ok, I see the failure now.

> 
> The same seems also to happen for "./debugglibc.sh -c --
> ./nss/tst-nss-gai-hv2-canonname".
> 
> Can you please check if your ./nss/tst-nss-gai-hv2-canonname.gdb
> contains "set sysroot"?
> For me it does not work on s390x / x86_64. In both cases, I'm using
> Fedora 38.
Yes, with the patch the sysroot is now set correctly.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
Stefan Liebler Oct. 16, 2023, 7:53 a.m. UTC | #5 
On 13.10.23 15:20, Adhemerval Zanella Netto wrote:
> 
> 
> On 04/10/23 05:03, Stefan Liebler wrote:
>> On 03.10.23 20:12, Adhemerval Zanella Netto wrote:
>>>
>>>
>>> On 28/09/23 07:50, Stefan Liebler wrote:
>>>> For container tests, gdb needs to set the sysroot to the corresponding
>>>> testroot.root directory.  The assumption was that PIDs < 3 means that
>>>> we are running within a container.
>>>>
>>>> Starting with commit 2fe64148a81f0d78050c302f34a6853d21f7cae4
>>>> "Allow for unpriviledged nested containers", the default is to use
>>>> the PID namespace of the parent.  Thus support_test_main.c does not
>>>> recognize our container anymore.
>>>>
>>>> This patch now assumes that we are running inside a container if
>>>> test-container.c has set PID_OUTSIDE_CONTAINER and always uses this
>>>> PID independent of having a new PID namespace or not.
>>>
>>> Do you have a scenario where debugglibc is failing to attach to a container
>>> test? I am trying to see if something it broken, but it does seems to be
>>> working.
>>>
>> For me, it failed with "WAIT_FOR_DEBUGGER=1 make test
>> t=nss/tst-nss-gai-hv2-canonname". After gdb attached to the process, gdb
>> has not found a suitable binary and thus can't set wait_for_debugger.
> 
> Ok, I see the failure now.
> 
>>
>> The same seems also to happen for "./debugglibc.sh -c --
>> ./nss/tst-nss-gai-hv2-canonname".
>>
>> Can you please check if your ./nss/tst-nss-gai-hv2-canonname.gdb
>> contains "set sysroot"?
>> For me it does not work on s390x / x86_64. In both cases, I'm using
>> Fedora 38.
> Yes, with the patch the sysroot is now set correctly.
> 
> LGTM, thanks.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

Thanks for the review. I've just committed the patch.
diff mbox series

Patch

diff --git a/support/support_test_main.c b/support/support_test_main.c
index c20c19e774..f19fce5644 100644
--- a/support/support_test_main.c
+++ b/support/support_test_main.c
@@ -195,16 +195,14 @@  run_test_function (int argc, char **argv, const struct test_config *config)
       char *gdb_script_name;
       int inside_container = 0;
 
-      mypid = getpid();
-      if (mypid < 3)
+      const char *outside_pid = getenv("PID_OUTSIDE_CONTAINER");
+      if (outside_pid)
 	{
-	  const char *outside_pid = getenv("PID_OUTSIDE_CONTAINER");
-	  if (outside_pid)
-	    {
-	      mypid = atoi (outside_pid);
-	      inside_container = 1;
-	    }
+	  mypid = atoi (outside_pid);
+	  inside_container = 1;
 	}
+      else
+	mypid = getpid();
 
       gdb_script_name = (char *) xmalloc (strlen (argv[0]) + strlen (".gdb") + 1);
       sprintf (gdb_script_name, "%s.gdb", argv[0]);