[PATCH v2] gdb/testsuite: Installed-GDB testing & data-directory

[Pedro Alves <pedro@palves.net>]

On 2022-03-18 16:43, Pedro Alves wrote:

> Do you disagree that what we suggest in testsuite/README:
> 
>     make check RUNTESTFLAGS=GDB=/usr/bin/gdb
> 
> behaves incorrectly (in the sense that it does unsupportable things),
> and we should instead suggest something that works correctly by default?
> 
> My first approach was to just let users override GDB_DATA_DIRECTORY, and
> thus tweak the README to read:
> 
>     make check RUNTESTFLAGS="GDB=/usr/bin/gdb GDB_DATA_DIRECTORY="
> 
> but then extending the README text to explain this felt pretty awkward and I
> realized that we should probably default GDB_DATA_DIRECTORY to "" if
> GDB was overridden, and then things would work for 99% of the users.
> 
> I admit I didn't think people were using this the way you are using it.

Below's what such an approach looks like.  This works for us as well,
we just have to pass the GDB_DATA_DIRECTORY= explicitly, which isn't as
nice, but isn't a big deal.

Let me know what you think.

From e701a7ea7812cf60a0a146c705f4ba27dd31ad11 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 18 Mar 2022 12:39:55 +0000
Subject: [[PATCH v2]] gdb/testsuite: Installed-GDB testing & data-directory

In testsuite/README, we suggest that you can run the testsuite against
some other GDB binary by using:

    make check RUNTESTFLAGS=GDB=/usr/bin/gdb

However, that example isn't fully correct, because with that command
line, the testsuite will still pass

  -data-directory=[pwd]/../data-directory

to /usr/bin/gdb, like e.g.:

  ...
  builtin_spawn /usr/bin/gdb -nw -nx -data-directory /home/pedro/gdb/build/gdb/testsuite/../data-directory -iex set height 0 -iex set width 0
  ...

while if you're testing an installed GDB (the system GDB being the
most usual scenario), then you should normally let it use its own
configured directory, not the just-built GDB's data directory.

This commit improves the status quo by letting the user override the
data directory, via a new GDB_DATA_DIRECTORY global.  This replaces
the existing BUILD_DATA_DIRECTORY variable in testsuite/lib/gdb.exp,
which wasn't overridable, and was a bit misnamed for the new purpose.

Here are some use case examples:

 # Test the non-installed GDB in some build dir:

    make check \
      RUNTESTFLAGS="GDB=/path/to/other/build/gdb \
                    GDB_DATA_DIRECTORY=/path/to/other/build/gdb/data-directory"

 # Test the GDB installed in some prefix:

    make check \
      RUNTESTFLAGS="GDB=/opt/gdb/bin/gdb GDB_DATA_DIRECTORY="

 # Test the built GDB with some alternative data directory, e.g., the
   system GDB's data directory:

    make check \
      RUNTESTFLAGS="GDB_DATA_DIRECTORY=/usr/share/gdb"

Change-Id: Icdc21c85219155d9564a9900961997e6624b78fb
---
 gdb/testsuite/README      | 29 +++++++++++++++++++++++++----
 gdb/testsuite/lib/gdb.exp | 37 +++++++++++++++++++++++++++++++------
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index c2f659a7188..cae78fe7946 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -13,14 +13,17 @@ There are two ways to run the testsuite and pass additional parameters
 to DejaGnu.  The first is to do `make check' in the main build
 directory and specifying the makefile variable `RUNTESTFLAGS':
 
-	 make check RUNTESTFLAGS='GDB=/usr/bin/gdb gdb.base/a2-run.exp'
+	 make check \
+	   RUNTESTFLAGS='GDB=/usr/bin/gdb \
+	                 GDB_DATA_DIRECTORY= \
+			 gdb.base/a2-run.exp'
 
 The second is to cd to the testsuite directory and invoke the DejaGnu
 `runtest' command directly.
 
 	cd testsuite
 	make site.exp
-	runtest GDB=/usr/bin/gdb
+	runtest GDB=/usr/bin/gdb GDB_DATA_DIRECTORY=
 
 (The `site.exp' file contains a handful of useful variables like host
 and target triplets, and pathnames.)
@@ -151,9 +154,10 @@ By default, the testsuite exercises the GDB in the build directory,
 but you can set GDB to be a pathname to a different version.  For
 instance,
 
-    make check RUNTESTFLAGS=GDB=/usr/bin/gdb
+    make check RUNTESTFLAGS="GDB=/usr/bin/gdb GDB_DATA_DIRECTORY="
 
-runs the testsuite on the GDB in /usr/bin.
+runs the testsuite on the GDB installed in /usr/bin.  See below for
+the description of GDB_DATA_DIRECTORY.
 
 GDBSERVER
 
