[PATCH 1/2] Attach to test in container from debugglibc.sh

[PATCH 1/2] Attach to test in container from debugglibc.sh

[Gabriel F. T. Gomes]

From: "Gabriel F. T. Gomes" <gabrielftg@linux.ibm.com>

Some test cases are meant to be ran inside the container infrastructure
and make check automatically runs them as such.  However, running a
single test case in a container without make check is useful.

This patch adds a new --tool option to testrun.sh that makes this easy,
as well as it adds a new option (-c or --in-container) to debugglibc.sh,
which causes the program under test to be ran in a container (with
WAIT_FOR_DEBUGGER=1), then automatically attaches GDB to it.

Automatically detecting if a test case is supposed to be ran inside a
container is harder (if not impossible), as Carlos pointed out [1],
however, this patch makes it easier to do it manually:

  Using testrun.sh with containerized test:

    $ ./testrun.sh --tool=container /absolute/path/to/program

  Using debugglibc.sh with containerized test:

    $ ./debugglibc.sh -c /absolute/path/to/program

Note: running these commands with relative paths causes error and
warning messages to be displayed, although the test case might succeed.

For example, with relative path:

  $ ./testrun.sh --tool=container elf/tst-ldconfig-bad-aux-cache
  error: subprocess failed: execv
  error:   unexpected error output from subprocess
  /sbin/ldconfig: Warning: ignoring configuration file that cannot be opened: /etc/ld.so.conf: No such file or directory
  info: f    0    1064   /var/cache/ldconfig/aux-cache            20 aux-cache
  [...]

Whereas with absolute paths, the errors and warnings are gone:

  $ ./testrun.sh --tool=container $PWD/elf/tst-ldconfig-bad-aux-cache
  info: f    0    1064   /var/cache/ldconfig/aux-cache            20 aux-cache
  [...]

[1] https://sourceware.org/ml/libc-alpha/2019-11/msg00873.html
---
 Makefile | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Makefile b/Makefile
index fae71aa287..924fdb6c0f 100644
--- a/Makefile
+++ b/Makefile
@@ -181,6 +181,11 @@ case "$$toolname" in
   valgrind)
     exec env $(run-program-env) valgrind $(test-via-rtld-prefix) $${1+"$$@"}
     ;;
+  container)
+    exec env $(run-program-env) $(test-via-rtld-prefix) \
+      $(common-objdir)/support/test-container \
+      env $(run-program-env) $(test-via-rtld-prefix) $${1+"$$@"}
+    ;;
   *)
     usage
     ;;
@@ -202,6 +207,7 @@ define debugglibc
 SOURCE_DIR="$(CURDIR)"
 BUILD_DIR="$(common-objpfx)"
 CMD_FILE="$(common-objpfx)debugglibc.gdb"
+CONTAINER=false
 DIRECT=true
 SYMBOLSFILE=true
 unset TESTCASE
@@ -235,6 +241,9 @@ Options:
 
   The following options do not take arguments:
 
+  -c, --in-container
+	Run the test case inside a container and automatically attach
+	GDB to it.
   -i, --no-direct
 	Selects whether to pass the --direct flag to the program.
 	--direct is useful when debugging glibc test cases. It inhibits the
@@ -263,6 +272,9 @@ do
       ENVVARS="$$2 $$ENVVARS"
       shift
       ;;
+    -c|--in-container)
+      CONTAINER=true
+      ;;
     -i|--no-direct)
       DIRECT=false
       ;;
@@ -348,6 +360,13 @@ echo "GDB Commands     : $$CMD_FILE"
 echo "Env vars         : $$ENVVARS"
 echo
 
+if [ "$$CONTAINER" == true ]
+then
+# Use testrun.sh to start the test case with WAIT_FOR_DEBUGGER=1, then
+# automatically attach GDB to it.
+WAIT_FOR_DEBUGGER=1 $(common-objpfx)testrun.sh --tool=container $${TESTCASE} &
+gdb -x $${TESTCASE}.gdb
+else
 # Start the test case debugging in two steps:
 #   1. the following command invokes gdb to run the loader;
 #   2. the commands file tells the loader to run the test case.
@@ -355,6 +374,7 @@ gdb -q \
   -x $${CMD_FILE} \
   -d $${SOURCE_DIR} \
   $${BUILD_DIR}/elf/ld.so
+fi
 endef
 
 # This is another handy script for debugging dynamically linked program
-- 
2.21.0

[PATCH 2/2] Do not use ld.so to open statically linked programs in debugglibc.sh

[Gabriel F. T. Gomes]

From: "Gabriel F. T. Gomes" <gabrielftg@linux.ibm.com>

Debugging programs that have been dynamically linked against an
uninstalled glibc requires unusual steps, such as letting gdb know where
the thread db library is located and explicitly calling the loader.
However, when the program under test is statically linked, these steps
are not required (as a matter of fact, using the dynamic loader to run a
statically linked program is wrong and will fail), and gdb should be
called the usual way.

