[v2] Add a test for the gcore script

  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, reoirganising it
and added some comments.

Tested by using make check-all-boards.

Co-Authored-By: Jan Kratochvil
---
v2: - cosmetics
    - Added Jan Kratochvil as the Co-Author
    - added asserts
    - added more comments
    - removed unnecessary code

 gdb/testsuite/gdb.base/gcorebg.c   | 68 +++++++++++++++++++++++++
 gdb/testsuite/gdb.base/gcorebg.exp | 81 ++++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp          | 17 +++++++
 3 files changed, 166 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/gcorebg.c
 create mode 100644 gdb/testsuite/gdb.base/gcorebg.exp
  

Alexandra Hájková <ahajkova@redhat.com> writes:

> 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, reoirganising it
> and added some comments.
>
> Tested by using make check-all-boards.
>
> Co-Authored-By: Jan Kratochvil
> ---
> v2: - cosmetics
>     - Added Jan Kratochvil as the Co-Author
>     - added asserts
>     - added more comments
>     - removed unnecessary code
>
>  gdb/testsuite/gdb.base/gcorebg.c   | 68 +++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/gcorebg.exp | 81 ++++++++++++++++++++++++++++++
>  gdb/testsuite/lib/gdb.exp          | 17 +++++++
>  3 files changed, 166 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/gcorebg.c
>  create mode 100644 gdb/testsuite/gdb.base/gcorebg.exp
>
> diff --git a/gdb/testsuite/gdb.base/gcorebg.c b/gdb/testsuite/gdb.base/gcorebg.c
> new file mode 100644
> index 00000000000..0d5168c63ad
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/gcorebg.c
> @@ -0,0 +1,68 @@
> +/* 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 <sys/types.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <assert.h>
> +
> +int
> +main (int argc, char **argv)
> +{
> +  pid_t pid = 0;
> +  pid_t ppid;
> +  char buf[1024*2 + 500];
> +  int gotint, res;
> +
> +  if (argc != 4)
> +    {
> +      fprintf (stderr, "Syntax: %s {standard|detached} <gcore command> <core output file>\n",
> +	       argv[0]);
> +      exit (1);
> +    }
> +
> +  pid = fork ();
> +
> +  switch (pid)
> +    {
> +      case 0:
> +	if (strcmp (argv[1], "detached") == 0)
> +	  setpgrp ();
> +	ppid = getppid ();
> +	gotint = snprintf (buf, sizeof (buf), "sh %s -o %s %d", argv[2], argv[3], (int) ppid);
> +	assert (gotint < sizeof (buf));
> +	res = system (buf);
> +	assert (res != -1);
> +	fprintf (stderr, "Killing parent PID %d\n", ppid);
> +	kill (ppid, SIGTERM);
> +	break;
> +
> +      case -1:
> +	perror ("fork err\n");
> +	exit (1);
> +	break;
> +
> +      default:
> +	fprintf (stderr, "Sleeping as PID %d\n", getpid ());
> +	/* Wait here until the child is done with gcore-ing us.  If things go right,
> +	   the child kills us with SIGTERM before this sleep returns.  */
> +	sleep (60);
> +    }
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/gcorebg.exp b/gdb/testsuite/gdb.base/gcorebg.exp
> new file mode 100644
> index 00000000000..bcfbe994c9e
> --- /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 form
> +# 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
> +
> +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
> +
> +    with_test_prefix "detached = $detached" {
> +	# We can't use gdb_test_multiple here because GDB is not started.
> +	set res [remote_spawn target "$binfile $detached $GCORE $corefile"]
> +	if { $res < 0 || $res == "" } {
> +	    fail "Spawning gcore"
> +	    return 1
> +	}
> +	pass "Spawned gcore"
> +
> +	remote_expect target 20 {
> +	    timeout {
> +		fail "Spawned gcore finished (timeout)"
> +		remote_exec target "kill -9 -[exp_pid -i $res]"
> +		return 1
> +	    }
> +	    "Saved corefile .*\r\nKilling parent PID " {
> +		pass "Spawned gcore finished"
> +	    }
> +	}
> +
> +	gdb_assert {1 == [llength [glob -nocomplain [join [list $corefile *] ""]]]} "Core file generated by gcore"
> +	core_clean
> +    }
> +}
> +
> +# First a general gcore script spawn with its controlling terminal available.
> +
> +test_body standard
> +
> +# And now gcore script spawn without its controlling terminal available.
> +# It is spawned through `gcorebg.c' using setpgrp ().
> +
> +test_body detached
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 9a906f0f349..7943fcbbe74 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -146,6 +146,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/../../gcore]

This isn't going to find the gcore script in the build tree.  For that
you would need:

  set GCORE [findfile $base_dir/../../gdb/gcore]

instead what you have will just result in GCORE being set to 'gcore', so
we'll end up testing with whatever version of gcore happens to be
installed on the machine, which is likely not what we want (be default).

> +} else {
> +    set GCORE ""
> +}

Setting this means that we can't do:

  make check-gdb TESTS="..." RUNTESTFLAGS="GCORE=/path/to/gcore"

So I think this should be dropped.

> +verbose "using GCORE = $GCORE" 2
> +
> +proc has_gcore_script {} {

This proc should have a comment before it.

However, if you fix the above and start testing the local gcore script,
then you're going to spot a slightly bigger issue.  When you run gcore
it will correctly find the locally built gdb executable, but it doesn't
pass the --data-directory option, so we end up with a bunch of errors
from starting GDB.  Now this doesn't actually cause any FAILs from your
test, but I think we need to consider how to address this.

I don't have a specific plan in mind, but I do have a couple of ideas.

We could add a --data-directory=.... line to the built gcore script
file, like:

    "$binary_path/gdb" </dev/null \
        --data-directory "$binary_path/data-directory" \
        ... etc ...

but we'd then need to remove this before we actually installed
gcore, I'm not sure this is really an "approved" technique or not, so
this might not be well received.

We could add an option to the gcore script which would allow us to
override the data-directory setting, then instead of passing
--data-directory based on the binary we could use the actual real data
directory that GDB would use (i.e. the value of --with-gdb-datadir
configure option), but then we'd need to make sure that this directory
was relocatable in the same way that GDB uses this value, and that all
gets a little messy.

So maybe we add a data-directory option to the gcore script which is
only passed to GDB in the event that the option is specified.  This
seems much easier than the previous choice.

Or maybe we add a mechanism, either a command line option, or an
environment variable that allows the complete GDB command, including
options, to be overridden, then the test framework can just use that?

Or .... maybe there's something else that we could do?

Anyway, I think we need to figure this out before this test can be
merged.

Thanks,
Andrew



> +    if { $::GCORE != "" } {
> +	return 1
> +    } else {
> +	return 0
> +    }
> +}
> +
>  # The path to the GDB binary to test.
>  global GDB
>  
> -- 
> 2.43.0