[v9] Add a test for the gcore script

Message ID 20240715132653.3172006-1-ahajkova@khirnov.net
State New
Headers
Series [v9] Add a test for the gcore script |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Alexandra Hájková July 15, 2024, 1:24 p.m. UTC
  From: Alexandra Hájková <ahajkova@redhat.com>

It also tests the gcore script being run without its accessible
terminal.

This test was written by Jan Kratochvil a long time ago. I modernized
the test making it use various procs from lib/gdb.exp, reorganizing it
and added some comments.

Modify the gcore script to make it possible to pass the --data-directory to
it. This prevents a lot of these warnings:

Python Exception <class 'AttributeError'>: module 'gdb' has no attribute
'_handle_missing_debuginfo'

Tested by using make check-all-boards.

Co-Authored-By: Jan Kratochvil <jan.kratochvil@redhat.com>

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
v9:
- rename the variable path to directory
- remove the dead code
- unify the strigs for pass and fail
- improve setting GCORE variable
- add a comment for has_gcore_script proc

 gdb/doc/gdb.texinfo                |  6 ++-
 gdb/gcore.in                       | 12 +++--
 gdb/testsuite/gdb.base/gcorebg.c   | 76 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/gcorebg.exp | 81 ++++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp          | 17 +++++++
 5 files changed, 188 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/gcorebg.c
 create mode 100644 gdb/testsuite/gdb.base/gcorebg.exp
  

Comments

Eli Zaretskii July 15, 2024, 2:33 p.m. UTC | #1
> From: Alexandra Hájková <ahajkova@khirnov.net>
> Cc: ahajkova@redhat.com,
> 	Jan Kratochvil <jan.kratochvil@redhat.com>,
> 	Eli Zaretskii <eliz@gnu.org>
> Date: Mon, 15 Jul 2024 15:24:11 +0200
> 
> From: Alexandra Hájková <ahajkova@redhat.com>
> 
> It also tests the gcore script being run without its accessible
> terminal.
> 
> This test was written by Jan Kratochvil a long time ago. I modernized
> the test making it use various procs from lib/gdb.exp, reorganizing it
> and added some comments.
> 
> Modify the gcore script to make it possible to pass the --data-directory to
> it. This prevents a lot of these warnings:
> 
> Python Exception <class 'AttributeError'>: module 'gdb' has no attribute
> '_handle_missing_debuginfo'
> 
> Tested by using make check-all-boards.
> 
> Co-Authored-By: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> ---
> v9:
> - rename the variable path to directory
> - remove the dead code
> - unify the strigs for pass and fail
> - improve setting GCORE variable
> - add a comment for has_gcore_script proc
> 
>  gdb/doc/gdb.texinfo                |  6 ++-
>  gdb/gcore.in                       | 12 +++--
>  gdb/testsuite/gdb.base/gcorebg.c   | 76 ++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/gcorebg.exp | 81 ++++++++++++++++++++++++++++++
>  gdb/testsuite/lib/gdb.exp          | 17 +++++++
>  5 files changed, 188 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/gcorebg.c
>  create mode 100644 gdb/testsuite/gdb.base/gcorebg.exp

Thanks, the gdb.texinfo part is okay.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  
Andrew Burgess July 29, 2024, 1:31 p.m. UTC | #2
Alexandra,

Thanks for working on this.  I have two really minor nits, but then I
think this can be merged.  No need to repost.

Alexandra Hájková <ahajkova@khirnov.net> writes:

