public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add tests for finding debug information within sysroot
@ 2024-05-20 14:14 Andrew Burgess
  2024-05-20 14:14 ` [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent Andrew Burgess
  2024-05-20 14:14 ` [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot Andrew Burgess
  0 siblings, 2 replies; 10+ messages in thread
From: Andrew Burgess @ 2024-05-20 14:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

This is an attempt to address PR gdb/30871.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb/testsuite: make gdb_gnu_strip_debug consistent
  gdb/testsuite: tests for debug lookup within the sysroot

 gdb/testsuite/gdb.base/corefile-buildid.exp   |  18 +-
 gdb/testsuite/gdb.base/sysroot-debug-lookup.c |  22 +++
 .../gdb.base/sysroot-debug-lookup.exp         | 162 ++++++++++++++++++
 .../build-id-no-debug-warning.exp             |  19 +-
 gdb/testsuite/lib/gdb.exp                     |  38 +++-
 5 files changed, 222 insertions(+), 37 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.c
 create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.exp


base-commit: ad666becfe075ca7c831ebbf4b44526994395e97
-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent
  2024-05-20 14:14 [PATCH 0/2] Add tests for finding debug information within sysroot Andrew Burgess
@ 2024-05-20 14:14 ` Andrew Burgess
  2024-05-23  9:13   ` Tom de Vries
  2024-05-20 14:14 ` [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot Andrew Burgess
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2024-05-20 14:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

While writing a test I realised that the default behaviour of
gdb_gnu_strip_debug doesn't match its comment.

The comment says that the function takes a FILENAME, and splits the
file into FILENAME.stripped and FILENAME.debug, leaving FILENAME
unchanged.  The comment says that a .gnu_debuglink will be added to
FILENAME.stripped.

However, this is not true, FILENAME.stripped is created, with no debug
information.  FILENAME.debug is created containing the debug
information.

But, when adding the .gnu_debuglink we take FILENAME.stripped as the
input, and then overwrite FILENAME with the output.  As a result,
FILENAME.stripped does not include a .gnu_debuglink, while FILENAME
contains the .gnu_debuglink and no debug information!

The users of gdb_gnu_strip_debug can be split into two groups, those
who are using the .gnu_debuglink, these tests are all written assuming
that FILENAME is updated.

Then there are some tests that rely on gdb_gnu_strip_debug's ability
to split out the debug information, these tests are then going to do a
lookup based on the build-id.  These tests use the FILENAME.stripped
output file.

This all seems too confused to me.

As most used of gdb_gnu_strip_debug assume that FILENAME is updated, I
propose that we just make that the actual, advertised behaviour of
this proc.

So now, gdb_gnu_strip_debug will take FILENAME, and will split the
debug information out into FILENAME.debug.  The debug information will
then be stripped from FILENAME, and by default a .gnu_debuglink will
be added to FILENAME pointing to FILENAME.debug.

I've updated the two tests that actually relied on FILENAME.stripped
to instead just use FILENAME.

One of the tests was doing a build-id based lookup, but was still
allowing the .gnu_debuglink to be added to FILENAME, I've updated this
test to pass the no-debuglink flag to gdb_gnu_strip_debug, which stops
the .gnu_debuglink from being added.

All of the tests that call gdb_gnu_strip_debug still pass for me.
---
 gdb/testsuite/gdb.base/corefile-buildid.exp   | 18 +++----------
 .../build-id-no-debug-warning.exp             | 19 ++++----------
 gdb/testsuite/lib/gdb.exp                     | 25 ++++++++++++-------
 3 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/gdb/testsuite/gdb.base/corefile-buildid.exp b/gdb/testsuite/gdb.base/corefile-buildid.exp
index 130198611ec..e1b9804d891 100644
--- a/gdb/testsuite/gdb.base/corefile-buildid.exp
+++ b/gdb/testsuite/gdb.base/corefile-buildid.exp
@@ -172,11 +172,9 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
 	"mkdir -p [file join $debugdir [file dirname $buildid]]"
 
     set files_list {}
+    lappend files_list $binfile $buildid
     if {$sepdebug} {
-	lappend files_list "$binfile.stripped" $buildid
 	lappend files_list "$binfile.debug" "$buildid.debug"
-    } else {
-	lappend files_list $binfile $buildid
     }
     if {$shared} {
 	global sharedir
@@ -200,12 +198,7 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
     gdb_test "core-file $corefile" "Program terminated with .*" \
 	"load core file"
     if {$symlink} {
-	if {$sepdebug} {
-	    set expected_file [file join $builddir \
-				   [file tail "$binfile.stripped"]]
-	} else {
-	    set expected_file [file join $builddir [file tail $binfile]]
-	}
+	set expected_file [file join $builddir [file tail $binfile]]
     } else {
 	set expected_file $buildid
     }
@@ -245,15 +238,12 @@ proc do_corefile_buildid_tests {args} {
 
     if {$sepdebug} {
 	# Strip debuginfo into its own file.
-	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run]] \
-		!= 0} {
+	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run] \
+		 no-debuglink] != 0} {
 	    untested "could not strip executable  for [join $suffix \ ]"
 	    return
 	}
 
-	# Run the stripped program instead of the original.
-	set program_to_run [file join $builddir \
-				[file tail "$binfile.stripped"]]
 	lappend suffix "sepdebug"
     }
 
diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
index b86622543ef..aa1c263e800 100644
--- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
+++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
@@ -30,14 +30,11 @@ if {[build_executable "build executable" ${testfile} ${srcfile} \
     return -1
 }
 
-# Split BINFILE into BINFILE.stripped and BINFILE.debug, the first is
-# the executable with the debug information removed, and the second is
-# the debug information.
+# Split debug information from BINFILE into BINFILE.debug.
 #
-# However, by passing the "no-debuglink" flag we prevent this proc
-# from adding a .gnu_debuglink section to the executable.  Any lookup
-# of the debug information by GDB will need to be done based on the
-# build-id.
+# By passing the "no-debuglink" flag we prevent this proc from adding
+# a .gnu_debuglink section to BINFILE.  Any lookup of the debug
+# information by GDB will need to be done based on the build-id.
 if {[gdb_gnu_strip_debug $binfile no-debuglink]} {
     unsupported "cannot produce separate debug info files"
     return -1
@@ -59,12 +56,6 @@ set debuginfod_debugdir [standard_output_file "debug"]
 remote_exec build "mkdir $debuginfod_debugdir"
 remote_exec build "mv $debugfile $debuginfod_debugdir"
 
-# This is BINFILE with the debug information removed.  We are going to
-# place this in the BUILD_ID_DEBUG_FILE location, this would usually
-# represent a mistake by the user, and will trigger a warning from
-# GDB, this is the warning we are checking for.
-set stripped_binfile [standard_output_file "${binfile}.stripped"]
-
 # Create the .build-id/PREFIX directory name from
 # .build-id/PREFIX/SUFFIX.debug filename.
 set debugdir [file dirname ${build_id_debug_file}]
@@ -76,7 +67,7 @@ remote_exec build "mkdir -p $debugdir"
 # information, which will point back at this file, which also doesn't
 # have debug information, which could cause a loop.  But GDB will spot
 # this and give a warning.
-remote_exec build "mv ${stripped_binfile} ${build_id_debug_file}"
+remote_exec build "mv ${binfile} ${build_id_debug_file}"
 
 # Now start GDB.
 clean_restart
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0d78691c381..a63c13f9cbc 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8017,10 +8017,11 @@ proc build_id_debug_filename_get { filename } {
 }
 
 # DEST should be a file compiled with debug information.  This proc
-# creates two new files DEST.debug which contains the debug
-# information extracted from DEST, and DEST.stripped, which is a copy
-# of DEST with the debug information removed.  A '.gnu_debuglink'
-# section will be added to DEST.stripped that points to DEST.debug.
+# creates DEST.debug which contains the debug information extracted
+# from DEST, and DEST is updated with the debug information removed.
+#
+# By default a '.gnu_debuglink' section will be added to DEST that
+# points to DEST.debug.
 #
 # If ARGS is passed, it is a list of optional flags.  The currently
 # supported flags are:
@@ -8028,7 +8029,7 @@ proc build_id_debug_filename_get { filename } {
 #   - no-main : remove the symbol entry for main from the separate
 #               debug file DEST.debug,
 #   - no-debuglink : don't add the '.gnu_debuglink' section to
-#                    DEST.stripped.
+#                    DEST.
 #
 # Function returns zero on success.  Function will return non-zero failure code
 # on some targets not supporting separate debug info (such as i386-msdos).
@@ -8087,20 +8088,26 @@ proc gdb_gnu_strip_debug { dest args } {
     # Unless the "no-debuglink" flag is passed, then link the two
     # previous output files together, adding the .gnu_debuglink
     # section to the stripped_file, containing a pointer to the
-    # debug_file, save the new file in dest.
+    # debug_file.
     if {[lsearch -exact $args "no-debuglink"] == -1} {
-	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${dest}" output]
+	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${stripped_file}-tmp" output]
 	verbose "result is $result"
 	verbose "output is $output"
 	if {$result == 1} {
 	    return 1
 	}
+	file delete "${stripped_file}"
+	file rename "${stripped_file}-tmp" "${stripped_file}"
     }
 
     # Workaround PR binutils/10802:
     # Preserve the 'x' bit also for PIEs (Position Independent Executables).
-    set perm [file attributes ${stripped_file} -permissions]
-    file attributes ${dest} -permissions $perm
+    set perm [file attributes ${dest} -permissions]
+    file attributes ${stripped_file} -permissions $perm
+
+    # Move the stripped_file back into dest.
+    file delete ${dest}
+    file rename ${stripped_file} ${dest}
 
     return 0
 }
-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot
  2024-05-20 14:14 [PATCH 0/2] Add tests for finding debug information within sysroot Andrew Burgess
  2024-05-20 14:14 ` [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent Andrew Burgess
@ 2024-05-20 14:14 ` Andrew Burgess
  2024-05-23 10:17   ` Tom de Vries
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2024-05-20 14:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Add tests for looking up debug information within the sysroot via both
build-id and gnu_debuglink.

I wanted to ensure that the gnu_debuglink test couldn't make use of
build-ids, so I added the 'no-build-id' flag to gdb_compile.

As these tests rely on setting the sysroot, if I'm running a
dynamically linked executable, GDB will try to find all shared
libraries within the sysroot.  This would mean I'd have to figure out
and copy all shared libraries the executable uses, certainly possible,
but a bit of a pain.

So instead, I've just compiled the test executable as a static binary.
Now there are no shared library dependencies.

I can now split the debug information out from the test binary, and
place it within the sysroot.  When GDB is started and the executable
loaded, we can check that GDB is finding the debug information within
the sysroot.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30871
---
 gdb/testsuite/gdb.base/sysroot-debug-lookup.c |  22 +++
 .../gdb.base/sysroot-debug-lookup.exp         | 162 ++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     |  13 ++
 3 files changed, 197 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.c
 create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.exp

diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.c b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
new file mode 100644
index 00000000000..e67331d3150
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 1992-2024 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
new file mode 100644
index 00000000000..e0377df0fa7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
@@ -0,0 +1,162 @@
+# Copyright 2024 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB's ability to find debug information by looking within the
+# sysroot.
+#
+# We compile a static binary (to reduce what we need to copy into the
+# sysroot), split the debug information from the binary, and setup a
+# sysroot.
+#
+# The debug-file-directory is set to just '/debug', but we're
+# expecting GDB to actually look in '$SYSROOT/debug'.
+#
+# There's a test for using .build-id based lookup, and a test for
+# gnu_debuglink based lookup.
+
+require {!is_remote host}
+
+standard_testfile
+
+# Create a copy of BINFILE, split out the debug information, and then
+# setup a sysroot.  Hide (by moving) the actual debug information file
+# and create a symlink to the hidden debug information from within the
+# sysroot.
+#
+# Start GDB, set the sysroot, and then load the executable, ensure GDB
+# finds the debug information, which must have happened by lookin in
+# the sysroot.
+proc_with_prefix lookup_via_build_id {} {
+    set filename ${::binfile}_1
+    if { [build_executable "build exec" ${filename} $::srcfile \
+	      {additional_flags=-static debug build-id}] } {
+	return
+    }
+
+    # Split debug information into a .debug file, remove debug
+    # information from FILENAME.  Don't add a .gnu_debuglink to
+    # FILENAME, we rely on the build-id.
+    if {[gdb_gnu_strip_debug $filename { no-debuglink }] != 0} {
+	unsupported "cannot split debug information from executable"
+	return
+    }
+
+    set sysroot [standard_output_file "sysroot1"]
+    set debug_dir "/debug"
+
+    set debug_symlink \
+	$sysroot/$debug_dir/[build_id_debug_filename_get $filename]
+
+    set build_id_dir [file dirname $debug_symlink]
+
+    set debug_filename ${filename}_hidden_debug
+
+    remote_exec build "mkdir -p $build_id_dir"
+    remote_exec build "mv $filename.debug $debug_filename"
+    remote_exec build "ln -sf $debug_filename $debug_symlink"
+
+    clean_restart
+
+    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
+    gdb_test_no_output "set debug-file-directory $debug_dir"
+
+    gdb_file_cmd $filename
+
+    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
+
+    set re [string_to_regexp "Reading symbols from $debug_filename..."]
+    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
+}
+
+# Create a copy of BINFILE, split out the debug information, and then
+# setup a sysroot.  Hide (by moving) the actual debug information file
+# and create a symlink to the hidden debug information from within the
+# sysroot.
+#
+# Copy the executable into the sysroot and then start GDB, set the
+# sysroot, and load the executable.  Check that GDB finds the debug
+# information, which must have happened by lookin in the sysroot.
+proc_with_prefix lookup_via_debuglink {} {
+    set filename ${::binfile}_2
+    if { [build_executable "build exec" ${filename} $::srcfile \
+	      {additional_flags=-static debug no-build-id}] } {
+	return
+    }
+
+    # Split debug information into a .debug file, remove debug
+    # information from FILENAME.
+    if {[gdb_gnu_strip_debug $filename] != 0} {
+	unsupported "cannot split debug information from executable"
+	return
+    }
+
+    # We're going to setup the sysroot like this:
+    #
+    # sysroot2/
+    #    bin/
+    #      $FILENAME
+    #    debug/
+    #      bin/
+    #        $FILENAME.debug
+    #
+    # When looking up debug information via the debuglink, GDB will
+    # only search in the sysroot if the original objfile was in the
+    # sysroot.  And GDB will resolve symlinks, so if the objfile is
+    # symlinked to outside the sysroot, GDB will not search in the
+    # sysroot for the debug information.
+    #
+    # So we have to copy the executable into the sysroot.
+    #
+    # We are OK to symlink the debug information to a file outside the
+    # sysroot though.
+
+    set sysroot [standard_output_file "sysroot2"]
+
+    foreach path { bin debug/bin } {
+	remote_exec build "mkdir -p $sysroot/$path"
+    }
+
+    # Copy the executable into the sysroot.
+    set file_basename [file tail $filename]
+    set exec_in_sysroot ${sysroot}/bin/${file_basename}
+    remote_exec build "cp $filename $exec_in_sysroot"
+
+    # Rename the debug file outside of the sysroot, this should stop
+    # GDB finding this file "by accident".
+    set debug_filename ${filename}_hidden_debug
+    remote_exec build "mv $filename.debug $debug_filename"
+
+    # Symlink the debug information into the sysroot.
+    set debug_symlink \
+	${sysroot}/debug/bin/${file_basename}.debug
+    remote_exec build "ln -sf $debug_filename $debug_symlink"
+
+    # Restart GDB and setup the sysroot and debug directory.
+    clean_restart
+    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
+    gdb_test_no_output "set debug-file-directory /debug"
+
+    # Load the executable, we expect GDB to find the debug information
+    # in the sysroot.
+    gdb_file_cmd $exec_in_sysroot
+
+    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
+
+    set re [string_to_regexp "Reading symbols from $debug_symlink..."]
+    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
+}
+
+lookup_via_build_id
+lookup_via_debuglink
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a63c13f9cbc..a8e8b69d6ef 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5211,6 +5211,7 @@ proc quote_for_host { args } {
 #     debug information
 #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
 #   - build-id: Ensure the final binary includes a build-id.
+#   - no-build-id: Ensure the final binary does not include a build-id.
 #   - column-info/no-column-info: Enable/Disable generation of column table
 #     information.
 #
@@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} {
 	lappend new_options "additional_flags=-Wl,--build-id"
     }
 
+    # If the 'no-build-id' option is used then disable the build-id.
+    if {[lsearch -exact $options no-build-id] > 0} {
+	lappend new_options "additional_flags=-Wl,--build-id=none"
+    }
+
+    # Sanity check.  If both 'build-id' and 'no-build-id' are used
+    # then what is expected from us!
+    if {[lsearch -exact $options build-id] > 0
+	&& [lsearch -exact $options no-build-id] > 0} {
+	error "cannot use build-id and no-build-id options"
+    }
+
     # Treating .c input files as C++ is deprecated in Clang, so
     # explicitly force C++ language.
     if { !$getting_compiler_info
-- 
2.25.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent
  2024-05-20 14:14 ` [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent Andrew Burgess
@ 2024-05-23  9:13   ` Tom de Vries
  2024-05-26 23:08     ` Andrew Burgess
  0 siblings, 1 reply; 10+ messages in thread
From: Tom de Vries @ 2024-05-23  9:13 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 5/20/24 16:14, Andrew Burgess wrote:
> While writing a test I realised that the default behaviour of
> gdb_gnu_strip_debug doesn't match its comment.
> 
> The comment says that the function takes a FILENAME, and splits the
> file into FILENAME.stripped and FILENAME.debug, leaving FILENAME
> unchanged.  The comment says that a .gnu_debuglink will be added to
> FILENAME.stripped.
> 
> However, this is not true, FILENAME.stripped is created, with no debug
> information.  FILENAME.debug is created containing the debug
> information.
> 
> But, when adding the .gnu_debuglink we take FILENAME.stripped as the
> input, and then overwrite FILENAME with the output.  As a result,
> FILENAME.stripped does not include a .gnu_debuglink, while FILENAME
> contains the .gnu_debuglink and no debug information!
> 
> The users of gdb_gnu_strip_debug can be split into two groups, those
> who are using the .gnu_debuglink, these tests are all written assuming
> that FILENAME is updated.
> 
> Then there are some tests that rely on gdb_gnu_strip_debug's ability
> to split out the debug information,

I got confused a bit here.  Doesn't the first group also rely on this?

> these tests are then going to do a
> lookup based on the build-id.  These tests use the FILENAME.stripped
> output file.
> 
> This all seems too confused to me.
> 
> As most used of gdb_gnu_strip_debug assume that FILENAME is updated, I

used -> uses

> propose that we just make that the actual, advertised behaviour of
> this proc.
> 

Make sense.

[ FWIW, the overwriting behaviour is not great for the case where the 
exec is used in the initial part of a test-case, and then modified by 
gdb_gnu_strip_debug (I don't know whether that occurs though).  When 
trying to reproduce the initial part of the tests using say -x gdb.in.1 
the behaviour may be different because the exec has changed in the mean 
time.  So going from that point of view we could have gone for the 
syntax of gdb_gnu_strip_debug returning the filename of the stripped 
exec, if successful.  Come to think of it, doing so might also allow us 
to name the stripped exec differently, say foo.stripped and 
foo.stripped-with-link, which will make it more clear in gdb.log what 
exec is used. ]

I browsed a bit through the code changes, and they look OK to me, but I 
haven't looked in detail, so ...

Acked-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

> So now, gdb_gnu_strip_debug will take FILENAME, and will split the
> debug information out into FILENAME.debug.  The debug information will
> then be stripped from FILENAME, and by default a .gnu_debuglink will
> be added to FILENAME pointing to FILENAME.debug.
> 
> I've updated the two tests that actually relied on FILENAME.stripped
> to instead just use FILENAME.
> 
> One of the tests was doing a build-id based lookup, but was still
> allowing the .gnu_debuglink to be added to FILENAME, I've updated this
> test to pass the no-debuglink flag to gdb_gnu_strip_debug, which stops
> the .gnu_debuglink from being added.
> 
> All of the tests that call gdb_gnu_strip_debug still pass for me.
> ---
>   gdb/testsuite/gdb.base/corefile-buildid.exp   | 18 +++----------
>   .../build-id-no-debug-warning.exp             | 19 ++++----------
>   gdb/testsuite/lib/gdb.exp                     | 25 ++++++++++++-------
>   3 files changed, 25 insertions(+), 37 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/corefile-buildid.exp b/gdb/testsuite/gdb.base/corefile-buildid.exp
> index 130198611ec..e1b9804d891 100644
> --- a/gdb/testsuite/gdb.base/corefile-buildid.exp
> +++ b/gdb/testsuite/gdb.base/corefile-buildid.exp
> @@ -172,11 +172,9 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
>   	"mkdir -p [file join $debugdir [file dirname $buildid]]"
>   
>       set files_list {}
> +    lappend files_list $binfile $buildid
>       if {$sepdebug} {
> -	lappend files_list "$binfile.stripped" $buildid
>   	lappend files_list "$binfile.debug" "$buildid.debug"
> -    } else {
> -	lappend files_list $binfile $buildid
>       }
>       if {$shared} {
>   	global sharedir
> @@ -200,12 +198,7 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
>       gdb_test "core-file $corefile" "Program terminated with .*" \
>   	"load core file"
>       if {$symlink} {
> -	if {$sepdebug} {
> -	    set expected_file [file join $builddir \
> -				   [file tail "$binfile.stripped"]]
> -	} else {
> -	    set expected_file [file join $builddir [file tail $binfile]]
> -	}
> +	set expected_file [file join $builddir [file tail $binfile]]
>       } else {
>   	set expected_file $buildid
>       }
> @@ -245,15 +238,12 @@ proc do_corefile_buildid_tests {args} {
>   
>       if {$sepdebug} {
>   	# Strip debuginfo into its own file.
> -	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run]] \
> -		!= 0} {
> +	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run] \
> +		 no-debuglink] != 0} {
>   	    untested "could not strip executable  for [join $suffix \ ]"
>   	    return
>   	}
>   
> -	# Run the stripped program instead of the original.
> -	set program_to_run [file join $builddir \
> -				[file tail "$binfile.stripped"]]
>   	lappend suffix "sepdebug"
>       }
>   
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> index b86622543ef..aa1c263e800 100644
> --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> @@ -30,14 +30,11 @@ if {[build_executable "build executable" ${testfile} ${srcfile} \
>       return -1
>   }
>   
> -# Split BINFILE into BINFILE.stripped and BINFILE.debug, the first is
> -# the executable with the debug information removed, and the second is
> -# the debug information.
> +# Split debug information from BINFILE into BINFILE.debug.
>   #
> -# However, by passing the "no-debuglink" flag we prevent this proc
> -# from adding a .gnu_debuglink section to the executable.  Any lookup
> -# of the debug information by GDB will need to be done based on the
> -# build-id.
> +# By passing the "no-debuglink" flag we prevent this proc from adding
> +# a .gnu_debuglink section to BINFILE.  Any lookup of the debug
> +# information by GDB will need to be done based on the build-id.
>   if {[gdb_gnu_strip_debug $binfile no-debuglink]} {
>       unsupported "cannot produce separate debug info files"
>       return -1
> @@ -59,12 +56,6 @@ set debuginfod_debugdir [standard_output_file "debug"]
>   remote_exec build "mkdir $debuginfod_debugdir"
>   remote_exec build "mv $debugfile $debuginfod_debugdir"
>   
> -# This is BINFILE with the debug information removed.  We are going to
> -# place this in the BUILD_ID_DEBUG_FILE location, this would usually
> -# represent a mistake by the user, and will trigger a warning from
> -# GDB, this is the warning we are checking for.
> -set stripped_binfile [standard_output_file "${binfile}.stripped"]
> -
>   # Create the .build-id/PREFIX directory name from
>   # .build-id/PREFIX/SUFFIX.debug filename.
>   set debugdir [file dirname ${build_id_debug_file}]
> @@ -76,7 +67,7 @@ remote_exec build "mkdir -p $debugdir"
>   # information, which will point back at this file, which also doesn't
>   # have debug information, which could cause a loop.  But GDB will spot
>   # this and give a warning.
> -remote_exec build "mv ${stripped_binfile} ${build_id_debug_file}"
> +remote_exec build "mv ${binfile} ${build_id_debug_file}"
>   
>   # Now start GDB.
>   clean_restart
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 0d78691c381..a63c13f9cbc 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -8017,10 +8017,11 @@ proc build_id_debug_filename_get { filename } {
>   }
>   
>   # DEST should be a file compiled with debug information.  This proc
> -# creates two new files DEST.debug which contains the debug
> -# information extracted from DEST, and DEST.stripped, which is a copy
> -# of DEST with the debug information removed.  A '.gnu_debuglink'
> -# section will be added to DEST.stripped that points to DEST.debug.
> +# creates DEST.debug which contains the debug information extracted
> +# from DEST, and DEST is updated with the debug information removed.
> +#
> +# By default a '.gnu_debuglink' section will be added to DEST that
> +# points to DEST.debug.
>   #
>   # If ARGS is passed, it is a list of optional flags.  The currently
>   # supported flags are:
> @@ -8028,7 +8029,7 @@ proc build_id_debug_filename_get { filename } {
>   #   - no-main : remove the symbol entry for main from the separate
>   #               debug file DEST.debug,
>   #   - no-debuglink : don't add the '.gnu_debuglink' section to
> -#                    DEST.stripped.
> +#                    DEST.
>   #
>   # Function returns zero on success.  Function will return non-zero failure code
>   # on some targets not supporting separate debug info (such as i386-msdos).
> @@ -8087,20 +8088,26 @@ proc gdb_gnu_strip_debug { dest args } {
>       # Unless the "no-debuglink" flag is passed, then link the two
>       # previous output files together, adding the .gnu_debuglink
>       # section to the stripped_file, containing a pointer to the
> -    # debug_file, save the new file in dest.
> +    # debug_file.
>       if {[lsearch -exact $args "no-debuglink"] == -1} {
> -	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${dest}" output]
> +	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${stripped_file}-tmp" output]
>   	verbose "result is $result"
>   	verbose "output is $output"
>   	if {$result == 1} {
>   	    return 1
>   	}
> +	file delete "${stripped_file}"
> +	file rename "${stripped_file}-tmp" "${stripped_file}"
>       }
>   
>       # Workaround PR binutils/10802:
>       # Preserve the 'x' bit also for PIEs (Position Independent Executables).
> -    set perm [file attributes ${stripped_file} -permissions]
> -    file attributes ${dest} -permissions $perm
> +    set perm [file attributes ${dest} -permissions]
> +    file attributes ${stripped_file} -permissions $perm
> +
> +    # Move the stripped_file back into dest.
> +    file delete ${dest}
> +    file rename ${stripped_file} ${dest}
>   
>       return 0
>   }


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot
  2024-05-20 14:14 ` [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot Andrew Burgess
@ 2024-05-23 10:17   ` Tom de Vries
  2024-05-26 23:04     ` Andrew Burgess
  0 siblings, 1 reply; 10+ messages in thread
From: Tom de Vries @ 2024-05-23 10:17 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 5/20/24 16:14, Andrew Burgess wrote:
> Add tests for looking up debug information within the sysroot via both
> build-id and gnu_debuglink.
> 
> I wanted to ensure that the gnu_debuglink test couldn't make use of
> build-ids, so I added the 'no-build-id' flag to gdb_compile.
> 
> As these tests rely on setting the sysroot, if I'm running a
> dynamically linked executable, GDB will try to find all shared
> libraries within the sysroot.  This would mean I'd have to figure out
> and copy all shared libraries the executable uses, certainly possible,
> but a bit of a pain.
> 
> So instead, I've just compiled the test executable as a static binary.
> Now there are no shared library dependencies.
> 

Make sense.

> I can now split the debug information out from the test binary, and
> place it within the sysroot.  When GDB is started and the executable
> loaded, we can check that GDB is finding the debug information within
> the sysroot.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30871
> ---
>   gdb/testsuite/gdb.base/sysroot-debug-lookup.c |  22 +++
>   .../gdb.base/sysroot-debug-lookup.exp         | 162 ++++++++++++++++++
>   gdb/testsuite/lib/gdb.exp                     |  13 ++
>   3 files changed, 197 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>   create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
> 
> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.c b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
> new file mode 100644
> index 00000000000..e67331d3150
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
> @@ -0,0 +1,22 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 1992-2024 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +main ()
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
> new file mode 100644
> index 00000000000..e0377df0fa7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
> @@ -0,0 +1,162 @@
> +# Copyright 2024 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test GDB's ability to find debug information by looking within the
> +# sysroot.
> +#
> +# We compile a static binary (to reduce what we need to copy into the
> +# sysroot), split the debug information from the binary, and setup a
> +# sysroot.
> +#
> +# The debug-file-directory is set to just '/debug', but we're
> +# expecting GDB to actually look in '$SYSROOT/debug'.
> +#
> +# There's a test for using .build-id based lookup, and a test for
> +# gnu_debuglink based lookup.
> +
> +require {!is_remote host}
> +
> +standard_testfile

Consider using "standard_testfile main.c" and dropping 
gdb/testsuite/gdb.base/sysroot-debug-lookup.c.

> +
> +# Create a copy of BINFILE, split out the debug information, and then
> +# setup a sysroot.  Hide (by moving) the actual debug information file
> +# and create a symlink to the hidden debug information from within the
> +# sysroot.
> +#
> +# Start GDB, set the sysroot, and then load the executable, ensure GDB
> +# finds the debug information, which must have happened by lookin in
> +# the sysroot.
> +proc_with_prefix lookup_via_build_id {} {
> +    set filename ${::binfile}_1
> +    if { [build_executable "build exec" ${filename} $::srcfile \
> +	      {additional_flags=-static debug build-id}] } {
> +	return
> +    }
> +
> +    # Split debug information into a .debug file, remove debug
> +    # information from FILENAME.  Don't add a .gnu_debuglink to
> +    # FILENAME, we rely on the build-id.
> +    if {[gdb_gnu_strip_debug $filename { no-debuglink }] != 0} {
> +	unsupported "cannot split debug information from executable"
> +	return
> +    }
> +
> +    set sysroot [standard_output_file "sysroot1"]
> +    set debug_dir "/debug"
> +
> +    set debug_symlink \
> +	$sysroot/$debug_dir/[build_id_debug_filename_get $filename]
> +
> +    set build_id_dir [file dirname $debug_symlink]
> +
> +    set debug_filename ${filename}_hidden_debug
> +
> +    remote_exec build "mkdir -p $build_id_dir"
> +    remote_exec build "mv $filename.debug $debug_filename"
> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
> +
> +    clean_restart
> +
> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
> +    gdb_test_no_output "set debug-file-directory $debug_dir"
> +
> +    gdb_file_cmd $filename
> +
> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
> +
> +    set re [string_to_regexp "Reading symbols from $debug_filename..."]
> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}

Consider adding a friendlier test name, likewise in the other proc.

Currently we get for a fail the not very meaningful:
...
FAIL: $exp: lookup_via_build_id: [regexp $re $::gdb_file_cmd_msg]
...

> +}
> +
> +# Create a copy of BINFILE, split out the debug information, and then
> +# setup a sysroot.  Hide (by moving) the actual debug information file
> +# and create a symlink to the hidden debug information from within the
> +# sysroot.
> +#
> +# Copy the executable into the sysroot and then start GDB, set the
> +# sysroot, and load the executable.  Check that GDB finds the debug
> +# information, which must have happened by lookin in the sysroot.
> +proc_with_prefix lookup_via_debuglink {} {
> +    set filename ${::binfile}_2
> +    if { [build_executable "build exec" ${filename} $::srcfile \
> +	      {additional_flags=-static debug no-build-id}] } {
> +	return
> +    }
> +
> +    # Split debug information into a .debug file, remove debug
> +    # information from FILENAME.
> +    if {[gdb_gnu_strip_debug $filename] != 0} {
> +	unsupported "cannot split debug information from executable"
> +	return
> +    }
> +
> +    # We're going to setup the sysroot like this:
> +    #
> +    # sysroot2/
> +    #    bin/
> +    #      $FILENAME
> +    #    debug/
> +    #      bin/
> +    #        $FILENAME.debug
> +    #
> +    # When looking up debug information via the debuglink, GDB will
> +    # only search in the sysroot if the original objfile was in the
> +    # sysroot.  And GDB will resolve symlinks, so if the objfile is
> +    # symlinked to outside the sysroot, GDB will not search in the
> +    # sysroot for the debug information.
> +    #
> +    # So we have to copy the executable into the sysroot.
> +    #
> +    # We are OK to symlink the debug information to a file outside the
> +    # sysroot though.
> +
> +    set sysroot [standard_output_file "sysroot2"]
> +
> +    foreach path { bin debug/bin } {
> +	remote_exec build "mkdir -p $sysroot/$path"
> +    }
> +
> +    # Copy the executable into the sysroot.
> +    set file_basename [file tail $filename]
> +    set exec_in_sysroot ${sysroot}/bin/${file_basename}
> +    remote_exec build "cp $filename $exec_in_sysroot"
> +
> +    # Rename the debug file outside of the sysroot, this should stop
> +    # GDB finding this file "by accident".
> +    set debug_filename ${filename}_hidden_debug
> +    remote_exec build "mv $filename.debug $debug_filename"
> +
> +    # Symlink the debug information into the sysroot.
> +    set debug_symlink \
> +	${sysroot}/debug/bin/${file_basename}.debug
> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
> +
> +    # Restart GDB and setup the sysroot and debug directory.
> +    clean_restart
> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
> +    gdb_test_no_output "set debug-file-directory /debug"
> +
> +    # Load the executable, we expect GDB to find the debug information
> +    # in the sysroot.
> +    gdb_file_cmd $exec_in_sysroot
> +
> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
> +
> +    set re [string_to_regexp "Reading symbols from $debug_symlink..."]
> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
> +}
> +
> +lookup_via_build_id
> +lookup_via_debuglink
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index a63c13f9cbc..a8e8b69d6ef 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5211,6 +5211,7 @@ proc quote_for_host { args } {
>   #     debug information
>   #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
>   #   - build-id: Ensure the final binary includes a build-id.
> +#   - no-build-id: Ensure the final binary does not include a build-id.
>   #   - column-info/no-column-info: Enable/Disable generation of column table
>   #     information.
>   #
> @@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} {
>   	lappend new_options "additional_flags=-Wl,--build-id"
>       }
>   
> +    # If the 'no-build-id' option is used then disable the build-id.
> +    if {[lsearch -exact $options no-build-id] > 0} {
> +	lappend new_options "additional_flags=-Wl,--build-id=none"
> +    }
> +
> +    # Sanity check.  If both 'build-id' and 'no-build-id' are used
> +    # then what is expected from us!
> +    if {[lsearch -exact $options build-id] > 0
> +	&& [lsearch -exact $options no-build-id] > 0} {
> +	error "cannot use build-id and no-build-id options"
> +    }
> +
>       # Treating .c input files as C++ is deprecated in Clang, so
>       # explicitly force C++ language.
>       if { !$getting_compiler_info


I've added the target: prefix for the set sysroot commands, and ran into 
these three fails:
...
FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_build_id: [regexp 
$re $::gdb_file_cmd_msg]
FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink: 
$::gdb_file_cmd_debug_info eq "debug"
FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink: [regexp 
$re $::gdb_file_cmd_msg]
...

The first one is just due to gdb mentioning an unresolved symlink.

But I think the next one reproduces PR30866.  So, IWBN to add this 
dimension to the test-case and add a kfail.

Anyway, as is, LGTM.

Approved-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot
  2024-05-23 10:17   ` Tom de Vries
@ 2024-05-26 23:04     ` Andrew Burgess
  2024-05-27  7:07       ` Tom de Vries
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2024-05-26 23:04 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

Tom de Vries <tdevries@suse.de> writes:

> On 5/20/24 16:14, Andrew Burgess wrote:
>> Add tests for looking up debug information within the sysroot via both
>> build-id and gnu_debuglink.
>> 
>> I wanted to ensure that the gnu_debuglink test couldn't make use of
>> build-ids, so I added the 'no-build-id' flag to gdb_compile.
>> 
>> As these tests rely on setting the sysroot, if I'm running a
>> dynamically linked executable, GDB will try to find all shared
>> libraries within the sysroot.  This would mean I'd have to figure out
>> and copy all shared libraries the executable uses, certainly possible,
>> but a bit of a pain.
>> 
>> So instead, I've just compiled the test executable as a static binary.
>> Now there are no shared library dependencies.
>> 
>
> Make sense.
>
>> I can now split the debug information out from the test binary, and
>> place it within the sysroot.  When GDB is started and the executable
>> loaded, we can check that GDB is finding the debug information within
>> the sysroot.
>> 
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30871
>> ---
>>   gdb/testsuite/gdb.base/sysroot-debug-lookup.c |  22 +++
>>   .../gdb.base/sysroot-debug-lookup.exp         | 162 ++++++++++++++++++
>>   gdb/testsuite/lib/gdb.exp                     |  13 ++
>>   3 files changed, 197 insertions(+)
>>   create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>>   create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>> 
>> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.c b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>> new file mode 100644
>> index 00000000000..e67331d3150
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>> @@ -0,0 +1,22 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 1992-2024 Free Software Foundation, Inc.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +int
>> +main ()
>> +{
>> +  return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>> new file mode 100644
>> index 00000000000..e0377df0fa7
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>> @@ -0,0 +1,162 @@
>> +# Copyright 2024 Free Software Foundation, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Test GDB's ability to find debug information by looking within the
>> +# sysroot.
>> +#
>> +# We compile a static binary (to reduce what we need to copy into the
>> +# sysroot), split the debug information from the binary, and setup a
>> +# sysroot.
>> +#
>> +# The debug-file-directory is set to just '/debug', but we're
>> +# expecting GDB to actually look in '$SYSROOT/debug'.
>> +#
>> +# There's a test for using .build-id based lookup, and a test for
>> +# gnu_debuglink based lookup.
>> +
>> +require {!is_remote host}
>> +
>> +standard_testfile
>
> Consider using "standard_testfile main.c" and dropping 
> gdb/testsuite/gdb.base/sysroot-debug-lookup.c.

Done.

>
>> +
>> +# Create a copy of BINFILE, split out the debug information, and then
>> +# setup a sysroot.  Hide (by moving) the actual debug information file
>> +# and create a symlink to the hidden debug information from within the
>> +# sysroot.
>> +#
>> +# Start GDB, set the sysroot, and then load the executable, ensure GDB
>> +# finds the debug information, which must have happened by lookin in
>> +# the sysroot.
>> +proc_with_prefix lookup_via_build_id {} {
>> +    set filename ${::binfile}_1
>> +    if { [build_executable "build exec" ${filename} $::srcfile \
>> +	      {additional_flags=-static debug build-id}] } {
>> +	return
>> +    }
>> +
>> +    # Split debug information into a .debug file, remove debug
>> +    # information from FILENAME.  Don't add a .gnu_debuglink to
>> +    # FILENAME, we rely on the build-id.
>> +    if {[gdb_gnu_strip_debug $filename { no-debuglink }] != 0} {
>> +	unsupported "cannot split debug information from executable"
>> +	return
>> +    }
>> +
>> +    set sysroot [standard_output_file "sysroot1"]
>> +    set debug_dir "/debug"
>> +
>> +    set debug_symlink \
>> +	$sysroot/$debug_dir/[build_id_debug_filename_get $filename]
>> +
>> +    set build_id_dir [file dirname $debug_symlink]
>> +
>> +    set debug_filename ${filename}_hidden_debug
>> +
>> +    remote_exec build "mkdir -p $build_id_dir"
>> +    remote_exec build "mv $filename.debug $debug_filename"
>> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
>> +
>> +    clean_restart
>> +
>> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
>> +    gdb_test_no_output "set debug-file-directory $debug_dir"
>> +
>> +    gdb_file_cmd $filename
>> +
>> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
>> +
>> +    set re [string_to_regexp "Reading symbols from $debug_filename..."]
>> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
>
> Consider adding a friendlier test name, likewise in the other proc.
>
> Currently we get for a fail the not very meaningful:
> ...
> FAIL: $exp: lookup_via_build_id: [regexp $re $::gdb_file_cmd_msg]

Done (I think).

> ...
>
>> +}
>> +
>> +# Create a copy of BINFILE, split out the debug information, and then
>> +# setup a sysroot.  Hide (by moving) the actual debug information file
>> +# and create a symlink to the hidden debug information from within the
>> +# sysroot.
>> +#
>> +# Copy the executable into the sysroot and then start GDB, set the
>> +# sysroot, and load the executable.  Check that GDB finds the debug
>> +# information, which must have happened by lookin in the sysroot.
>> +proc_with_prefix lookup_via_debuglink {} {
>> +    set filename ${::binfile}_2
>> +    if { [build_executable "build exec" ${filename} $::srcfile \
>> +	      {additional_flags=-static debug no-build-id}] } {
>> +	return
>> +    }
>> +
>> +    # Split debug information into a .debug file, remove debug
>> +    # information from FILENAME.
>> +    if {[gdb_gnu_strip_debug $filename] != 0} {
>> +	unsupported "cannot split debug information from executable"
>> +	return
>> +    }
>> +
>> +    # We're going to setup the sysroot like this:
>> +    #
>> +    # sysroot2/
>> +    #    bin/
>> +    #      $FILENAME
>> +    #    debug/
>> +    #      bin/
>> +    #        $FILENAME.debug
>> +    #
>> +    # When looking up debug information via the debuglink, GDB will
>> +    # only search in the sysroot if the original objfile was in the
>> +    # sysroot.  And GDB will resolve symlinks, so if the objfile is
>> +    # symlinked to outside the sysroot, GDB will not search in the
>> +    # sysroot for the debug information.
>> +    #
>> +    # So we have to copy the executable into the sysroot.
>> +    #
>> +    # We are OK to symlink the debug information to a file outside the
>> +    # sysroot though.
>> +
>> +    set sysroot [standard_output_file "sysroot2"]
>> +
>> +    foreach path { bin debug/bin } {
>> +	remote_exec build "mkdir -p $sysroot/$path"
>> +    }
>> +
>> +    # Copy the executable into the sysroot.
>> +    set file_basename [file tail $filename]
>> +    set exec_in_sysroot ${sysroot}/bin/${file_basename}
>> +    remote_exec build "cp $filename $exec_in_sysroot"
>> +
>> +    # Rename the debug file outside of the sysroot, this should stop
>> +    # GDB finding this file "by accident".
>> +    set debug_filename ${filename}_hidden_debug
>> +    remote_exec build "mv $filename.debug $debug_filename"
>> +
>> +    # Symlink the debug information into the sysroot.
>> +    set debug_symlink \
>> +	${sysroot}/debug/bin/${file_basename}.debug
>> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
>> +
>> +    # Restart GDB and setup the sysroot and debug directory.
>> +    clean_restart
>> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
>> +    gdb_test_no_output "set debug-file-directory /debug"
>> +
>> +    # Load the executable, we expect GDB to find the debug information
>> +    # in the sysroot.
>> +    gdb_file_cmd $exec_in_sysroot
>> +
>> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
>> +
>> +    set re [string_to_regexp "Reading symbols from $debug_symlink..."]
>> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
>> +}
>> +
>> +lookup_via_build_id
>> +lookup_via_debuglink
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index a63c13f9cbc..a8e8b69d6ef 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -5211,6 +5211,7 @@ proc quote_for_host { args } {
>>   #     debug information
>>   #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
>>   #   - build-id: Ensure the final binary includes a build-id.
>> +#   - no-build-id: Ensure the final binary does not include a build-id.
>>   #   - column-info/no-column-info: Enable/Disable generation of column table
>>   #     information.
>>   #
>> @@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} {
>>   	lappend new_options "additional_flags=-Wl,--build-id"
>>       }
>>   
>> +    # If the 'no-build-id' option is used then disable the build-id.
>> +    if {[lsearch -exact $options no-build-id] > 0} {
>> +	lappend new_options "additional_flags=-Wl,--build-id=none"
>> +    }
>> +
>> +    # Sanity check.  If both 'build-id' and 'no-build-id' are used
>> +    # then what is expected from us!
>> +    if {[lsearch -exact $options build-id] > 0
>> +	&& [lsearch -exact $options no-build-id] > 0} {
>> +	error "cannot use build-id and no-build-id options"
>> +    }
>> +
>>       # Treating .c input files as C++ is deprecated in Clang, so
>>       # explicitly force C++ language.
>>       if { !$getting_compiler_info
>
>
> I've added the target: prefix for the set sysroot commands, and ran into 
> these three fails:
> ...
> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_build_id: [regexp 
> $re $::gdb_file_cmd_msg]
> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink: 
> $::gdb_file_cmd_debug_info eq "debug"
> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink: [regexp 
> $re $::gdb_file_cmd_msg]
> ...
>
> The first one is just due to gdb mentioning an unresolved symlink.

Yeah.  The build-id lookup w.r.t target: sysroots seems pretty sane.
I've extended the test to cover the target: case.

>
> But I think the next one reproduces PR30866.  So, IWBN to add this 
> dimension to the test-case and add a kfail.

I'm not 100% certain if the issue here is 30866 or not.  I can't
reproduce 30866, I think the namespace mounting support in GDB is not
working for me, for some reason.  I'll need to dig into this at some
future time.

However, if it's not exactly 30866, then it's certainly something like
30866!  So, for now I've added an xfail, blamed it on 30866, and figure
that if/when 30866 is fixed, if this test doesn't start working, we can
open a new bug at that time.

You did already approve, but given the changes I wanted to let you take
another look before I merged this.

Let me know if you have any further thoughts.

Thanks,
Andrew

---

commit bbdb9e9a479a6a2120a7abe1ce07d033b6e52b0c
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat May 18 10:50:23 2024 +0100

    gdb/testsuite: tests for debug lookup within the sysroot
    
    Add tests for looking up debug information within the sysroot via both
    build-id and gnu_debuglink.
    
    I wanted to ensure that the gnu_debuglink test couldn't make use of
    build-ids, so I added the 'no-build-id' flag to gdb_compile.
    
    As these tests rely on setting the sysroot, if I'm running a
    dynamically linked executable, GDB will try to find all shared
    libraries within the sysroot.  This would mean I'd have to figure out
    and copy all shared libraries the executable uses, certainly possible,
    but a bit of a pain.
    
    So instead, I've just compiled the test executable as a static binary.
    Now there are no shared library dependencies.
    
    I can now split the debug information out from the test binary, and
    place it within the sysroot.  When GDB is started and the executable
    loaded, we can check that GDB is finding the debug information within
    the sysroot.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30871
    
    Approved-By: Tom de Vries <tdevries@suse.de>

diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
new file mode 100644
index 00000000000..49e5b54fbd3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
@@ -0,0 +1,202 @@
+# Copyright 2024 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB's ability to find debug information by looking within the
+# sysroot.
+#
+# We compile a static binary (to reduce what we need to copy into the
+# sysroot), split the debug information from the binary, and setup a
+# sysroot.
+#
+# The debug-file-directory is set to just '/debug', but we're
+# expecting GDB to actually look in '$SYSROOT/debug'.
+#
+# There's a test for using .build-id based lookup, and a test for
+# gnu_debuglink based lookup.
+
+require {!is_remote host}
+
+standard_testfile main.c
+
+# Create a copy of BINFILE, split out the debug information, and then
+# setup a sysroot.  Hide (by moving) the actual debug information file
+# and create a symlink to the hidden debug information from within the
+# sysroot.
+#
+# Start GDB, set the sysroot, and then load the executable, ensure GDB
+# finds the debug information, which must have happened by lookin in
+# the sysroot.
+proc_with_prefix lookup_via_build_id {} {
+    set filename ${::binfile}_1
+    if { [build_executable "build exec" ${filename} $::srcfile \
+	      {additional_flags=-static debug build-id}] } {
+	return
+    }
+
+    # Split debug information into a .debug file, remove debug
+    # information from FILENAME.  Don't add a .gnu_debuglink to
+    # FILENAME, we rely on the build-id.
+    if {[gdb_gnu_strip_debug $filename { no-debuglink }] != 0} {
+	unsupported "cannot split debug information from executable"
+	return
+    }
+
+    set sysroot [standard_output_file "sysroot1"]
+    set debug_dir "/debug"
+
+    set debug_symlink \
+	${sysroot}${debug_dir}/[build_id_debug_filename_get $filename]
+
+    set build_id_dir [file dirname $debug_symlink]
+
+    set debug_filename ${filename}_hidden_debug
+
+    remote_exec build "mkdir -p $build_id_dir"
+    remote_exec build "mv $filename.debug $debug_filename"
+    remote_exec build "ln -sf $debug_filename $debug_symlink"
+
+    foreach_with_prefix sysroot_prefix { "" "target:" } {
+	clean_restart
+
+	gdb_test_no_output "set sysroot ${sysroot_prefix}$sysroot" "set sysroot"
+	gdb_test_no_output "set debug-file-directory $debug_dir"
+
+	gdb_file_cmd $filename
+
+	gdb_assert { $::gdb_file_cmd_debug_info eq "debug" } \
+	    "ensure debug information was found"
+
+	if { $sysroot_prefix eq "" } {
+	    set lookup_filename $debug_filename
+	} else {
+	    # Only when using the extended-remote board will we have
+	    # started a remote target by this point.  In this case GDB
+	    # will see the 'target:' prefix as remote, and so the
+	    # reported filename will include the 'target:' prefix.
+	    #
+	    # In all other cases we will still be using the default,
+	    # initial target, in which case GDB considers the
+	    # 'target:' prefix to indicate the local filesystem.
+	    if {[target_info gdb_protocol] == "extended-remote"} {
+		set lookup_filename $sysroot_prefix$debug_symlink
+	    } else {
+		set lookup_filename $debug_symlink
+	    }
+	}
+	set re [string_to_regexp "Reading symbols from $lookup_filename..."]
+	gdb_assert {[regexp $re $::gdb_file_cmd_msg]} \
+	    "debug symbols read from correct file"
+    }
+}
+
+# Create a copy of BINFILE, split out the debug information, and then
+# setup a sysroot.  Hide (by moving) the actual debug information file
+# and create a symlink to the hidden debug information from within the
+# sysroot.
+#
+# Copy the executable into the sysroot and then start GDB, set the
+# sysroot, and load the executable.  Check that GDB finds the debug
+# information, which must have happened by lookin in the sysroot.
+proc_with_prefix lookup_via_debuglink {} {
+    set filename ${::binfile}_2
+    if { [build_executable "build exec" ${filename} $::srcfile \
+	      {additional_flags=-static debug no-build-id}] } {
+	return
+    }
+
+    # Split debug information into a .debug file, remove debug
+    # information from FILENAME.
+    if {[gdb_gnu_strip_debug $filename] != 0} {
+	unsupported "cannot split debug information from executable"
+	return
+    }
+
+    # We're going to setup the sysroot like this:
+    #
+    # sysroot2/
+    #    bin/
+    #      $FILENAME
+    #    debug/
+    #      bin/
+    #        $FILENAME.debug
+    #
+    # When looking up debug information via the debuglink, GDB will
+    # only search in the sysroot if the original objfile was in the
+    # sysroot.  And GDB will resolve symlinks, so if the objfile is
+    # symlinked to outside the sysroot, GDB will not search in the
+    # sysroot for the debug information.
+    #
+    # So we have to copy the executable into the sysroot.
+    #
+    # We are OK to symlink the debug information to a file outside the
+    # sysroot though.
+
+    set sysroot [standard_output_file "sysroot2"]
+
+    foreach path { bin debug/bin } {
+	remote_exec build "mkdir -p $sysroot/$path"
+    }
+
+    # Copy the executable into the sysroot.
+    set file_basename [file tail $filename]
+    set exec_in_sysroot ${sysroot}/bin/${file_basename}
+    remote_exec build "cp $filename $exec_in_sysroot"
+
+    # Rename the debug file outside of the sysroot, this should stop
+    # GDB finding this file "by accident".
+    set debug_filename ${filename}_hidden_debug
+    remote_exec build "mv $filename.debug $debug_filename"
+
+    # Symlink the debug information into the sysroot.
+    set debug_symlink \
+	${sysroot}/debug/bin/${file_basename}.debug
+    remote_exec build "ln -sf $debug_filename $debug_symlink"
+
+    foreach_with_prefix sysroot_prefix { "" "target:" } {
+	# Restart GDB and setup the sysroot and debug directory.
+	clean_restart
+	gdb_test_no_output "set sysroot ${sysroot_prefix}$sysroot" "set sysroot"
+	gdb_test_no_output "set debug-file-directory /debug"
+
+	# Load the executable, we expect GDB to find the debug information
+	# in the sysroot.
+	gdb_file_cmd ${sysroot_prefix}$exec_in_sysroot
+
+	# Giving a sysroot a 'target:' prefix doesn't seem to work as
+	# might be expected for debug-link based lookups.  For this
+	# style of lookup GDB only seems to care if the original file
+	# itself had a 'target:' prefix.  The 'target:' prefix on the
+	# sysroot just seems to cause GDB to bail on looking up via
+	# the 'target:' prefixed sysroot.
+	#
+	# Bug PR gdb/30866 seems to be the (or a) relevant bug for
+	# this problem.
+	if { $sysroot_prefix ne "" } {
+	    setup_xfail "*-*-*" 30866
+	}
+	gdb_assert { $::gdb_file_cmd_debug_info eq "debug" } \
+	    "ensure debug information was found"
+
+	if { $sysroot_prefix ne "" } {
+	    setup_xfail "*-*-*" 30866
+	}
+	set re [string_to_regexp "Reading symbols from ${sysroot_prefix}$debug_symlink..."]
+	gdb_assert {[regexp $re $::gdb_file_cmd_msg]} \
+	    "debug symbols read from correct file"
+    }
+}
+
+lookup_via_build_id
+lookup_via_debuglink
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a63c13f9cbc..a8e8b69d6ef 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5211,6 +5211,7 @@ proc quote_for_host { args } {
 #     debug information
 #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
 #   - build-id: Ensure the final binary includes a build-id.
+#   - no-build-id: Ensure the final binary does not include a build-id.
 #   - column-info/no-column-info: Enable/Disable generation of column table
 #     information.
 #
@@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} {
 	lappend new_options "additional_flags=-Wl,--build-id"
     }
 
+    # If the 'no-build-id' option is used then disable the build-id.
+    if {[lsearch -exact $options no-build-id] > 0} {
+	lappend new_options "additional_flags=-Wl,--build-id=none"
+    }
+
+    # Sanity check.  If both 'build-id' and 'no-build-id' are used
+    # then what is expected from us!
+    if {[lsearch -exact $options build-id] > 0
+	&& [lsearch -exact $options no-build-id] > 0} {
+	error "cannot use build-id and no-build-id options"
+    }
+
     # Treating .c input files as C++ is deprecated in Clang, so
     # explicitly force C++ language.
     if { !$getting_compiler_info


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent
  2024-05-23  9:13   ` Tom de Vries
@ 2024-05-26 23:08     ` Andrew Burgess
  2024-05-27  7:15       ` Tom de Vries
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2024-05-26 23:08 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

Tom de Vries <tdevries@suse.de> writes:

> On 5/20/24 16:14, Andrew Burgess wrote:
>> While writing a test I realised that the default behaviour of
>> gdb_gnu_strip_debug doesn't match its comment.
>> 
>> The comment says that the function takes a FILENAME, and splits the
>> file into FILENAME.stripped and FILENAME.debug, leaving FILENAME
>> unchanged.  The comment says that a .gnu_debuglink will be added to
>> FILENAME.stripped.
>> 
>> However, this is not true, FILENAME.stripped is created, with no debug
>> information.  FILENAME.debug is created containing the debug
>> information.
>> 
>> But, when adding the .gnu_debuglink we take FILENAME.stripped as the
>> input, and then overwrite FILENAME with the output.  As a result,
>> FILENAME.stripped does not include a .gnu_debuglink, while FILENAME
>> contains the .gnu_debuglink and no debug information!
>> 
>> The users of gdb_gnu_strip_debug can be split into two groups, those
>> who are using the .gnu_debuglink, these tests are all written assuming
>> that FILENAME is updated.
>> 
>> Then there are some tests that rely on gdb_gnu_strip_debug's ability
>> to split out the debug information,
>
> I got confused a bit here.  Doesn't the first group also rely on this?
>
>> these tests are then going to do a
>> lookup based on the build-id.  These tests use the FILENAME.stripped
>> output file.
>> 
>> This all seems too confused to me.
>> 
>> As most used of gdb_gnu_strip_debug assume that FILENAME is updated, I
>
> used -> uses
>
>> propose that we just make that the actual, advertised behaviour of
>> this proc.
>> 
>
> Make sense.
>
> [ FWIW, the overwriting behaviour is not great for the case where the 
> exec is used in the initial part of a test-case, and then modified by 
> gdb_gnu_strip_debug (I don't know whether that occurs though).  When 
> trying to reproduce the initial part of the tests using say -x gdb.in.1 
> the behaviour may be different because the exec has changed in the mean 
> time.  So going from that point of view we could have gone for the 
> syntax of gdb_gnu_strip_debug returning the filename of the stripped 
> exec, if successful.  Come to think of it, doing so might also allow us 
> to name the stripped exec differently, say foo.stripped and 
> foo.stripped-with-link, which will make it more clear in gdb.log what 
> exec is used. ]

I see what you're saying here.  But I'm really not super keen to start
rewriting all the tests that use gdb_gnu_strip_debug right now :-/ As
most of the tests that call this proc are already written assuming the
overwrite behaviour.

Unless you strongly object then I'm planning to just go with this as it
is right now.  But if it really bothers you I can tackle fixing all the
tests.  Let me know.

Anyway, updated patch with the minor type fixes, and slightly clearer
commit message is below, just in case you wanted another look.

Thanks,
Andrew

---

commit b888aea514e5eb968ef0e2e30f8042848f88fc90
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat May 18 11:05:49 2024 +0100

    gdb/testsuite: make gdb_gnu_strip_debug consistent
    
    While writing a test I realised that the default behaviour of
    gdb_gnu_strip_debug doesn't match its comment.
    
    The comment says that the function takes a FILENAME, and splits the
    file into FILENAME.stripped and FILENAME.debug, leaving FILENAME
    unchanged.  The comment says that a .gnu_debuglink will be added to
    FILENAME.stripped.
    
    However, this is not true, FILENAME.stripped is created, with no debug
    information.  FILENAME.debug is created containing the debug
    information.
    
    But, when adding the .gnu_debuglink we take FILENAME.stripped as the
    input, and then overwrite FILENAME with the output.  As a result,
    FILENAME.stripped does not include a .gnu_debuglink, while FILENAME
    contains the .gnu_debuglink and no debug information!
    
    The users of gdb_gnu_strip_debug can be split into two groups, those
    who are using the .gnu_debuglink, these tests are all written assuming
    that FILENAME is updated.
    
    Then there are some tests that only rely on gdb_gnu_strip_debug's
    ability to split out the debug information, these tests are then going
    to do a lookup based on the build-id, these tests don't require the
    .gnu_debuglink.  These tests use the FILENAME.stripped output file.
    
    This all seems too confused to me.
    
    As most uses of gdb_gnu_strip_debug assume that FILENAME is updated, I
    propose that we just make that the actual, advertised behaviour of
    this proc.
    
    So now, gdb_gnu_strip_debug will take FILENAME, and will split the
    debug information out into FILENAME.debug.  The debug information will
    then be stripped from FILENAME, and by default a .gnu_debuglink will
    be added to FILENAME pointing to FILENAME.debug.
    
    I've updated the two tests that actually relied on FILENAME.stripped
    to instead just use FILENAME.
    
    One of the tests was doing a build-id based lookup, but was still
    allowing the .gnu_debuglink to be added to FILENAME, I've updated this
    test to pass the no-debuglink flag to gdb_gnu_strip_debug, which stops
    the .gnu_debuglink from being added.
    
    All of the tests that call gdb_gnu_strip_debug still pass for me.
    
    Acked-By: Tom de Vries <tdevries@suse.de>

diff --git a/gdb/testsuite/gdb.base/corefile-buildid.exp b/gdb/testsuite/gdb.base/corefile-buildid.exp
index 130198611ec..e1b9804d891 100644
--- a/gdb/testsuite/gdb.base/corefile-buildid.exp
+++ b/gdb/testsuite/gdb.base/corefile-buildid.exp
@@ -172,11 +172,9 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
 	"mkdir -p [file join $debugdir [file dirname $buildid]]"
 
     set files_list {}
+    lappend files_list $binfile $buildid
     if {$sepdebug} {
-	lappend files_list "$binfile.stripped" $buildid
 	lappend files_list "$binfile.debug" "$buildid.debug"
-    } else {
-	lappend files_list $binfile $buildid
     }
     if {$shared} {
 	global sharedir
@@ -200,12 +198,7 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
     gdb_test "core-file $corefile" "Program terminated with .*" \
 	"load core file"
     if {$symlink} {
-	if {$sepdebug} {
-	    set expected_file [file join $builddir \
-				   [file tail "$binfile.stripped"]]
-	} else {
-	    set expected_file [file join $builddir [file tail $binfile]]
-	}
+	set expected_file [file join $builddir [file tail $binfile]]
     } else {
 	set expected_file $buildid
     }
@@ -245,15 +238,12 @@ proc do_corefile_buildid_tests {args} {
 
     if {$sepdebug} {
 	# Strip debuginfo into its own file.
-	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run]] \
-		!= 0} {
+	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run] \
+		 no-debuglink] != 0} {
 	    untested "could not strip executable  for [join $suffix \ ]"
 	    return
 	}
 
-	# Run the stripped program instead of the original.
-	set program_to_run [file join $builddir \
-				[file tail "$binfile.stripped"]]
 	lappend suffix "sepdebug"
     }
 
diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
index b86622543ef..aa1c263e800 100644
--- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
+++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
@@ -30,14 +30,11 @@ if {[build_executable "build executable" ${testfile} ${srcfile} \
     return -1
 }
 
-# Split BINFILE into BINFILE.stripped and BINFILE.debug, the first is
-# the executable with the debug information removed, and the second is
-# the debug information.
+# Split debug information from BINFILE into BINFILE.debug.
 #
-# However, by passing the "no-debuglink" flag we prevent this proc
-# from adding a .gnu_debuglink section to the executable.  Any lookup
-# of the debug information by GDB will need to be done based on the
-# build-id.
+# By passing the "no-debuglink" flag we prevent this proc from adding
+# a .gnu_debuglink section to BINFILE.  Any lookup of the debug
+# information by GDB will need to be done based on the build-id.
 if {[gdb_gnu_strip_debug $binfile no-debuglink]} {
     unsupported "cannot produce separate debug info files"
     return -1
@@ -59,12 +56,6 @@ set debuginfod_debugdir [standard_output_file "debug"]
 remote_exec build "mkdir $debuginfod_debugdir"
 remote_exec build "mv $debugfile $debuginfod_debugdir"
 
-# This is BINFILE with the debug information removed.  We are going to
-# place this in the BUILD_ID_DEBUG_FILE location, this would usually
-# represent a mistake by the user, and will trigger a warning from
-# GDB, this is the warning we are checking for.
-set stripped_binfile [standard_output_file "${binfile}.stripped"]
-
 # Create the .build-id/PREFIX directory name from
 # .build-id/PREFIX/SUFFIX.debug filename.
 set debugdir [file dirname ${build_id_debug_file}]
@@ -76,7 +67,7 @@ remote_exec build "mkdir -p $debugdir"
 # information, which will point back at this file, which also doesn't
 # have debug information, which could cause a loop.  But GDB will spot
 # this and give a warning.
-remote_exec build "mv ${stripped_binfile} ${build_id_debug_file}"
+remote_exec build "mv ${binfile} ${build_id_debug_file}"
 
 # Now start GDB.
 clean_restart
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0d78691c381..a63c13f9cbc 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8017,10 +8017,11 @@ proc build_id_debug_filename_get { filename } {
 }
 
 # DEST should be a file compiled with debug information.  This proc
-# creates two new files DEST.debug which contains the debug
-# information extracted from DEST, and DEST.stripped, which is a copy
-# of DEST with the debug information removed.  A '.gnu_debuglink'
-# section will be added to DEST.stripped that points to DEST.debug.
+# creates DEST.debug which contains the debug information extracted
+# from DEST, and DEST is updated with the debug information removed.
+#
+# By default a '.gnu_debuglink' section will be added to DEST that
+# points to DEST.debug.
 #
 # If ARGS is passed, it is a list of optional flags.  The currently
 # supported flags are:
@@ -8028,7 +8029,7 @@ proc build_id_debug_filename_get { filename } {
 #   - no-main : remove the symbol entry for main from the separate
 #               debug file DEST.debug,
 #   - no-debuglink : don't add the '.gnu_debuglink' section to
-#                    DEST.stripped.
+#                    DEST.
 #
 # Function returns zero on success.  Function will return non-zero failure code
 # on some targets not supporting separate debug info (such as i386-msdos).
@@ -8087,20 +8088,26 @@ proc gdb_gnu_strip_debug { dest args } {
     # Unless the "no-debuglink" flag is passed, then link the two
     # previous output files together, adding the .gnu_debuglink
     # section to the stripped_file, containing a pointer to the
-    # debug_file, save the new file in dest.
+    # debug_file.
     if {[lsearch -exact $args "no-debuglink"] == -1} {
-	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${dest}" output]
+	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${stripped_file}-tmp" output]
 	verbose "result is $result"
 	verbose "output is $output"
 	if {$result == 1} {
 	    return 1
 	}
+	file delete "${stripped_file}"
+	file rename "${stripped_file}-tmp" "${stripped_file}"
     }
 
     # Workaround PR binutils/10802:
     # Preserve the 'x' bit also for PIEs (Position Independent Executables).
-    set perm [file attributes ${stripped_file} -permissions]
-    file attributes ${dest} -permissions $perm
+    set perm [file attributes ${dest} -permissions]
+    file attributes ${stripped_file} -permissions $perm
+
+    # Move the stripped_file back into dest.
+    file delete ${dest}
+    file rename ${stripped_file} ${dest}
 
     return 0
 }


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot
  2024-05-26 23:04     ` Andrew Burgess
@ 2024-05-27  7:07       ` Tom de Vries
  2024-06-04 12:40         ` Andrew Burgess
  0 siblings, 1 reply; 10+ messages in thread
From: Tom de Vries @ 2024-05-27  7:07 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 5/27/24 01:04, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
> 
>> On 5/20/24 16:14, Andrew Burgess wrote:
>>> Add tests for looking up debug information within the sysroot via both
>>> build-id and gnu_debuglink.
>>>
>>> I wanted to ensure that the gnu_debuglink test couldn't make use of
>>> build-ids, so I added the 'no-build-id' flag to gdb_compile.
>>>
>>> As these tests rely on setting the sysroot, if I'm running a
>>> dynamically linked executable, GDB will try to find all shared
>>> libraries within the sysroot.  This would mean I'd have to figure out
>>> and copy all shared libraries the executable uses, certainly possible,
>>> but a bit of a pain.
>>>
>>> So instead, I've just compiled the test executable as a static binary.
>>> Now there are no shared library dependencies.
>>>
>>
>> Make sense.
>>
>>> I can now split the debug information out from the test binary, and
>>> place it within the sysroot.  When GDB is started and the executable
>>> loaded, we can check that GDB is finding the debug information within
>>> the sysroot.
>>>
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30871
>>> ---
>>>    gdb/testsuite/gdb.base/sysroot-debug-lookup.c |  22 +++
>>>    .../gdb.base/sysroot-debug-lookup.exp         | 162 ++++++++++++++++++
>>>    gdb/testsuite/lib/gdb.exp                     |  13 ++
>>>    3 files changed, 197 insertions(+)
>>>    create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>>>    create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>>>
>>> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.c b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>>> new file mode 100644
>>> index 00000000000..e67331d3150
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>>> @@ -0,0 +1,22 @@
>>> +/* This testcase is part of GDB, the GNU debugger.
>>> +
>>> +   Copyright 1992-2024 Free Software Foundation, Inc.
>>> +
>>> +   This program is free software; you can redistribute it and/or modify
>>> +   it under the terms of the GNU General Public License as published by
>>> +   the Free Software Foundation; either version 3 of the License, or
>>> +   (at your option) any later version.
>>> +
>>> +   This program is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +   GNU General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU General Public License
>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>> +
>>> +int
>>> +main ()
>>> +{
>>> +  return 0;
>>> +}
>>> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>>> new file mode 100644
>>> index 00000000000..e0377df0fa7
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>>> @@ -0,0 +1,162 @@
>>> +# Copyright 2024 Free Software Foundation, Inc.
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 3 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> +# Test GDB's ability to find debug information by looking within the
>>> +# sysroot.
>>> +#
>>> +# We compile a static binary (to reduce what we need to copy into the
>>> +# sysroot), split the debug information from the binary, and setup a
>>> +# sysroot.
>>> +#
>>> +# The debug-file-directory is set to just '/debug', but we're
>>> +# expecting GDB to actually look in '$SYSROOT/debug'.
>>> +#
>>> +# There's a test for using .build-id based lookup, and a test for
>>> +# gnu_debuglink based lookup.
>>> +
>>> +require {!is_remote host}
>>> +
>>> +standard_testfile
>>
>> Consider using "standard_testfile main.c" and dropping
>> gdb/testsuite/gdb.base/sysroot-debug-lookup.c.
> 
> Done.
> 
>>
>>> +
>>> +# Create a copy of BINFILE, split out the debug information, and then
>>> +# setup a sysroot.  Hide (by moving) the actual debug information file
>>> +# and create a symlink to the hidden debug information from within the
>>> +# sysroot.
>>> +#
>>> +# Start GDB, set the sysroot, and then load the executable, ensure GDB
>>> +# finds the debug information, which must have happened by lookin in
>>> +# the sysroot.
>>> +proc_with_prefix lookup_via_build_id {} {
>>> +    set filename ${::binfile}_1
>>> +    if { [build_executable "build exec" ${filename} $::srcfile \
>>> +	      {additional_flags=-static debug build-id}] } {
>>> +	return
>>> +    }
>>> +
>>> +    # Split debug information into a .debug file, remove debug
>>> +    # information from FILENAME.  Don't add a .gnu_debuglink to
>>> +    # FILENAME, we rely on the build-id.
>>> +    if {[gdb_gnu_strip_debug $filename { no-debuglink }] != 0} {
>>> +	unsupported "cannot split debug information from executable"
>>> +	return
>>> +    }
>>> +
>>> +    set sysroot [standard_output_file "sysroot1"]
>>> +    set debug_dir "/debug"
>>> +
>>> +    set debug_symlink \
>>> +	$sysroot/$debug_dir/[build_id_debug_filename_get $filename]
>>> +
>>> +    set build_id_dir [file dirname $debug_symlink]
>>> +
>>> +    set debug_filename ${filename}_hidden_debug
>>> +
>>> +    remote_exec build "mkdir -p $build_id_dir"
>>> +    remote_exec build "mv $filename.debug $debug_filename"
>>> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
>>> +
>>> +    clean_restart
>>> +
>>> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
>>> +    gdb_test_no_output "set debug-file-directory $debug_dir"
>>> +
>>> +    gdb_file_cmd $filename
>>> +
>>> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
>>> +
>>> +    set re [string_to_regexp "Reading symbols from $debug_filename..."]
>>> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
>>
>> Consider adding a friendlier test name, likewise in the other proc.
>>
>> Currently we get for a fail the not very meaningful:
>> ...
>> FAIL: $exp: lookup_via_build_id: [regexp $re $::gdb_file_cmd_msg]
> 
> Done (I think).
> 
>> ...
>>
>>> +}
>>> +
>>> +# Create a copy of BINFILE, split out the debug information, and then
>>> +# setup a sysroot.  Hide (by moving) the actual debug information file
>>> +# and create a symlink to the hidden debug information from within the
>>> +# sysroot.
>>> +#
>>> +# Copy the executable into the sysroot and then start GDB, set the
>>> +# sysroot, and load the executable.  Check that GDB finds the debug
>>> +# information, which must have happened by lookin in the sysroot.
>>> +proc_with_prefix lookup_via_debuglink {} {
>>> +    set filename ${::binfile}_2
>>> +    if { [build_executable "build exec" ${filename} $::srcfile \
>>> +	      {additional_flags=-static debug no-build-id}] } {
>>> +	return
>>> +    }
>>> +
>>> +    # Split debug information into a .debug file, remove debug
>>> +    # information from FILENAME.
>>> +    if {[gdb_gnu_strip_debug $filename] != 0} {
>>> +	unsupported "cannot split debug information from executable"
>>> +	return
>>> +    }
>>> +
>>> +    # We're going to setup the sysroot like this:
>>> +    #
>>> +    # sysroot2/
>>> +    #    bin/
>>> +    #      $FILENAME
>>> +    #    debug/
>>> +    #      bin/
>>> +    #        $FILENAME.debug
>>> +    #
>>> +    # When looking up debug information via the debuglink, GDB will
>>> +    # only search in the sysroot if the original objfile was in the
>>> +    # sysroot.  And GDB will resolve symlinks, so if the objfile is
>>> +    # symlinked to outside the sysroot, GDB will not search in the
>>> +    # sysroot for the debug information.
>>> +    #
>>> +    # So we have to copy the executable into the sysroot.
>>> +    #
>>> +    # We are OK to symlink the debug information to a file outside the
>>> +    # sysroot though.
>>> +
>>> +    set sysroot [standard_output_file "sysroot2"]
>>> +
>>> +    foreach path { bin debug/bin } {
>>> +	remote_exec build "mkdir -p $sysroot/$path"
>>> +    }
>>> +
>>> +    # Copy the executable into the sysroot.
>>> +    set file_basename [file tail $filename]
>>> +    set exec_in_sysroot ${sysroot}/bin/${file_basename}
>>> +    remote_exec build "cp $filename $exec_in_sysroot"
>>> +
>>> +    # Rename the debug file outside of the sysroot, this should stop
>>> +    # GDB finding this file "by accident".
>>> +    set debug_filename ${filename}_hidden_debug
>>> +    remote_exec build "mv $filename.debug $debug_filename"
>>> +
>>> +    # Symlink the debug information into the sysroot.
>>> +    set debug_symlink \
>>> +	${sysroot}/debug/bin/${file_basename}.debug
>>> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
>>> +
>>> +    # Restart GDB and setup the sysroot and debug directory.
>>> +    clean_restart
>>> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
>>> +    gdb_test_no_output "set debug-file-directory /debug"
>>> +
>>> +    # Load the executable, we expect GDB to find the debug information
>>> +    # in the sysroot.
>>> +    gdb_file_cmd $exec_in_sysroot
>>> +
>>> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
>>> +
>>> +    set re [string_to_regexp "Reading symbols from $debug_symlink..."]
>>> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
>>> +}
>>> +
>>> +lookup_via_build_id
>>> +lookup_via_debuglink
>>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>>> index a63c13f9cbc..a8e8b69d6ef 100644
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -5211,6 +5211,7 @@ proc quote_for_host { args } {
>>>    #     debug information
>>>    #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
>>>    #   - build-id: Ensure the final binary includes a build-id.
>>> +#   - no-build-id: Ensure the final binary does not include a build-id.
>>>    #   - column-info/no-column-info: Enable/Disable generation of column table
>>>    #     information.
>>>    #
>>> @@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} {
>>>    	lappend new_options "additional_flags=-Wl,--build-id"
>>>        }
>>>    
>>> +    # If the 'no-build-id' option is used then disable the build-id.
>>> +    if {[lsearch -exact $options no-build-id] > 0} {
>>> +	lappend new_options "additional_flags=-Wl,--build-id=none"
>>> +    }
>>> +
>>> +    # Sanity check.  If both 'build-id' and 'no-build-id' are used
>>> +    # then what is expected from us!
>>> +    if {[lsearch -exact $options build-id] > 0
>>> +	&& [lsearch -exact $options no-build-id] > 0} {
>>> +	error "cannot use build-id and no-build-id options"
>>> +    }
>>> +
>>>        # Treating .c input files as C++ is deprecated in Clang, so
>>>        # explicitly force C++ language.
>>>        if { !$getting_compiler_info
>>
>>
>> I've added the target: prefix for the set sysroot commands, and ran into
>> these three fails:
>> ...
>> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_build_id: [regexp
>> $re $::gdb_file_cmd_msg]
>> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink:
>> $::gdb_file_cmd_debug_info eq "debug"
>> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink: [regexp
>> $re $::gdb_file_cmd_msg]
>> ...
>>
>> The first one is just due to gdb mentioning an unresolved symlink.
> 
> Yeah.  The build-id lookup w.r.t target: sysroots seems pretty sane.
> I've extended the test to cover the target: case.
> 
>>
>> But I think the next one reproduces PR30866.  So, IWBN to add this
>> dimension to the test-case and add a kfail.
> 
> I'm not 100% certain if the issue here is 30866 or not.  I can't
> reproduce 30866, I think the namespace mounting support in GDB is not
> working for me, for some reason.  I'll need to dig into this at some
> future time.
> 
> However, if it's not exactly 30866, then it's certainly something like
> 30866!  So, for now I've added an xfail, blamed it on 30866, and figure
> that if/when 30866 is fixed, if this test doesn't start working, we can
> open a new bug at that time.
> 
> You did already approve, but given the changes I wanted to let you take
> another look before I merged this.
> 
> Let me know if you have any further thoughts.
> 

Interesting.  Looking into it a bit further, I think it is indeed a 
different issue, thanks for pointing that out.  I filed a PR ( 
https://sourceware.org/bugzilla/show_bug.cgi?id=31804 ) for this, and 
investigated a bit and posted a demonstrator patch.

Anyway, please refer to this PR, and do so using a kfail, since AFAIU 
this a problem in gdb, not external to gdb.

Otherwise, LGTM.

Thanks,
- Tom

> Thanks,
> Andrew
> 
> ---
> 
> commit bbdb9e9a479a6a2120a7abe1ce07d033b6e52b0c
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Sat May 18 10:50:23 2024 +0100
> 
>      gdb/testsuite: tests for debug lookup within the sysroot
>      
>      Add tests for looking up debug information within the sysroot via both
>      build-id and gnu_debuglink.
>      
>      I wanted to ensure that the gnu_debuglink test couldn't make use of
>      build-ids, so I added the 'no-build-id' flag to gdb_compile.
>      
>      As these tests rely on setting the sysroot, if I'm running a
>      dynamically linked executable, GDB will try to find all shared
>      libraries within the sysroot.  This would mean I'd have to figure out
>      and copy all shared libraries the executable uses, certainly possible,
>      but a bit of a pain.
>      
>      So instead, I've just compiled the test executable as a static binary.
>      Now there are no shared library dependencies.
>      
>      I can now split the debug information out from the test binary, and
>      place it within the sysroot.  When GDB is started and the executable
>      loaded, we can check that GDB is finding the debug information within
>      the sysroot.
>      
>      Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30871
>      
>      Approved-By: Tom de Vries <tdevries@suse.de>
> 
> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
> new file mode 100644
> index 00000000000..49e5b54fbd3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
> @@ -0,0 +1,202 @@
> +# Copyright 2024 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test GDB's ability to find debug information by looking within the
> +# sysroot.
> +#
> +# We compile a static binary (to reduce what we need to copy into the
> +# sysroot), split the debug information from the binary, and setup a
> +# sysroot.
> +#
> +# The debug-file-directory is set to just '/debug', but we're
> +# expecting GDB to actually look in '$SYSROOT/debug'.
> +#
> +# There's a test for using .build-id based lookup, and a test for
> +# gnu_debuglink based lookup.
> +
> +require {!is_remote host}
> +
> +standard_testfile main.c
> +
> +# Create a copy of BINFILE, split out the debug information, and then
> +# setup a sysroot.  Hide (by moving) the actual debug information file
> +# and create a symlink to the hidden debug information from within the
> +# sysroot.
> +#
> +# Start GDB, set the sysroot, and then load the executable, ensure GDB
> +# finds the debug information, which must have happened by lookin in
> +# the sysroot.
> +proc_with_prefix lookup_via_build_id {} {
> +    set filename ${::binfile}_1
> +    if { [build_executable "build exec" ${filename} $::srcfile \
> +	      {additional_flags=-static debug build-id}] } {
> +	return
> +    }
> +
> +    # Split debug information into a .debug file, remove debug
> +    # information from FILENAME.  Don't add a .gnu_debuglink to
> +    # FILENAME, we rely on the build-id.
> +    if {[gdb_gnu_strip_debug $filename { no-debuglink }] != 0} {
> +	unsupported "cannot split debug information from executable"
> +	return
> +    }
> +
> +    set sysroot [standard_output_file "sysroot1"]
> +    set debug_dir "/debug"
> +
> +    set debug_symlink \
> +	${sysroot}${debug_dir}/[build_id_debug_filename_get $filename]
> +
> +    set build_id_dir [file dirname $debug_symlink]
> +
> +    set debug_filename ${filename}_hidden_debug
> +
> +    remote_exec build "mkdir -p $build_id_dir"
> +    remote_exec build "mv $filename.debug $debug_filename"
> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
> +
> +    foreach_with_prefix sysroot_prefix { "" "target:" } {
> +	clean_restart
> +
> +	gdb_test_no_output "set sysroot ${sysroot_prefix}$sysroot" "set sysroot"
> +	gdb_test_no_output "set debug-file-directory $debug_dir"
> +
> +	gdb_file_cmd $filename
> +
> +	gdb_assert { $::gdb_file_cmd_debug_info eq "debug" } \
> +	    "ensure debug information was found"
> +
> +	if { $sysroot_prefix eq "" } {
> +	    set lookup_filename $debug_filename
> +	} else {
> +	    # Only when using the extended-remote board will we have
> +	    # started a remote target by this point.  In this case GDB
> +	    # will see the 'target:' prefix as remote, and so the
> +	    # reported filename will include the 'target:' prefix.
> +	    #
> +	    # In all other cases we will still be using the default,
> +	    # initial target, in which case GDB considers the
> +	    # 'target:' prefix to indicate the local filesystem.
> +	    if {[target_info gdb_protocol] == "extended-remote"} {
> +		set lookup_filename $sysroot_prefix$debug_symlink
> +	    } else {
> +		set lookup_filename $debug_symlink
> +	    }
> +	}
> +	set re [string_to_regexp "Reading symbols from $lookup_filename..."]
> +	gdb_assert {[regexp $re $::gdb_file_cmd_msg]} \
> +	    "debug symbols read from correct file"
> +    }
> +}
> +
> +# Create a copy of BINFILE, split out the debug information, and then
> +# setup a sysroot.  Hide (by moving) the actual debug information file
> +# and create a symlink to the hidden debug information from within the
> +# sysroot.
> +#
> +# Copy the executable into the sysroot and then start GDB, set the
> +# sysroot, and load the executable.  Check that GDB finds the debug
> +# information, which must have happened by lookin in the sysroot.
> +proc_with_prefix lookup_via_debuglink {} {
> +    set filename ${::binfile}_2
> +    if { [build_executable "build exec" ${filename} $::srcfile \
> +	      {additional_flags=-static debug no-build-id}] } {
> +	return
> +    }
> +
> +    # Split debug information into a .debug file, remove debug
> +    # information from FILENAME.
> +    if {[gdb_gnu_strip_debug $filename] != 0} {
> +	unsupported "cannot split debug information from executable"
> +	return
> +    }
> +
> +    # We're going to setup the sysroot like this:
> +    #
> +    # sysroot2/
> +    #    bin/
> +    #      $FILENAME
> +    #    debug/
> +    #      bin/
> +    #        $FILENAME.debug
> +    #
> +    # When looking up debug information via the debuglink, GDB will
> +    # only search in the sysroot if the original objfile was in the
> +    # sysroot.  And GDB will resolve symlinks, so if the objfile is
> +    # symlinked to outside the sysroot, GDB will not search in the
> +    # sysroot for the debug information.
> +    #
> +    # So we have to copy the executable into the sysroot.
> +    #
> +    # We are OK to symlink the debug information to a file outside the
> +    # sysroot though.
> +
> +    set sysroot [standard_output_file "sysroot2"]
> +
> +    foreach path { bin debug/bin } {
> +	remote_exec build "mkdir -p $sysroot/$path"
> +    }
> +
> +    # Copy the executable into the sysroot.
> +    set file_basename [file tail $filename]
> +    set exec_in_sysroot ${sysroot}/bin/${file_basename}
> +    remote_exec build "cp $filename $exec_in_sysroot"
> +
> +    # Rename the debug file outside of the sysroot, this should stop
> +    # GDB finding this file "by accident".
> +    set debug_filename ${filename}_hidden_debug
> +    remote_exec build "mv $filename.debug $debug_filename"
> +
> +    # Symlink the debug information into the sysroot.
> +    set debug_symlink \
> +	${sysroot}/debug/bin/${file_basename}.debug
> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
> +
> +    foreach_with_prefix sysroot_prefix { "" "target:" } {
> +	# Restart GDB and setup the sysroot and debug directory.
> +	clean_restart
> +	gdb_test_no_output "set sysroot ${sysroot_prefix}$sysroot" "set sysroot"
> +	gdb_test_no_output "set debug-file-directory /debug"
> +
> +	# Load the executable, we expect GDB to find the debug information
> +	# in the sysroot.
> +	gdb_file_cmd ${sysroot_prefix}$exec_in_sysroot
> +
> +	# Giving a sysroot a 'target:' prefix doesn't seem to work as
> +	# might be expected for debug-link based lookups.  For this
> +	# style of lookup GDB only seems to care if the original file
> +	# itself had a 'target:' prefix.  The 'target:' prefix on the
> +	# sysroot just seems to cause GDB to bail on looking up via
> +	# the 'target:' prefixed sysroot.
> +	#
> +	# Bug PR gdb/30866 seems to be the (or a) relevant bug for
> +	# this problem.
> +	if { $sysroot_prefix ne "" } {
> +	    setup_xfail "*-*-*" 30866
> +	}
> +	gdb_assert { $::gdb_file_cmd_debug_info eq "debug" } \
> +	    "ensure debug information was found"
> +
> +	if { $sysroot_prefix ne "" } {
> +	    setup_xfail "*-*-*" 30866
> +	}
> +	set re [string_to_regexp "Reading symbols from ${sysroot_prefix}$debug_symlink..."]
> +	gdb_assert {[regexp $re $::gdb_file_cmd_msg]} \
> +	    "debug symbols read from correct file"
> +    }
> +}
> +
> +lookup_via_build_id
> +lookup_via_debuglink
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index a63c13f9cbc..a8e8b69d6ef 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5211,6 +5211,7 @@ proc quote_for_host { args } {
>   #     debug information
>   #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
>   #   - build-id: Ensure the final binary includes a build-id.
> +#   - no-build-id: Ensure the final binary does not include a build-id.
>   #   - column-info/no-column-info: Enable/Disable generation of column table
>   #     information.
>   #
> @@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} {
>   	lappend new_options "additional_flags=-Wl,--build-id"
>       }
>   
> +    # If the 'no-build-id' option is used then disable the build-id.
> +    if {[lsearch -exact $options no-build-id] > 0} {
> +	lappend new_options "additional_flags=-Wl,--build-id=none"
> +    }
> +
> +    # Sanity check.  If both 'build-id' and 'no-build-id' are used
> +    # then what is expected from us!
> +    if {[lsearch -exact $options build-id] > 0
> +	&& [lsearch -exact $options no-build-id] > 0} {
> +	error "cannot use build-id and no-build-id options"
> +    }
> +
>       # Treating .c input files as C++ is deprecated in Clang, so
>       # explicitly force C++ language.
>       if { !$getting_compiler_info
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent
  2024-05-26 23:08     ` Andrew Burgess
@ 2024-05-27  7:15       ` Tom de Vries
  0 siblings, 0 replies; 10+ messages in thread
From: Tom de Vries @ 2024-05-27  7:15 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 5/27/24 01:08, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
> 
>> On 5/20/24 16:14, Andrew Burgess wrote:
>>> While writing a test I realised that the default behaviour of
>>> gdb_gnu_strip_debug doesn't match its comment.
>>>
>>> The comment says that the function takes a FILENAME, and splits the
>>> file into FILENAME.stripped and FILENAME.debug, leaving FILENAME
>>> unchanged.  The comment says that a .gnu_debuglink will be added to
>>> FILENAME.stripped.
>>>
>>> However, this is not true, FILENAME.stripped is created, with no debug
>>> information.  FILENAME.debug is created containing the debug
>>> information.
>>>
>>> But, when adding the .gnu_debuglink we take FILENAME.stripped as the
>>> input, and then overwrite FILENAME with the output.  As a result,
>>> FILENAME.stripped does not include a .gnu_debuglink, while FILENAME
>>> contains the .gnu_debuglink and no debug information!
>>>
>>> The users of gdb_gnu_strip_debug can be split into two groups, those
>>> who are using the .gnu_debuglink, these tests are all written assuming
>>> that FILENAME is updated.
>>>
>>> Then there are some tests that rely on gdb_gnu_strip_debug's ability
>>> to split out the debug information,
>>
>> I got confused a bit here.  Doesn't the first group also rely on this?
>>
>>> these tests are then going to do a
>>> lookup based on the build-id.  These tests use the FILENAME.stripped
>>> output file.
>>>
>>> This all seems too confused to me.
>>>
>>> As most used of gdb_gnu_strip_debug assume that FILENAME is updated, I
>>
>> used -> uses
>>
>>> propose that we just make that the actual, advertised behaviour of
>>> this proc.
>>>
>>
>> Make sense.
>>
>> [ FWIW, the overwriting behaviour is not great for the case where the
>> exec is used in the initial part of a test-case, and then modified by
>> gdb_gnu_strip_debug (I don't know whether that occurs though).  When
>> trying to reproduce the initial part of the tests using say -x gdb.in.1
>> the behaviour may be different because the exec has changed in the mean
>> time.  So going from that point of view we could have gone for the
>> syntax of gdb_gnu_strip_debug returning the filename of the stripped
>> exec, if successful.  Come to think of it, doing so might also allow us
>> to name the stripped exec differently, say foo.stripped and
>> foo.stripped-with-link, which will make it more clear in gdb.log what
>> exec is used. ]
> 
> I see what you're saying here.  But I'm really not super keen to start
> rewriting all the tests that use gdb_gnu_strip_debug right now :-/ As
> most of the tests that call this proc are already written assuming the
> overwrite behaviour.
> 
> Unless you strongly object then I'm planning to just go with this as it
> is right now.  But if it really bothers you I can tackle fixing all the
> tests.  Let me know.
> 

Hi Andrew,

No objections whatsoever.  I was just mentioning it to make explicit 
that the overwriting behaviour is a bit problematic, but I don't see a 
pressing need to fix this in this patch.

Thanks,
- Tom

> Anyway, updated patch with the minor type fixes, and slightly clearer
> commit message is below, just in case you wanted another look.
> 
> Thanks,
> Andrew
> 
> ---
> 
> commit b888aea514e5eb968ef0e2e30f8042848f88fc90
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Sat May 18 11:05:49 2024 +0100
> 
>      gdb/testsuite: make gdb_gnu_strip_debug consistent
>      
>      While writing a test I realised that the default behaviour of
>      gdb_gnu_strip_debug doesn't match its comment.
>      
>      The comment says that the function takes a FILENAME, and splits the
>      file into FILENAME.stripped and FILENAME.debug, leaving FILENAME
>      unchanged.  The comment says that a .gnu_debuglink will be added to
>      FILENAME.stripped.
>      
>      However, this is not true, FILENAME.stripped is created, with no debug
>      information.  FILENAME.debug is created containing the debug
>      information.
>      
>      But, when adding the .gnu_debuglink we take FILENAME.stripped as the
>      input, and then overwrite FILENAME with the output.  As a result,
>      FILENAME.stripped does not include a .gnu_debuglink, while FILENAME
>      contains the .gnu_debuglink and no debug information!
>      
>      The users of gdb_gnu_strip_debug can be split into two groups, those
>      who are using the .gnu_debuglink, these tests are all written assuming
>      that FILENAME is updated.
>      
>      Then there are some tests that only rely on gdb_gnu_strip_debug's
>      ability to split out the debug information, these tests are then going
>      to do a lookup based on the build-id, these tests don't require the
>      .gnu_debuglink.  These tests use the FILENAME.stripped output file.
>      
>      This all seems too confused to me.
>      
>      As most uses of gdb_gnu_strip_debug assume that FILENAME is updated, I
>      propose that we just make that the actual, advertised behaviour of
>      this proc.
>      
>      So now, gdb_gnu_strip_debug will take FILENAME, and will split the
>      debug information out into FILENAME.debug.  The debug information will
>      then be stripped from FILENAME, and by default a .gnu_debuglink will
>      be added to FILENAME pointing to FILENAME.debug.
>      
>      I've updated the two tests that actually relied on FILENAME.stripped
>      to instead just use FILENAME.
>      
>      One of the tests was doing a build-id based lookup, but was still
>      allowing the .gnu_debuglink to be added to FILENAME, I've updated this
>      test to pass the no-debuglink flag to gdb_gnu_strip_debug, which stops
>      the .gnu_debuglink from being added.
>      
>      All of the tests that call gdb_gnu_strip_debug still pass for me.
>      
>      Acked-By: Tom de Vries <tdevries@suse.de>
> 
> diff --git a/gdb/testsuite/gdb.base/corefile-buildid.exp b/gdb/testsuite/gdb.base/corefile-buildid.exp
> index 130198611ec..e1b9804d891 100644
> --- a/gdb/testsuite/gdb.base/corefile-buildid.exp
> +++ b/gdb/testsuite/gdb.base/corefile-buildid.exp
> @@ -172,11 +172,9 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
>   	"mkdir -p [file join $debugdir [file dirname $buildid]]"
>   
>       set files_list {}
> +    lappend files_list $binfile $buildid
>       if {$sepdebug} {
> -	lappend files_list "$binfile.stripped" $buildid
>   	lappend files_list "$binfile.debug" "$buildid.debug"
> -    } else {
> -	lappend files_list $binfile $buildid
>       }
>       if {$shared} {
>   	global sharedir
> @@ -200,12 +198,7 @@ proc locate_exec_from_core_build_id {corefile buildid suffix \
>       gdb_test "core-file $corefile" "Program terminated with .*" \
>   	"load core file"
>       if {$symlink} {
> -	if {$sepdebug} {
> -	    set expected_file [file join $builddir \
> -				   [file tail "$binfile.stripped"]]
> -	} else {
> -	    set expected_file [file join $builddir [file tail $binfile]]
> -	}
> +	set expected_file [file join $builddir [file tail $binfile]]
>       } else {
>   	set expected_file $buildid
>       }
> @@ -245,15 +238,12 @@ proc do_corefile_buildid_tests {args} {
>   
>       if {$sepdebug} {
>   	# Strip debuginfo into its own file.
> -	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run]] \
> -		!= 0} {
> +	if {[gdb_gnu_strip_debug [standard_output_file $program_to_run] \
> +		 no-debuglink] != 0} {
>   	    untested "could not strip executable  for [join $suffix \ ]"
>   	    return
>   	}
>   
> -	# Run the stripped program instead of the original.
> -	set program_to_run [file join $builddir \
> -				[file tail "$binfile.stripped"]]
>   	lappend suffix "sepdebug"
>       }
>   
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> index b86622543ef..aa1c263e800 100644
> --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> @@ -30,14 +30,11 @@ if {[build_executable "build executable" ${testfile} ${srcfile} \
>       return -1
>   }
>   
> -# Split BINFILE into BINFILE.stripped and BINFILE.debug, the first is
> -# the executable with the debug information removed, and the second is
> -# the debug information.
> +# Split debug information from BINFILE into BINFILE.debug.
>   #
> -# However, by passing the "no-debuglink" flag we prevent this proc
> -# from adding a .gnu_debuglink section to the executable.  Any lookup
> -# of the debug information by GDB will need to be done based on the
> -# build-id.
> +# By passing the "no-debuglink" flag we prevent this proc from adding
> +# a .gnu_debuglink section to BINFILE.  Any lookup of the debug
> +# information by GDB will need to be done based on the build-id.
>   if {[gdb_gnu_strip_debug $binfile no-debuglink]} {
>       unsupported "cannot produce separate debug info files"
>       return -1
> @@ -59,12 +56,6 @@ set debuginfod_debugdir [standard_output_file "debug"]
>   remote_exec build "mkdir $debuginfod_debugdir"
>   remote_exec build "mv $debugfile $debuginfod_debugdir"
>   
> -# This is BINFILE with the debug information removed.  We are going to
> -# place this in the BUILD_ID_DEBUG_FILE location, this would usually
> -# represent a mistake by the user, and will trigger a warning from
> -# GDB, this is the warning we are checking for.
> -set stripped_binfile [standard_output_file "${binfile}.stripped"]
> -
>   # Create the .build-id/PREFIX directory name from
>   # .build-id/PREFIX/SUFFIX.debug filename.
>   set debugdir [file dirname ${build_id_debug_file}]
> @@ -76,7 +67,7 @@ remote_exec build "mkdir -p $debugdir"
>   # information, which will point back at this file, which also doesn't
>   # have debug information, which could cause a loop.  But GDB will spot
>   # this and give a warning.
> -remote_exec build "mv ${stripped_binfile} ${build_id_debug_file}"
> +remote_exec build "mv ${binfile} ${build_id_debug_file}"
>   
>   # Now start GDB.
>   clean_restart
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 0d78691c381..a63c13f9cbc 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -8017,10 +8017,11 @@ proc build_id_debug_filename_get { filename } {
>   }
>   
>   # DEST should be a file compiled with debug information.  This proc
> -# creates two new files DEST.debug which contains the debug
> -# information extracted from DEST, and DEST.stripped, which is a copy
> -# of DEST with the debug information removed.  A '.gnu_debuglink'
> -# section will be added to DEST.stripped that points to DEST.debug.
> +# creates DEST.debug which contains the debug information extracted
> +# from DEST, and DEST is updated with the debug information removed.
> +#
> +# By default a '.gnu_debuglink' section will be added to DEST that
> +# points to DEST.debug.
>   #
>   # If ARGS is passed, it is a list of optional flags.  The currently
>   # supported flags are:
> @@ -8028,7 +8029,7 @@ proc build_id_debug_filename_get { filename } {
>   #   - no-main : remove the symbol entry for main from the separate
>   #               debug file DEST.debug,
>   #   - no-debuglink : don't add the '.gnu_debuglink' section to
> -#                    DEST.stripped.
> +#                    DEST.
>   #
>   # Function returns zero on success.  Function will return non-zero failure code
>   # on some targets not supporting separate debug info (such as i386-msdos).
> @@ -8087,20 +8088,26 @@ proc gdb_gnu_strip_debug { dest args } {
>       # Unless the "no-debuglink" flag is passed, then link the two
>       # previous output files together, adding the .gnu_debuglink
>       # section to the stripped_file, containing a pointer to the
> -    # debug_file, save the new file in dest.
> +    # debug_file.
>       if {[lsearch -exact $args "no-debuglink"] == -1} {
> -	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${dest}" output]
> +	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${stripped_file}-tmp" output]
>   	verbose "result is $result"
>   	verbose "output is $output"
>   	if {$result == 1} {
>   	    return 1
>   	}
> +	file delete "${stripped_file}"
> +	file rename "${stripped_file}-tmp" "${stripped_file}"
>       }
>   
>       # Workaround PR binutils/10802:
>       # Preserve the 'x' bit also for PIEs (Position Independent Executables).
> -    set perm [file attributes ${stripped_file} -permissions]
> -    file attributes ${dest} -permissions $perm
> +    set perm [file attributes ${dest} -permissions]
> +    file attributes ${stripped_file} -permissions $perm
> +
> +    # Move the stripped_file back into dest.
> +    file delete ${dest}
> +    file rename ${stripped_file} ${dest}
>   
>       return 0
>   }
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot
  2024-05-27  7:07       ` Tom de Vries
@ 2024-06-04 12:40         ` Andrew Burgess
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2024-06-04 12:40 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

Tom de Vries <tdevries@suse.de> writes:

> On 5/27/24 01:04, Andrew Burgess wrote:
>> Tom de Vries <tdevries@suse.de> writes:
>> 
>>> On 5/20/24 16:14, Andrew Burgess wrote:
>>>> Add tests for looking up debug information within the sysroot via both
>>>> build-id and gnu_debuglink.
>>>>
>>>> I wanted to ensure that the gnu_debuglink test couldn't make use of
>>>> build-ids, so I added the 'no-build-id' flag to gdb_compile.
>>>>
>>>> As these tests rely on setting the sysroot, if I'm running a
>>>> dynamically linked executable, GDB will try to find all shared
>>>> libraries within the sysroot.  This would mean I'd have to figure out
>>>> and copy all shared libraries the executable uses, certainly possible,
>>>> but a bit of a pain.
>>>>
>>>> So instead, I've just compiled the test executable as a static binary.
>>>> Now there are no shared library dependencies.
>>>>
>>>
>>> Make sense.
>>>
>>>> I can now split the debug information out from the test binary, and
>>>> place it within the sysroot.  When GDB is started and the executable
>>>> loaded, we can check that GDB is finding the debug information within
>>>> the sysroot.
>>>>
>>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30871
>>>> ---
>>>>    gdb/testsuite/gdb.base/sysroot-debug-lookup.c |  22 +++
>>>>    .../gdb.base/sysroot-debug-lookup.exp         | 162 ++++++++++++++++++
>>>>    gdb/testsuite/lib/gdb.exp                     |  13 ++
>>>>    3 files changed, 197 insertions(+)
>>>>    create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>>>>    create mode 100644 gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>>>>
>>>> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.c b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>>>> new file mode 100644
>>>> index 00000000000..e67331d3150
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.c
>>>> @@ -0,0 +1,22 @@
>>>> +/* This testcase is part of GDB, the GNU debugger.
>>>> +
>>>> +   Copyright 1992-2024 Free Software Foundation, Inc.
>>>> +
>>>> +   This program is free software; you can redistribute it and/or modify
>>>> +   it under the terms of the GNU General Public License as published by
>>>> +   the Free Software Foundation; either version 3 of the License, or
>>>> +   (at your option) any later version.
>>>> +
>>>> +   This program is distributed in the hope that it will be useful,
>>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> +   GNU General Public License for more details.
>>>> +
>>>> +   You should have received a copy of the GNU General Public License
>>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>> +
>>>> +int
>>>> +main ()
>>>> +{
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>>>> new file mode 100644
>>>> index 00000000000..e0377df0fa7
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
>>>> @@ -0,0 +1,162 @@
>>>> +# Copyright 2024 Free Software Foundation, Inc.
>>>> +#
>>>> +# This program is free software; you can redistribute it and/or modify
>>>> +# it under the terms of the GNU General Public License as published by
>>>> +# the Free Software Foundation; either version 3 of the License, or
>>>> +# (at your option) any later version.
>>>> +#
>>>> +# This program is distributed in the hope that it will be useful,
>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> +# GNU General Public License for more details.
>>>> +#
>>>> +# You should have received a copy of the GNU General Public License
>>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>> +
>>>> +# Test GDB's ability to find debug information by looking within the
>>>> +# sysroot.
>>>> +#
>>>> +# We compile a static binary (to reduce what we need to copy into the
>>>> +# sysroot), split the debug information from the binary, and setup a
>>>> +# sysroot.
>>>> +#
>>>> +# The debug-file-directory is set to just '/debug', but we're
>>>> +# expecting GDB to actually look in '$SYSROOT/debug'.
>>>> +#
>>>> +# There's a test for using .build-id based lookup, and a test for
>>>> +# gnu_debuglink based lookup.
>>>> +
>>>> +require {!is_remote host}
>>>> +
>>>> +standard_testfile
>>>
>>> Consider using "standard_testfile main.c" and dropping
>>> gdb/testsuite/gdb.base/sysroot-debug-lookup.c.
>> 
>> Done.
>> 
>>>
>>>> +
>>>> +# Create a copy of BINFILE, split out the debug information, and then
>>>> +# setup a sysroot.  Hide (by moving) the actual debug information file
>>>> +# and create a symlink to the hidden debug information from within the
>>>> +# sysroot.
>>>> +#
>>>> +# Start GDB, set the sysroot, and then load the executable, ensure GDB
>>>> +# finds the debug information, which must have happened by lookin in
>>>> +# the sysroot.
>>>> +proc_with_prefix lookup_via_build_id {} {
>>>> +    set filename ${::binfile}_1
>>>> +    if { [build_executable "build exec" ${filename} $::srcfile \
>>>> +	      {additional_flags=-static debug build-id}] } {
>>>> +	return
>>>> +    }
>>>> +
>>>> +    # Split debug information into a .debug file, remove debug
>>>> +    # information from FILENAME.  Don't add a .gnu_debuglink to
>>>> +    # FILENAME, we rely on the build-id.
>>>> +    if {[gdb_gnu_strip_debug $filename { no-debuglink }] != 0} {
>>>> +	unsupported "cannot split debug information from executable"
>>>> +	return
>>>> +    }
>>>> +
>>>> +    set sysroot [standard_output_file "sysroot1"]
>>>> +    set debug_dir "/debug"
>>>> +
>>>> +    set debug_symlink \
>>>> +	$sysroot/$debug_dir/[build_id_debug_filename_get $filename]
>>>> +
>>>> +    set build_id_dir [file dirname $debug_symlink]
>>>> +
>>>> +    set debug_filename ${filename}_hidden_debug
>>>> +
>>>> +    remote_exec build "mkdir -p $build_id_dir"
>>>> +    remote_exec build "mv $filename.debug $debug_filename"
>>>> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
>>>> +
>>>> +    clean_restart
>>>> +
>>>> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
>>>> +    gdb_test_no_output "set debug-file-directory $debug_dir"
>>>> +
>>>> +    gdb_file_cmd $filename
>>>> +
>>>> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
>>>> +
>>>> +    set re [string_to_regexp "Reading symbols from $debug_filename..."]
>>>> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
>>>
>>> Consider adding a friendlier test name, likewise in the other proc.
>>>
>>> Currently we get for a fail the not very meaningful:
>>> ...
>>> FAIL: $exp: lookup_via_build_id: [regexp $re $::gdb_file_cmd_msg]
>> 
>> Done (I think).
>> 
>>> ...
>>>
>>>> +}
>>>> +
>>>> +# Create a copy of BINFILE, split out the debug information, and then
>>>> +# setup a sysroot.  Hide (by moving) the actual debug information file
>>>> +# and create a symlink to the hidden debug information from within the
>>>> +# sysroot.
>>>> +#
>>>> +# Copy the executable into the sysroot and then start GDB, set the
>>>> +# sysroot, and load the executable.  Check that GDB finds the debug
>>>> +# information, which must have happened by lookin in the sysroot.
>>>> +proc_with_prefix lookup_via_debuglink {} {
>>>> +    set filename ${::binfile}_2
>>>> +    if { [build_executable "build exec" ${filename} $::srcfile \
>>>> +	      {additional_flags=-static debug no-build-id}] } {
>>>> +	return
>>>> +    }
>>>> +
>>>> +    # Split debug information into a .debug file, remove debug
>>>> +    # information from FILENAME.
>>>> +    if {[gdb_gnu_strip_debug $filename] != 0} {
>>>> +	unsupported "cannot split debug information from executable"
>>>> +	return
>>>> +    }
>>>> +
>>>> +    # We're going to setup the sysroot like this:
>>>> +    #
>>>> +    # sysroot2/
>>>> +    #    bin/
>>>> +    #      $FILENAME
>>>> +    #    debug/
>>>> +    #      bin/
>>>> +    #        $FILENAME.debug
>>>> +    #
>>>> +    # When looking up debug information via the debuglink, GDB will
>>>> +    # only search in the sysroot if the original objfile was in the
>>>> +    # sysroot.  And GDB will resolve symlinks, so if the objfile is
>>>> +    # symlinked to outside the sysroot, GDB will not search in the
>>>> +    # sysroot for the debug information.
>>>> +    #
>>>> +    # So we have to copy the executable into the sysroot.
>>>> +    #
>>>> +    # We are OK to symlink the debug information to a file outside the
>>>> +    # sysroot though.
>>>> +
>>>> +    set sysroot [standard_output_file "sysroot2"]
>>>> +
>>>> +    foreach path { bin debug/bin } {
>>>> +	remote_exec build "mkdir -p $sysroot/$path"
>>>> +    }
>>>> +
>>>> +    # Copy the executable into the sysroot.
>>>> +    set file_basename [file tail $filename]
>>>> +    set exec_in_sysroot ${sysroot}/bin/${file_basename}
>>>> +    remote_exec build "cp $filename $exec_in_sysroot"
>>>> +
>>>> +    # Rename the debug file outside of the sysroot, this should stop
>>>> +    # GDB finding this file "by accident".
>>>> +    set debug_filename ${filename}_hidden_debug
>>>> +    remote_exec build "mv $filename.debug $debug_filename"
>>>> +
>>>> +    # Symlink the debug information into the sysroot.
>>>> +    set debug_symlink \
>>>> +	${sysroot}/debug/bin/${file_basename}.debug
>>>> +    remote_exec build "ln -sf $debug_filename $debug_symlink"
>>>> +
>>>> +    # Restart GDB and setup the sysroot and debug directory.
>>>> +    clean_restart
>>>> +    gdb_test_no_output "set sysroot $sysroot" "set sysroot"
>>>> +    gdb_test_no_output "set debug-file-directory /debug"
>>>> +
>>>> +    # Load the executable, we expect GDB to find the debug information
>>>> +    # in the sysroot.
>>>> +    gdb_file_cmd $exec_in_sysroot
>>>> +
>>>> +    gdb_assert { $::gdb_file_cmd_debug_info eq "debug" }
>>>> +
>>>> +    set re [string_to_regexp "Reading symbols from $debug_symlink..."]
>>>> +    gdb_assert {[regexp $re $::gdb_file_cmd_msg]}
>>>> +}
>>>> +
>>>> +lookup_via_build_id
>>>> +lookup_via_debuglink
>>>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>>>> index a63c13f9cbc..a8e8b69d6ef 100644
>>>> --- a/gdb/testsuite/lib/gdb.exp
>>>> +++ b/gdb/testsuite/lib/gdb.exp
>>>> @@ -5211,6 +5211,7 @@ proc quote_for_host { args } {
>>>>    #     debug information
>>>>    #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
>>>>    #   - build-id: Ensure the final binary includes a build-id.
>>>> +#   - no-build-id: Ensure the final binary does not include a build-id.
>>>>    #   - column-info/no-column-info: Enable/Disable generation of column table
>>>>    #     information.
>>>>    #
>>>> @@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} {
>>>>    	lappend new_options "additional_flags=-Wl,--build-id"
>>>>        }
>>>>    
>>>> +    # If the 'no-build-id' option is used then disable the build-id.
>>>> +    if {[lsearch -exact $options no-build-id] > 0} {
>>>> +	lappend new_options "additional_flags=-Wl,--build-id=none"
>>>> +    }
>>>> +
>>>> +    # Sanity check.  If both 'build-id' and 'no-build-id' are used
>>>> +    # then what is expected from us!
>>>> +    if {[lsearch -exact $options build-id] > 0
>>>> +	&& [lsearch -exact $options no-build-id] > 0} {
>>>> +	error "cannot use build-id and no-build-id options"
>>>> +    }
>>>> +
>>>>        # Treating .c input files as C++ is deprecated in Clang, so
>>>>        # explicitly force C++ language.
>>>>        if { !$getting_compiler_info
>>>
>>>
>>> I've added the target: prefix for the set sysroot commands, and ran into
>>> these three fails:
>>> ...
>>> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_build_id: [regexp
>>> $re $::gdb_file_cmd_msg]
>>> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink:
>>> $::gdb_file_cmd_debug_info eq "debug"
>>> FAIL: gdb.base/sysroot-debug-lookup.exp: lookup_via_debuglink: [regexp
>>> $re $::gdb_file_cmd_msg]
>>> ...
>>>
>>> The first one is just due to gdb mentioning an unresolved symlink.
>> 
>> Yeah.  The build-id lookup w.r.t target: sysroots seems pretty sane.
>> I've extended the test to cover the target: case.
>> 
>>>
>>> But I think the next one reproduces PR30866.  So, IWBN to add this
>>> dimension to the test-case and add a kfail.
>> 
>> I'm not 100% certain if the issue here is 30866 or not.  I can't
>> reproduce 30866, I think the namespace mounting support in GDB is not
>> working for me, for some reason.  I'll need to dig into this at some
>> future time.
>> 
>> However, if it's not exactly 30866, then it's certainly something like
>> 30866!  So, for now I've added an xfail, blamed it on 30866, and figure
>> that if/when 30866 is fixed, if this test doesn't start working, we can
>> open a new bug at that time.
>> 
>> You did already approve, but given the changes I wanted to let you take
>> another look before I merged this.
>> 
>> Let me know if you have any further thoughts.
>> 
>
> Interesting.  Looking into it a bit further, I think it is indeed a 
> different issue, thanks for pointing that out.  I filed a PR ( 
> https://sourceware.org/bugzilla/show_bug.cgi?id=31804 ) for this, and 
> investigated a bit and posted a demonstrator patch.
>
> Anyway, please refer to this PR, and do so using a kfail, since AFAIU 
> this a problem in gdb, not external to gdb.
>
> Otherwise, LGTM.

I changed to kfail, updated the bug number to 31804, and pushed this
series.

Thanks for your feedback,
Andrew


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-06-04 12:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-20 14:14 [PATCH 0/2] Add tests for finding debug information within sysroot Andrew Burgess
2024-05-20 14:14 ` [PATCH 1/2] gdb/testsuite: make gdb_gnu_strip_debug consistent Andrew Burgess
2024-05-23  9:13   ` Tom de Vries
2024-05-26 23:08     ` Andrew Burgess
2024-05-27  7:15       ` Tom de Vries
2024-05-20 14:14 ` [PATCH 2/2] gdb/testsuite: tests for debug lookup within the sysroot Andrew Burgess
2024-05-23 10:17   ` Tom de Vries
2024-05-26 23:04     ` Andrew Burgess
2024-05-27  7:07       ` Tom de Vries
2024-06-04 12:40         ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).