This patch modifies debugglibc.sh so that it checks if the program under
test is statically linked, then runs the debugger appropriately.
---
 Makefile | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 924fdb6c0f..b43226c35a 100644
--- a/Makefile
+++ b/Makefile
@@ -209,6 +209,7 @@ BUILD_DIR="$(common-objpfx)"
 CMD_FILE="$(common-objpfx)debugglibc.gdb"
 CONTAINER=false
 DIRECT=true
+STATIC=false
 SYMBOLSFILE=true
 unset TESTCASE
 unset BREAKPOINTS
@@ -297,8 +298,8 @@ do
   shift
 done
 
-# Check for required argument
-if [ ! -v TESTCASE ]
+# Check for required argument and if the testcase exists
+if [ ! -v TESTCASE ] || [ ! -f $${TESTCASE} ]
 then
   usage
   exit 1
@@ -318,6 +319,14 @@ else
   DIRECT=""
 fi
 
+# Check if the test case is static
+if file $${TESTCASE} | grep "statically linked" >/dev/null
+then
+  STATIC=true
+else
+  STATIC=false
+fi
+
 # Expand symbols loading command
 if [ "$$SYMBOLSFILE" == true ]
 then
@@ -366,6 +375,9 @@ then
 # automatically attach GDB to it.
 WAIT_FOR_DEBUGGER=1 $(common-objpfx)testrun.sh --tool=container $${TESTCASE} &
 gdb -x $${TESTCASE}.gdb
+elif [ "$$STATIC" == true ]
+then
+gdb $${TESTCASE}
 else
 # Start the test case debugging in two steps:
 #   1. the following command invokes gdb to run the loader;
-- 
2.21.0

[PATCH 0/2] New options for debugglibc.sh and testrun.sh

[Gabriel F. T. Gomes]

From: "Gabriel F. T. Gomes" <gabrielftg@linux.ibm.com>

Carlos noticed [1] that the support for tests inside containers and for
statically linked tests is not complete in debugglibc.sh and testrun.sh.
This patch set tries to address some of the issues he raised.

Note: there might be some design problems with the implementation,
anyhow I think the patches will be useful to advance the discussion.

[1] https://sourceware.org/ml/libc-alpha/2019-11/msg00873.html

Gabriel F. T. Gomes (2):
  Attach to test in container from debugglibc.sh
  Do not use ld.so to open statically linked programs in debugglibc.sh

 Makefile | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

-- 
2.21.0

Re: [PATCH 1/2] Attach to test in container from debugglibc.sh

[Carlos O'Donell]

On 12/3/19 9:36 PM, Gabriel F. T. Gomes wrote:
> From: "Gabriel F. T. Gomes" <gabrielftg@linux.ibm.com>

This is much better than what we have, thank you for implementing this!

We can always improve this as time goes on.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> Some test cases are meant to be ran inside the container infrastructure
> and make check automatically runs them as such.  However, running a
> single test case in a container without make check is useful.
> 
> This patch adds a new --tool option to testrun.sh that makes this easy,
> as well as it adds a new option (-c or --in-container) to debugglibc.sh,
> which causes the program under test to be ran in a container (with
> WAIT_FOR_DEBUGGER=1), then automatically attaches GDB to it.
> 
> Automatically detecting if a test case is supposed to be ran inside a
> container is harder (if not impossible), as Carlos pointed out [1],
> however, this patch makes it easier to do it manually:
> 
>   Using testrun.sh with containerized test:
> 
>     $ ./testrun.sh --tool=container /absolute/path/to/program
> 
>   Using debugglibc.sh with containerized test:
> 
>     $ ./debugglibc.sh -c /absolute/path/to/program
> 
> Note: running these commands with relative paths causes error and
> warning messages to be displayed, although the test case might succeed.
> 
> For example, with relative path:
> 
>   $ ./testrun.sh --tool=container elf/tst-ldconfig-bad-aux-cache
>   error: subprocess failed: execv
>   error:   unexpected error output from subprocess
>   /sbin/ldconfig: Warning: ignoring configuration file that cannot be opened: /etc/ld.so.conf: No such file or directory
>   info: f    0    1064   /var/cache/ldconfig/aux-cache            20 aux-cache
>   [...]
> 
> Whereas with absolute paths, the errors and warnings are gone:
> 
>   $ ./testrun.sh --tool=container $PWD/elf/tst-ldconfig-bad-aux-cache
>   info: f    0    1064   /var/cache/ldconfig/aux-cache            20 aux-cache
>   [...]
> 
> [1] https://sourceware.org/ml/libc-alpha/2019-11/msg00873.html
> ---
>  Makefile | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index fae71aa287..924fdb6c0f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -181,6 +181,11 @@ case "$$toolname" in
>    valgrind)
>      exec env $(run-program-env) valgrind $(test-via-rtld-prefix) $${1+"$$@"}
>      ;;
> +  container)
> +    exec env $(run-program-env) $(test-via-rtld-prefix) \
> +      $(common-objdir)/support/test-container \
> +      env $(run-program-env) $(test-via-rtld-prefix) $${1+"$$@"}

