Re: [PATCH v3] Add helper script for glibc debugging

[Carlos O'Donell <carlos at redhat dot com>]

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

Almost! Thank you for working through this, I think the script will be
really useful for new and old people... I always have to retype this
from the wiki ;}

See my suggestions for v3 and then I think we're done:
- Suggestion for more verbose output.
- Remove wrong comment.
- Test with --enable-hardcoded-path-in-tests

> Changes since v2:
> 
>   - Call gdb directly, instead of loading it with ld.so;
>   - Use 'set auto-load safe-path' and 'set libthread-db-search-path',
>     to let gdb know of libthread_db.so location;
>   - Let environment variables be set from the command line (-e option).
> 
> Changes since v1:
> 
>   - Added nptl_db to the argument to --library-path, which allows
>     debugging with multiple threads (see additional comment below), as
>     observed by Carlos;
>   - Removed the hardcoded path to GDB, as suggested by Dmitry;
>   - Removed the default test case and source directory paths, as
>     suggested by Joseph, which removed some of the clutter in the
>     script;
> 
> Note about multi-threaded test case debugging:
> 
> As Carlos correctly noticed, the first version of the script lacked the
> required GDB setup for multi-threaded debugging.  This could be noticed
> by running a multi-threaded test case, such as nptl/tst-exec1, and
> observing the warning message in the output:
> 
>   warning: Unable to find libthread_db matching inferior's thread
>            library, thread debugging will not be available.
> 
> In this second version, which explicitly adds nptl_db to the argument of
> --library-path, the warning message is gone, and converted into the
> following info message:
> 
>   [Thread debugging using libthread_db enabled]
>   Using host libthread_db library
>   "/home/gabriel/build/x86_64/glibc//nptl_db/libthread_db.so.1".
> 
> Not-for-commit comment from v1:
> 
> At Cauldron, while Florian showed his development workflow to us, we had
> a brief discussion about debugging glibc with GDB.  After that, I spent
> some time with Arjun and we shared our debugging scripts and tricks,
> then we came to the conclusion that this could be helpful to other
> people, maybe even more so to newcomers.  So, I got my off-tree script
> and converted it into this auto-generated version.
> 
> Still at Cauldron, Carlos mentioned that it wasn't trivial to let GDB
> know about the symbols within glibc.  This script doesn't solve this for
> every glibc debugging needs (I suppose it doesn't help at all when
> debugging the initial steps of program loading, for example), but it
> makes it a little easier when debugging test cases, as it automatically
> loads the symbols from them.
> 
> Maybe there are more trivial ways to do it, but that's what I have been
> using for some time.
> 
> -- 8< --

OK. Nice commit message.

> This patch adds a new make rule that generates a helper script for
> debugging glibc test cases.  The new script, debugglibc.sh, is similar
> to testrun.sh, in the sense that it allows the execution of the
> specified test case, however, it opens the test case in GDB, setting the
> library path the same way that testrun.sh does.  The commands are based
> on the instructions on the wiki page for glibc debugging [1].
> 
> By default, the script tells GDB to load the test case for symbol
> information, so that, when a breakpoint is hit, the call stack is
> displayed correctly (instead of printing lots of '??'s).  For instance,
> after running 'make' and 'make check', one could do the following:
> 
>   $ ./debugglibc.sh nptl/tst-exec1 -b pthread_join
> 
>   Reading symbols from /home/gabriel/build/powerpc64le/glibc//elf/ld.so...done.
>   Breakpoint 1 at 0x1444
>   add symbol table from file "nptl/tst-exec1"
>   [Thread debugging using libthread_db enabled]
>   Using host libthread_db library "/home/gabriel/build/powerpc64le/glibc//nptl_db/libthread_db.so.1".
> 
>   Breakpoint 1, 0x00007ffff7fb1444 in _dl_start_user () from /home/gabriel/build/powerpc64le/glibc/elf/ld.so
>   Breakpoint 2 at 0x7ffff7f49d48: file pthread_join.c, line 23.
> 
> Notice that the script will always start GDB with the program running
> and halted at _dl_start_user.  So, in order to reach the actual
> breakpoint of interest, one should hit 'c', not 'r':
> 
>   >>> c
>   Continuing.
>   [New Thread 0x7ffff7d1f180 (LWP 76443)]
>   [Switching to Thread 0x7ffff7d1f180 (LWP 76443)]
> 
>   Thread 2 "ld.so" hit Breakpoint 2, __pthread_join (threadid=140737354087616, thread_return=0x0) at pthread_join.c:24
>   24        return __pthread_timedjoin_ex (threadid, thread_return, NULL, true);
> 
> Then inspect the call stack with 'bt', as usual, and see symbols from
> both the test case and from the libraries themselves:
> 
>   >>> bt
>   #0  __pthread_join (threadid=140737354087616, thread_return=0x0) at pthread_join.c:24
>   #1  0x0000000010001f4c in tf (arg=<optimized out>) at tst-exec1.c:37
>   #2  0x00007ffff7f487e8 in start_thread (arg=0x7ffff7510000) at pthread_create.c:479
>   #3  0x00007ffff7e523a8 in clone () at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:82
> 
> Tested for powerpc64le and x86_64.
> 
> [1] https://sourceware.org/glibc/wiki/Debugging/Loader_Debugging
> ---
>  Makefile | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 157 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 67ddd01bfe..f1df06d3c7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -187,7 +187,163 @@ $(common-objpfx)testrun.sh: $(common-objpfx)config.make \
>  	mv -f $@T $@
>  postclean-generated += testrun.sh
>  
> -others: $(common-objpfx)testrun.sh
> +define debugglibc

OK.