> diff --git a/gdb/testsuite/gdb.base/gcorebg.exp b/gdb/testsuite/gdb.base/gcorebg.exp
> new file mode 100644
> index 00000000000..0e5908f1fd1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/gcorebg.exp
> @@ -0,0 +1,81 @@
> +# Copyright 2007-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/>.
> +#
> +# This is a test for the gcore script (not the gcore command from
> +# inside GDB).  It also tests the gcore script being run without its
> +# accessible terminal.
> +
> +standard_testfile
> +require {!is_remote host}
> +require {!is_remote target}
> +require has_gcore_script
> +require can_spawn_for_attach
> +
> +set corefile [standard_output_file ${testfile}.core]
> +
> +if {[build_executable "failed to build" $testfile ${srcfile}] == -1 } {
> +     return -1
> +}
> +
> +# Cleanup.
> +
> +proc core_clean {} {
> +    global corefile
> +
> +    foreach file [glob -nocomplain [join [list $corefile *] ""]] {
> +	verbose "Delete file $file" 1
> +	remote_file target delete $file
> +    }
> +}
> +core_clean
> +
> +# Generate the core file.
> +proc test_body { detached } {
> +    global binfile
> +    global GCORE
> +    global corefile
> +    global GDB_DATA_DIRECTORY
> +
> +    # We can't use gdb_test_multiple here because GDB is not started.
> +    set res [remote_spawn target "$binfile $detached $GCORE $GDB_DATA_DIRECTORY $corefile"]
> +    if { ![gdb_assert { ![expr {$res < 0 || $res == ""}] } \
> +	"spawned gcore"] } {
> +            return
> +	}
> +
> +    set saw_corefile_created false
> +    set testname "Spawned gcore finished"
> +    remote_expect target 20 {
> +	timeout {
> +	    fail "$testname (timeout)"
> +	    remote_exec target "kill -9 -[exp_pid -i $res]"
> +	    return
> +	}
> +	-re "Saved corefile \[^\r\n\]+\r\n" {
> +	    set saw_corefile_created true
> +	    exp_continue
> +	}
> +	eof {
> +	    gdb_assert { $saw_corefile_created } $testname
> +	}
> +    }
> +
> +    gdb_assert {1 == [llength [glob -nocomplain [join [list $corefile *] ""]]]} "Core file generated by gcore"

This line is a little long, could you wrap it before the test name
please:

    gdb_assert {1 == [llength [glob -nocomplain [join [list $corefile *] ""]]]} \
	"Core file generated by gcore"

that's still a little over 80 characters, but it's better than it was.


> +    core_clean
> +}
> +
> +foreach_with_prefix detached { detached standard } {
> +    test_body $detached
> +}
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index dfe19c9410d..a3bdeba3e7a 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -158,6 +158,23 @@ load_lib gdb-utils.exp
>  load_lib memory.exp
>  load_lib check-test-names.exp
>  
> +# The path to the GCORE script to test.
> +global GCORE
> +if {![info exists GCORE]} {
> +    set GCORE [findfile $base_dir/../../gdb/gcore]
> +}
> +verbose "using GCORE = $GCORE" 2
> +
> +# Return 0 if the gcore scipt is missing

Missing '.' at the end of this sentence.

With these two issues fixed, please go ahead and push this.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 86cd420832a..0f64c43b170 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -51011,7 +51011,7 @@  Richard M. Stallman and Roland H. Pesch, July 1991.
 
 @format
 @c man begin SYNOPSIS gcore
-gcore [-a] [-o @var{prefix}] @var{pid1} [@var{pid2}...@var{pidN}]
+gcore [-a] [-o @var{prefix}] [-d @var{directory}] @var{pid1} [@var{pid2}...@var{pidN}]
 @c man end
 @end format
 
@@ -51039,6 +51039,10 @@  when composing the file names of the core dumps.  The file name is
 composed as @file{@var{prefix}.@var{pid}}, where @var{pid} is the
 process ID of the running program being analyzed by @command{gcore}.
 If not specified, @var{prefix} defaults to @var{gcore}.
+
+@item -d @var{directory}
+Use @var{directory} as the data directory when invoking @value{GDBN} for running
+the gcore command. This argument is optional.
 @end table
 @c man end
 
diff --git a/gdb/gcore.in b/gdb/gcore.in
index 982c854eb70..0c40e5a54cd 100644
--- a/gdb/gcore.in
+++ b/gdb/gcore.in
@@ -27,7 +27,9 @@  prefix=core
 # to ensure gdb dumps all mappings (OS dependent).
 dump_all_cmds=()
 
-while getopts :ao: opt; do
+data_directory_opt=()
+
+while getopts :ao:d: opt; do
     case "$opt" in
         a)
             case "$OSTYPE" in
@@ -40,8 +42,11 @@  while getopts :ao: opt; do
         o)
             prefix=$OPTARG
             ;;
+        d)
+            data_directory_opt=("--data-directory" "$OPTARG")
+            ;;
         *)
-            echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o prefix] pid1 [pid2...pidN]"
+            echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o prefix] [-d data-directory] pid1 [pid2...pidN]"
             exit 2
             ;;
     esac
@@ -51,7 +56,7 @@  shift $((OPTIND-1))
 
 if [ "$#" -eq "0" ]
 then
-    echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o prefix] pid1 [pid2...pidN]"
+    echo "usage:  @GCORE_TRANSFORM_NAME@ [-a] [-o prefix] [-d data-directory] pid1 [pid2...pidN]"
     exit 2
 fi
 
@@ -98,6 +103,7 @@  do
 	# `</dev/null' to avoid touching interactive terminal if it is
 	# available but not accessible as GDB would get stopped on SIGTTIN.
 	"$binary_path/@GDB_TRANSFORM_NAME@" </dev/null \
+            "${data_directory_opt[@]}" \
 	    --nx --batch --readnever -iex 'set debuginfod enabled off' \
 	    -ex "set pagination off" -ex "set height 0" -ex "set width 0" \
 	    "${dump_all_cmds[@]}" \