OK.

> +    ;;
>    *)
>      usage
>      ;;
> @@ -202,6 +207,7 @@ define debugglibc
>  SOURCE_DIR="$(CURDIR)"
>  BUILD_DIR="$(common-objpfx)"
>  CMD_FILE="$(common-objpfx)debugglibc.gdb"
> +CONTAINER=false
>  DIRECT=true
>  SYMBOLSFILE=true
>  unset TESTCASE
> @@ -235,6 +241,9 @@ Options:
>  
>    The following options do not take arguments:
>  
> +  -c, --in-container
> +	Run the test case inside a container and automatically attach
> +	GDB to it.

OK.

>    -i, --no-direct
>  	Selects whether to pass the --direct flag to the program.
>  	--direct is useful when debugging glibc test cases. It inhibits the
> @@ -263,6 +272,9 @@ do
>        ENVVARS="$$2 $$ENVVARS"
>        shift
>        ;;
> +    -c|--in-container)
> +      CONTAINER=true
> +      ;;

OK.

>      -i|--no-direct)
>        DIRECT=false
>        ;;
> @@ -348,6 +360,13 @@ echo "GDB Commands     : $$CMD_FILE"
>  echo "Env vars         : $$ENVVARS"
>  echo
>  
> +if [ "$$CONTAINER" == true ]
> +then
> +# Use testrun.sh to start the test case with WAIT_FOR_DEBUGGER=1, then
> +# automatically attach GDB to it.
> +WAIT_FOR_DEBUGGER=1 $(common-objpfx)testrun.sh --tool=container $${TESTCASE} &
> +gdb -x $${TESTCASE}.gdb

OK. Good use of this infrastructure.

> +else
>  # Start the test case debugging in two steps:
>  #   1. the following command invokes gdb to run the loader;
>  #   2. the commands file tells the loader to run the test case.
> @@ -355,6 +374,7 @@ gdb -q \
>    -x $${CMD_FILE} \
>    -d $${SOURCE_DIR} \
>    $${BUILD_DIR}/elf/ld.so
> +fi

OK.

>  endef
>  
>  # This is another handy script for debugging dynamically linked program
> 


-- 
Cheers,
Carlos.

Re: [PATCH 2/2] Do not use ld.so to open statically linked programs in debugglibc.sh

[Carlos O'Donell]

On 12/3/19 9:36 PM, Gabriel F. T. Gomes wrote:
> From: "Gabriel F. T. Gomes" <gabrielftg@linux.ibm.com>
> 
> Debugging programs that have been dynamically linked against an
> uninstalled glibc requires unusual steps, such as letting gdb know where
> the thread db library is located and explicitly calling the loader.
> However, when the program under test is statically linked, these steps
> are not required (as a matter of fact, using the dynamic loader to run a
> statically linked program is wrong and will fail), and gdb should be
> called the usual way.

We still need to inform gdb where the libthread_db.so is though?

This is better than what we have and helps. I think having
this code in place means people can use it and it works, even if we have
slight problems we'll iron them out as we use it.

OK for master. 

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> This patch modifies debugglibc.sh so that it checks if the program under
> test is statically linked, then runs the debugger appropriately.
> ---
>  Makefile | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 924fdb6c0f..b43226c35a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -209,6 +209,7 @@ BUILD_DIR="$(common-objpfx)"
>  CMD_FILE="$(common-objpfx)debugglibc.gdb"
>  CONTAINER=false
>  DIRECT=true
> +STATIC=false

OK.

>  SYMBOLSFILE=true
>  unset TESTCASE
>  unset BREAKPOINTS
> @@ -297,8 +298,8 @@ do
>    shift
>  done
>  
> -# Check for required argument
> -if [ ! -v TESTCASE ]
> +# Check for required argument and if the testcase exists
> +if [ ! -v TESTCASE ] || [ ! -f $${TESTCASE} ]

OK.

>  then
>    usage
>    exit 1
> @@ -318,6 +319,14 @@ else
>    DIRECT=""
>  fi
>  
> +# Check if the test case is static
> +if file $${TESTCASE} | grep "statically linked" >/dev/null
> +then
> +  STATIC=true
> +else
> +  STATIC=false
> +fi

OK. Detect with `file` which is good enough.

> +
>  # Expand symbols loading command
>  if [ "$$SYMBOLSFILE" == true ]
>  then
> @@ -366,6 +375,9 @@ then
>  # automatically attach GDB to it.
>  WAIT_FOR_DEBUGGER=1 $(common-objpfx)testrun.sh --tool=container $${TESTCASE} &
>  gdb -x $${TESTCASE}.gdb
> +elif [ "$$STATIC" == true ]
> +then
> +gdb $${TESTCASE}

OK. This is better than anything, but we might need more to make threads in
static binaries work properly.

>  else
>  # Start the test case debugging in two steps:
>  #   1. the following command invokes gdb to run the loader;
> 


-- 
Cheers,
Carlos.