> +#!/bin/bash
> +
> +SOURCE_DIR="$(CURDIR)"
> +BUILD_DIR="$(common-objpfx)"
> +CMD_FILE="$(common-objpfx)debugglibc.gdb"
> +DIRECT=true
> +SYMBOLSFILE=true
> +unset TESTCASE
> +unset BREAKPOINTS
> +unset ENVVARS
> +
> +usage()
> +{
> +cat << EOF
> +Usage: $$0 [OPTIONS] <testcase>
> +
> +  where <testcase> is the path to the program being tested.

OK.

> +
> +Options:
> +
> +  -h, --help
> +	Prints this message and leaves.
> +
> +  The following options require one argument:
> +
> +  -b, --breakpoint
> +	Breakpoints to set at the beginning of the execution
> +	(each breakpoint demands its own -b option, e.g. -b foo -b bar)
> +  -e, --environment-variable
> +	Environment variables to be set with 'exec-wrapper env' in GDB
> +	(each environment variable demands its own -e option, e.g.
> +	-e FOO=foo -e BAR=bar)
> +
> +  The following options do not take arguments:
> +
> +  -i, --no-direct
> +	Selects whether to pass the flag --direct to gdb.
> +	Required for glibc test cases and not allowed for non-glibc tests.
> +	Default behaviour is to pass the flag --direct to gdb.
> +  -s, --no-symbols-file
> +	Do not tell GDB to load debug symbols from the testcase.

OK.

> +EOF
> +}
> +
> +# Parse input options
> +while [[ $$# > 0 ]]
> +do
> +  key="$$1"
> +  case $$key in
> +    -h|--help)
> +      usage
> +      exit 0
> +      ;;
> +    -b|--breakpoint)
> +      BREAKPOINTS="break $$2\n$$BREAKPOINTS"
> +      shift
> +      ;;
> +    -e|--environment-variable)
> +      ENVVARS="$$2 $$ENVVARS"
> +      shift
> +      ;;
> +    -i|--no-direct)
> +      DIRECT=false
> +      ;;
> +    -s|--no-symbols-file)
> +      SYMBOLSFILE=false
> +      ;;
> +    *)
> +      TESTCASE=$$1
> +      ;;
> +  esac
> +  shift
> +done

OK.

> +
> +# Check for required argument
> +if [ ! -v TESTCASE ]
> +then
> +  usage
> +  exit 1
> +fi
> +
> +# Expand environment setup command
> +if [ -v ENVVARS ]
> +then
> +  ENVVARS="set exec-wrapper env $$ENVVARS"
> +fi
> +
> +# Expand direct argument
> +if [ "$$DIRECT" == true ]
> +then
> +  DIRECT="--direct"
> +else
> +  DIRECT=""
> +fi
> +
> +# Expand symbols loading command
> +if [ "$$SYMBOLSFILE" == true ]
> +then
> +  SYMBOLSFILE="add-symbol-file $${TESTCASE}"
> +else
> +  SYMBOLSFILE=""
> +fi
> +
> +# GDB commands template
> +template ()
> +{
> +cat <<EOF
> +set environment C -E -x c-header
> +set auto-load safe-path $${BUILD_DIR}/nptl_db:\$$debugdir:\$$datadir/auto-load
> +set libthread-db-search-path $${BUILD_DIR}/nptl_db
> +__ENVVARS__
> +__SYMBOLSFILE__
> +break _dl_start_user
> +run --library-path $(rpath-link):$${BUILD_DIR}/nptl_db \

OK, run the loader directly.

> +__TESTCASE__ __DIRECT__
> +__BREAKPOINTS__

OK.

> +EOF
> +}

OK.

> +
> +# Generate the commands file for gdb initialization
> +template | sed \
> +  -e "s|__ENVVARS__|$$ENVVARS|" \
> +  -e "s|__SYMBOLSFILE__|$$SYMBOLSFILE|" \
> +  -e "s|__TESTCASE__|$$TESTCASE|" \
> +  -e "s|__DIRECT__|$$DIRECT|" \
> +  -e "s|__BREAKPOINTS__|$$BREAKPOINTS|" \
> +  > $$CMD_FILE
> +
> +echo
> +echo "Debugging glibc..."
> +echo "Build directory  : $$BUILD_DIR"
> +echo "Source directory : $$SOURCE_DIR"
> +echo "GLIBC Testcase   : $$TESTCASE"
> +echo "GDB Commands     : $$CMD_FILE"

Suggest:

echo "Env vars: $$ENVVARS"

> +echo
> +
> +# We need to make sure that gdb is linked against the standalone glibc
> +# so that it picks up the correct nptl_db/libthread_db.so. So that means
> +# invoking gdb using the standalone glibc's linker and passing nptl_db
> +# in the argument to --library-path.

This comment is no longer correct. We should remove it or rewrite it.

> +gdb -q \
> +  -x $${CMD_FILE} \
> +  -d $${SOURCE_DIR} \
> +  $${BUILD_DIR}/elf/ld.so
> +endef
> +
> +# This is another handy script for debugging dynamically linked program
> +# against the current libc build for testing.
> +$(common-objpfx)debugglibc.sh: $(common-objpfx)config.make \
> +			    $(..)Makeconfig $(..)Makefile
> +	$(file >$@T,$(debugglibc))
> +	chmod a+x $@T
> +	mv -f $@T $@
> +postclean-generated += debugglibc.sh debugglibc.gdb
> +
> +others: $(common-objpfx)testrun.sh $(common-objpfx)debugglibc.sh
>  
>  # Makerules creates a file `stubs' in each subdirectory, which
>  # contains `#define __stub_FUNCTION' for each function defined in that
> 

Did you test the script with --enalbe-hardcoded-path-in-tests also? ;}

-- 
Cheers,
Carlos.