@@ -164,6 +168,23 @@ instance
 
 checks both the installed GDB and GDBserver.
 
+GDB_DATA_DIRECTORY
+
+By default the tested GDB uses the data directory found under the GDB
+build directory.  You can override this by setting GDB_DATA_DIRECTORY.
+For instance:
+
+    make check \
+      RUNTESTFLAGS="GDB=/path/to/other/build/gdb \
+                    GDB_DATA_DIRECTORY=/path/to/other/build/gdb/data-directory"
+
+If GDB_DATA_DIRECTORY is set to the empty string, then the testsuite
+lets GDB use its configured data directory.  This is useful to test an
+installed GDB.  For example:
+
+    make check \
+      RUNTESTFLAGS="GDB=/usr/bin/gdb GDB_DATA_DIRECTORY="
+
 INTERNAL_GDBFLAGS
 
 Command line options passed to all GDB invocations.
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 08726f78563..89011336b6d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -92,8 +92,13 @@ load_lib gdb-utils.exp
 load_lib memory.exp
 load_lib check-test-names.exp
 
+# The path to the GDB binary to test.
 global GDB
 
+# The data directory to use for testing.  If this is the empty string,
+# then we let GDB use its own configured data directory.
+global GDB_DATA_DIRECTORY
+
 # The spawn ID used for I/O interaction with the inferior.  For native
 # targets, or remote targets that can do I/O through GDB
 # (semi-hosting) this will be the same as the host/GDB's spawn ID.
@@ -114,6 +119,14 @@ if ![info exists GDB] {
 }
 verbose "using GDB = $GDB" 2
 
+# The data directory the testing GDB will use.  By default, assume
+# we're testing a non-installed GDB in the build directory.  Users may
+# also explictly override the -data-directory from the command line.
+if ![info exists GDB_DATA_DIRECTORY] {
+    set GDB_DATA_DIRECTORY "[pwd]/../data-directory"
+}
+verbose "using GDB_DATA_DIRECTORY = $GDB_DATA_DIRECTORY" 2
+
 # GDBFLAGS is available for the user to set on the command line.
 # E.g. make check RUNTESTFLAGS=GDBFLAGS=mumble
 # Testcases may use it to add additional flags, but they must:
@@ -125,23 +138,35 @@ if ![info exists GDBFLAGS] {
 }
 verbose "using GDBFLAGS = $GDBFLAGS" 2
 
-# Make the build data directory available to tests.
-set BUILD_DATA_DIRECTORY "[pwd]/../data-directory"
+# Append the -data-directory option to pass to GDB to CMDLINE and
+# return the resulting string.  If GDB_DATA_DIRECTORY is empty,
+# nothing is appended.
+proc append_gdb_data_directory_option {cmdline} {
+    global GDB_DATA_DIRECTORY
+
+    if { $GDB_DATA_DIRECTORY != "" } {
+	return "$cmdline -data-directory $GDB_DATA_DIRECTORY"
+    } else {
+	return $cmdline
+    }
+}
 
 # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
 # `-nw' disables any of the windowed interfaces.
 # `-nx' disables ~/.gdbinit, so that it doesn't interfere with the tests.
-# `-data-directory' points to the data directory in the build directory.
 # `-iex "set {height,width} 0"' disables pagination.
+# `-data-directory' points to the data directory, usually in the build
+# directory.
 global INTERNAL_GDBFLAGS
 if ![info exists INTERNAL_GDBFLAGS] {
     set INTERNAL_GDBFLAGS \
 	[join [list \
 		   "-nw" \
 		   "-nx" \
-		   "-data-directory $BUILD_DATA_DIRECTORY" \
 		   {-iex "set height 0"} \
 		   {-iex "set width 0"}]]
+
+    set INTERNAL_GDBFLAGS [append_gdb_data_directory_option $INTERNAL_GDBFLAGS]
 }
 
 # The variable gdb_prompt is a regexp which matches the gdb prompt.
@@ -8021,9 +8046,9 @@ proc verify_psymtab_expanded { filename readin } {
 # string (the default) means .gdb_index; "-dwarf-5" means .debug_names.
 
 proc add_gdb_index { program {style ""} } {
-    global srcdir GDB env BUILD_DATA_DIRECTORY
+    global srcdir GDB env
     set contrib_dir "$srcdir/../contrib"
-    set env(GDB) "$GDB --data-directory=$BUILD_DATA_DIRECTORY"
+    set env(GDB) [append_gdb_data_directory_option $GDB]
     set result [catch "exec $contrib_dir/gdb-add-index.sh $style $program" output]
     if { $result != 0 } {
 	verbose -log "result is $result"

base-commit: 0a30596cfad9cd221a81eea984b6fe3fabb20b95
-- 
2.26.2