diff --git a/gdb/testsuite/gdb.base/gcorebg.c b/gdb/testsuite/gdb.base/gcorebg.c
new file mode 100644
index 00000000000..9b585b500db
--- /dev/null
+++ b/gdb/testsuite/gdb.base/gcorebg.c
@@ -0,0 +1,76 @@ 
+/* Copyright 2007-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/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <sys/wait.h>
+#include <errno.h>
+
+/* Expects 4 arguments:
+
+   1. Either 'standard' or 'detached', where 'standard' tests
+   a general gcore script spawn with its controlling terminal available
+   and 'detached' tests gcore script spawn without its controlling
+   terminal available.
+   2. Path to the gcore script.
+   3. Path to the data-directory to pass to the gcore script.
+   4. The core file output name.  */
+
+int
+main (int argc, char **argv)
+{
+  pid_t pid = 0;
+  pid_t ppid;
+  char buf[1024*2 + 500];
+  int gotint, res;
+
+  assert (argc == 5);
+
+  pid = fork ();
+
+  switch (pid)
+    {
+    case 0:
+      if (strcmp (argv[1], "detached") == 0)
+	setpgrp ();
+      ppid = getppid ();
+      gotint = snprintf (buf, sizeof (buf), "%s -d %s -o %s %d",
+			 argv[2], argv[3], argv[4], (int) ppid);
+      assert (gotint < sizeof (buf));
+      res = system (buf);
+      assert (res != -1);
+      break;
+
+    case -1:
+      perror ("fork err\n");
+      exit (1);
+      break;
+
+    default:
+      do
+	{
+	  res = waitpid (pid, NULL, 0);
+	}
+      while (res == -1 && errno == EINTR);
+
+      assert (res == pid);
+      break;
+    }
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/gcorebg.exp b/gdb/testsuite/gdb.base/gcorebg.exp
new file mode 100644
index 00000000000..0e5908f1fd1
--- /dev/null
+++ b/gdb/testsuite/gdb.base/gcorebg.exp
@@ -0,0 +1,81 @@ 
+# Copyright 2007-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/>.
+#
+# This is a test for the gcore script (not the gcore command from
+# inside GDB).  It also tests the gcore script being run without its
+# accessible terminal.
+
+standard_testfile
+require {!is_remote host}
+require {!is_remote target}
+require has_gcore_script
+require can_spawn_for_attach
+
+set corefile [standard_output_file ${testfile}.core]
+
+if {[build_executable "failed to build" $testfile ${srcfile}] == -1 } {
+     return -1
+}
+
+# Cleanup.
+
+proc core_clean {} {
+    global corefile
+
+    foreach file [glob -nocomplain [join [list $corefile *] ""]] {
+	verbose "Delete file $file" 1
+	remote_file target delete $file
+    }
+}
+core_clean
+
+# Generate the core file.
+proc test_body { detached } {
+    global binfile
+    global GCORE
+    global corefile
+    global GDB_DATA_DIRECTORY
+
+    # We can't use gdb_test_multiple here because GDB is not started.
+    set res [remote_spawn target "$binfile $detached $GCORE $GDB_DATA_DIRECTORY $corefile"]
+    if { ![gdb_assert { ![expr {$res < 0 || $res == ""}] } \
+	"spawned gcore"] } {
+            return
+	}
+
+    set saw_corefile_created false
+    set testname "Spawned gcore finished"
+    remote_expect target 20 {
+	timeout {
+	    fail "$testname (timeout)"
+	    remote_exec target "kill -9 -[exp_pid -i $res]"
+	    return
+	}
+	-re "Saved corefile \[^\r\n\]+\r\n" {
+	    set saw_corefile_created true
+	    exp_continue
+	}
+	eof {
+	    gdb_assert { $saw_corefile_created } $testname
+	}
+    }
+
+    gdb_assert {1 == [llength [glob -nocomplain [join [list $corefile *] ""]]]} "Core file generated by gcore"
+    core_clean
+}
+
+foreach_with_prefix detached { detached standard } {
+    test_body $detached
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index dfe19c9410d..a3bdeba3e7a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -158,6 +158,23 @@  load_lib gdb-utils.exp
 load_lib memory.exp
 load_lib check-test-names.exp
 
+# The path to the GCORE script to test.
+global GCORE
+if {![info exists GCORE]} {
+    set GCORE [findfile $base_dir/../../gdb/gcore]
+}
+verbose "using GCORE = $GCORE" 2
+
+# Return 0 if the gcore scipt is missing
+proc has_gcore_script {} {
+    global GCORE
+    if {$GCORE == ""} {
+	return 0
+    } else {
+	return 1
+    }
+}
+
 # The path to the GDB binary to test.
 global GDB