[patch] Fix 'gcore' with exited threads

[patch] Fix 'gcore' with exited threads

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

Hi,

https://bugzilla.redhat.com/show_bug.cgi?id=1099405

Program received signal SIGABRT, Aborted.
[...]
(gdb) gcore foobar
Couldn't get registers: No such process.
(gdb) info threads 
[...]
(gdb) gcore foobar
Saved corefile foobar
(gdb) 

gcore tries to access the exited thread:
[Thread 0x7ffff7fce700 (LWP 6895) exited]
ptrace(PTRACE_GETREGS, 6895, 0, 0x7fff18167dd0) = -1 ESRCH (No such process)

Without the TRY_CATCH protection testsuite FAILs for:
	FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
	FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)
Maybe the TRY_CATCH could be more inside update_thread_list().

Similar update_thread_list() call is IMO missing in procfs_make_note_section()
but I do not have where to verify that change.


Jan

[-- Attachment #2: gcore-stale-thread.patch --]
[-- Type: text/plain, Size: 4519 bytes --]

gdb/
2014-06-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-tdep.c (linux_make_corefile_notes): call update_thread_list,
	protected against exceptions.

gdb/testsuite/
2014-06-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/gcore-stale-thread.c: New file.
	* gdb.threads/gcore-stale-thread.exp: New file.

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index c10b8ee..fc5a943 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1440,6 +1440,7 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
   char *note_data = NULL;
   gdb_byte *auxv;
   int auxv_len;
+  volatile struct gdb_exception e;
 
   if (linux_fill_prpsinfo (&prpsinfo))
     {
@@ -1463,6 +1464,12 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
     }
 
   /* Thread register information.  */
+  TRY_CATCH (e, RETURN_MASK_ERROR)
+    {
+      update_thread_list ();
+    }
+  if (e.reason < 0)
+    exception_print (gdb_stderr, e);
   thread_args.gdbarch = gdbarch;
   thread_args.pid = ptid_get_pid (inferior_ptid);
   thread_args.obfd = obfd;
diff --git a/gdb/testsuite/gdb.threads/gcore-stale-thread.c b/gdb/testsuite/gdb.threads/gcore-stale-thread.c
new file mode 100644
index 0000000..9fba7a7
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/gcore-stale-thread.c
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 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 <pthread.h>
+#include <assert.h>
+
+static void *
+start (void *arg)
+{
+  return arg;
+}
+
+int
+main (void)
+{
+  pthread_t thread;
+  int i;
+
+  i = pthread_create (&thread, NULL, start, NULL);
+  assert (i == 0);
+  i = pthread_join (thread, NULL);
+  assert (i == 0);
+
+  return 0; /* break-here */
+}
diff --git a/gdb/testsuite/gdb.threads/gcore-stale-thread.exp b/gdb/testsuite/gdb.threads/gcore-stale-thread.exp
new file mode 100644
index 0000000..a6677a8
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/gcore-stale-thread.exp
@@ -0,0 +1,56 @@
+# Copyright 2014 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 file was written by Michael Snyder (msnyder@redhat.com)
+# This is a test for the gdb command "generate-core-file".
+
+standard_testfile
+set corefile [standard_output_file ${testfile}.core]
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable debug] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test_multiple "help gcore" "help gcore" {
+    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
+	# gcore command not supported -- nothing to test here.
+	unsupported "gdb does not support gcore on this target"
+	return -1
+    }
+    -re "Save a core file .*\r\n$gdb_prompt $" {
+	pass "help gcore"
+    }
+}
+
+if { ! [ runto_main ] } then {
+    return -1
+}
+
+gdb_test_multiple "info threads" "threads are supported" {
+    -re ".* main .*\r\n$gdb_prompt $" {
+	# OK, threads are supported.
+    }
+    -re "\r\n$gdb_prompt $" {
+	unsupported "gdb does not support threads on this target"
+	return -1
+    }
+}
+
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+gdb_gcore_cmd "$corefile" "save a corefile"

Re: [patch] Fix 'gcore' with exited threads

[Pedro Alves]

On 06/09/2014 09:30 PM, Jan Kratochvil wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1099405
>
> Program received signal SIGABRT, Aborted.
> [...]
> (gdb) gcore foobar
> Couldn't get registers: No such process.
> (gdb) info threads
> [...]
> (gdb) gcore foobar
> Saved corefile foobar
> (gdb)
>
> gcore tries to access the exited thread:
> [Thread 0x7ffff7fce700 (LWP 6895) exited]
> ptrace(PTRACE_GETREGS, 6895, 0, 0x7fff18167dd0) = -1 ESRCH (No such process)

Note this will still happen if you have the exited thread selected,
as in that case the thread can't be deleted:

$ ./gdb ~/gdb/tests/threads -ex "set non-stop on"
...
(gdb) t 2
[Switching to thread 2 (Thread 0x7ffff7fc6700 (LWP 23009))]
#0  thread_function0 (arg=0x0) at threads.c:64
64              usleep (1);  /* Loop increment.  */
(gdb) p *myp=0
$1 = 0
(gdb) c&
Continuing.
(gdb) [Thread 0x7ffff7fc6700 (LWP 23009) exited]
(gdb) thread
[Current thread is 2 (Thread 0x7ffff7fc6700 (LWP 22973)) (exited)]
(gdb) info threads
  Id   Target Id         Frame
  3    Thread 0x7ffff77c5700 (LWP 22974) "threads" (running)
  1    Thread 0x7ffff7fc7740 (LWP 22972) "threads" (running)

The current thread <Thread ID 2> has terminated.  See `help thread'.
(gdb) gcore
Couldn't get registers: No such process.
(gdb)

It seems to me linux_corefile_thread_callback should skip exited
threads too.

> Without the TRY_CATCH protection testsuite FAILs for:
> 	FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
> 	FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)

What does the log show ?

> Maybe the TRY_CATCH could be more inside update_thread_list().

I'll assume "info threads" is failing at that point too
then.  Maybe we should downgrade whatever error is triggering
to a warning?

> Similar update_thread_list() call is IMO missing in procfs_make_note_section()
> but I do not have where to verify that change.

I wonder whether we should update the thread list in generic
code (write_gcore_file).

> +gdb_test_multiple "help gcore" "help gcore" {
> +    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {

Is this coming from copy/paste of existing tests?  I believe
this is is stale -- gcore.o has been in COMMON_OBS for a while
now.  I think the actual error will be whatever the default for
the target method throws.

> +	# gcore command not supported -- nothing to test here.
> +	unsupported "gdb does not support gcore on this target"
> +	return -1
> +    }
> +    -re "Save a core file .*\r\n$gdb_prompt $" {
> +	pass "help gcore"
> +    }
> +}

-- 
Pedro Alves

[patch+7.8?] Fix 'gcore' with exited threads

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 3671 bytes --]

On Mon, 23 Jun 2014 17:06:37 +0200, Pedro Alves wrote:
> On 06/09/2014 09:30 PM, Jan Kratochvil wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1099405
> >
> > Program received signal SIGABRT, Aborted.
> > [...]
> > (gdb) gcore foobar
> > Couldn't get registers: No such process.
> > (gdb) info threads
> > [...]
> > (gdb) gcore foobar
> > Saved corefile foobar
> > (gdb)
> >
> > gcore tries to access the exited thread:
> > [Thread 0x7ffff7fce700 (LWP 6895) exited]
> > ptrace(PTRACE_GETREGS, 6895, 0, 0x7fff18167dd0) = -1 ESRCH (No such process)
> 
> Note this will still happen if you have the exited thread selected,
> as in that case the thread can't be deleted:

Confirmed, testcase extended.


> It seems to me linux_corefile_thread_callback should skip exited
> threads too.

Done.


> > Without the TRY_CATCH protection testsuite FAILs for:
> > 	FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
> > 	FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)
> 
> What does the log show ?

gcore .../gdb/testsuite/gdb.threads/gcore-thread0.test^M
Cannot find new threads: debugger service failed^M
(gdb) FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
+
core .../gdb/testsuite/gdb.threads/gcore-thread0.test^M
".../gdb/testsuite/gdb.threads/gcore-thread0.test" is not a core dump: File format not recognized^M
(gdb) FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)


> > Maybe the TRY_CATCH could be more inside update_thread_list().
> 
> I'll assume "info threads" is failing at that point too
> then.  Maybe we should downgrade whatever error is triggering
> to a warning?

It has other disadvantages.  Warning may get lost in some other messages and
user may miss some thread(s) during a falsely successfully finished command.


> > Similar update_thread_list() call is IMO missing in procfs_make_note_section()
> > but I do not have where to verify that change.
> 
> I wonder whether we should update the thread list in generic
> code (write_gcore_file).

This is one of the general problems of GDB that one cannot do any change
affecting platforms which one cannot (or at least not easily enough) test on.

Platform not automatically being tested by Jenkins for any patch submitted to
Gerrit should be officially unsupported.


> > +gdb_test_multiple "help gcore" "help gcore" {
> > +    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
> 
> Is this coming from copy/paste of existing tests?

Yes.


> I believe this is is stale -- gcore.o has been in COMMON_OBS for a while
> now.  I think the actual error will be whatever the default for the target
> method throws.

Likewise two paragraphs above.  Not sure what I should do with it:
 * If I remove the test from this my testcase and someone later fixes
   the 6 existing instances of this test this only test will remain unfixed.
 * If I waste the time reading the sources what should be the expected output
   I risk the test will not work anyway as I read it wrongly.
   And I will probably wrongly modify 6 other testsuite cases.
 * I can try to find a platform without gcore but in most cases in the past my
   attempts to build GDB on any non-Linux platforms failed.

I find this problem generally out of the scope of this mail thread / patch.


> > +	# gcore command not supported -- nothing to test here.
> > +	unsupported "gdb does not support gcore on this target"
> > +	return -1
> > +    }
> > +    -re "Save a core file .*\r\n$gdb_prompt $" {
> > +	pass "help gcore"
> > +    }
> > +}


No regressions on {x86_64,x86_64-m32,i686}-fedora21pre-linux-gnu.


Thanks,
Jan

[-- Attachment #2: gcore-stale-thread2.patch --]
[-- Type: text/plain, Size: 5680 bytes --]

gdb/
2014-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-tdep.c (linux_corefile_thread_callback): Ignore THREAD_EXITED.
	(linux_make_corefile_notes): call update_thread_list, protected against
	exceptions.

gdb/testsuite/
2014-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/gcore-stale-thread.c: New file.
	* gdb.threads/gcore-stale-thread.exp: New file.

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d0f1106..875f32e 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1194,6 +1194,11 @@ linux_corefile_thread_callback (struct thread_info *info, void *data)
 {
   struct linux_corefile_thread_data *args = data;
 
+  /* It can be current thread in non-stop mode
+     which cannot be removed by update_thread_list.  */
+  if (info->state == THREAD_EXITED)
+    return 0;
+
   if (ptid_get_pid (info->ptid) == args->pid)
     {
       struct cleanup *old_chain;
@@ -1445,6 +1450,7 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
   char *note_data = NULL;
   gdb_byte *auxv;
   int auxv_len;
+  volatile struct gdb_exception e;
 
   if (linux_fill_prpsinfo (&prpsinfo))
     {
@@ -1468,6 +1474,12 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
     }
 
   /* Thread register information.  */
+  TRY_CATCH (e, RETURN_MASK_ERROR)
+    {
+      update_thread_list ();
+    }
+  if (e.reason < 0)
+    exception_print (gdb_stderr, e);
   thread_args.gdbarch = gdbarch;
   thread_args.pid = ptid_get_pid (inferior_ptid);
   thread_args.obfd = obfd;
diff --git a/gdb/testsuite/gdb.threads/gcore-stale-thread.c b/gdb/testsuite/gdb.threads/gcore-stale-thread.c
new file mode 100644
index 0000000..944acba
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/gcore-stale-thread.c
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 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 <pthread.h>
+#include <assert.h>
+
+static pthread_t main_thread;
+
+static void *
+start (void *arg)
+{
+  int i;
+
+  i = pthread_join (main_thread, NULL);
+  assert (i == 0);
+
+  return arg; /* break-here */
+}
+
+int
+main (void)
+{
+  pthread_t thread;
+  int i;
+
+  main_thread = pthread_self ();
+
+  i = pthread_create (&thread, NULL, start, NULL);
+  assert (i == 0);
+
+  pthread_exit (NULL);
+  assert (0);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/gcore-stale-thread.exp b/gdb/testsuite/gdb.threads/gcore-stale-thread.exp
new file mode 100644
index 0000000..dc73056
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/gcore-stale-thread.exp
@@ -0,0 +1,71 @@
+# Copyright 2014 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 file was written by Michael Snyder (msnyder@redhat.com)
+# This is a test for the gdb command "generate-core-file".
+
+standard_testfile
+set corefile [standard_output_file ${testfile}.core]
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable debug] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test_no_output "set non-stop on"
+
+gdb_test_multiple "help gcore" "help gcore" {
+    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
+	# gcore command not supported -- nothing to test here.
+	unsupported "gdb does not support gcore on this target"
+	return -1
+    }
+    -re "Save a core file .*\r\n$gdb_prompt $" {
+	pass "help gcore"
+    }
+}
+
+if { ! [ runto_main ] } then {
+    return -1
+}
+
+gdb_test_multiple "info threads" "threads are supported" {
+    -re ".* main .*\r\n$gdb_prompt $" {
+	# OK, threads are supported.
+    }
+    -re "\r\n$gdb_prompt $" {
+	unsupported "gdb does not support threads on this target"
+	return -1
+    }
+}
+
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "break-here"]
+# gdb_continue_to_breakpoint does not work as it uses "$gdb_prompt $" regex
+# which does not work due to the output: (gdb) [Thread ... exited]
+set name "continue to breakpoint: break-here"
+gdb_test_multiple "continue" $name {
+    -re "Breakpoint .* (at|in) .* break-here .*\r\n$gdb_prompt " {
+	pass $name
+    }
+}
+
+gdb_gcore_cmd "$corefile" "save a corefile"
+
+# Do not run "info threads" before "gcore" as it could workaround the bug
+# by discarding non-current exited threads.
+gdb_test "info threads" \
+         {The current thread <Thread ID 1> has terminated\.  See `help thread'\.} \
+	 "exited thread is current due to non-stop"

[PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads)

[Pedro Alves]

On 08/17/2014 10:16 PM, Jan Kratochvil wrote:

>>> > > +gdb_test_multiple "help gcore" "help gcore" {
>>> > > +    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
>> > 
>> > Is this coming from copy/paste of existing tests?
> Yes.
> 
> 
>> > I believe this is is stale -- gcore.o has been in COMMON_OBS for a while
>> > now.  I think the actual error will be whatever the default for the target
>> > method throws.
> Likewise two paragraphs above.  Not sure what I should do with it:
>  * If I remove the test from this my testcase and someone later fixes
>    the 6 existing instances of this test this only test will remain unfixed.
>  * If I waste the time reading the sources what should be the expected output
>    I risk the test will not work anyway as I read it wrongly.
>    And I will probably wrongly modify 6 other testsuite cases.
>  * I can try to find a platform without gcore but in most cases in the past my
>    attempts to build GDB on any non-Linux platforms failed.

No no, you misunderstand, we should just delete it completely.
It's a useless test.  See patch below.

If we happened to still need it, then the right solution would be to factor
out all those 6 instances into a procedure in gdb.exp that test call
('supports_gcore' or some such), so we only had a single place to maintain,
instead of piling on more.

> I find this problem generally out of the scope of this mail thread / patch.

It's not out of scope as the new test was adding copy/pasted useless code.

I've pushed the patch below.

---------
[PATCH] Remove useless gcore command detection

Checking whether the gcore command is included in the GDB build as
proxy for checking whether core dumping is supported by the target is
useless, as gcore.o has been in COMMON_OBS since git 9b4eba8e:

    2009-10-26  Michael Snyder  <msnyder@vmware.com>
                Hui Zhu  <teawater@gmail.com>

        * Makefile.in (SFILES): Add gcore.c.
        (COMMON_OBS): Add gcore.o.
        * config/alpha/alpha-linux.mh (NATDEPFILES): Delete gcore.o.
        * config/alpha/fbsd.mh (NATDEPFILES): Ditto.
	...

IOW, the command is always included in the build.

Instead, nowadays, tests bail out if actually trying to generate a
core fails with an indication the target doesn't support it.  See
gdb_gcore_cmd and callers.

Tested on x86_64 Fedora 20.

gdb/testsuite/ChangeLog:

	* gdb.base/gcore-buffer-overflow.exp: Remove "help gcore" test.
	* gdb.base/gcore-relro-pie.exp: Likewise.
	* gdb.base/gcore-relro.exp: Likewise.
	* gdb.base/gcore.exp: Likewise.
	* gdb.base/print-symbol-loading.exp: Likewise.
	* gdb.threads/gcore-thread.exp: Likewise.
	* lib/gdb.exp (gdb_gcore_cmd): Don't expect "Undefined command".
---
 gdb/testsuite/ChangeLog                          | 10 ++++++++++
 gdb/testsuite/gdb.base/gcore-buffer-overflow.exp | 12 ------------
 gdb/testsuite/gdb.base/gcore-relro-pie.exp       | 13 -------------
 gdb/testsuite/gdb.base/gcore-relro.exp           | 13 -------------
 gdb/testsuite/gdb.base/gcore.exp                 | 12 ------------
 gdb/testsuite/gdb.base/print-symbol-loading.exp  | 13 -------------
 gdb/testsuite/gdb.threads/gcore-thread.exp       | 11 -----------
 gdb/testsuite/lib/gdb.exp                        |  6 ------
 8 files changed, 10 insertions(+), 80 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index a9205d1..31c64ff 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,13 @@
+2014-08-21  Pedro Alves  <palves@redhat.com>
+
+	* gdb.base/gcore-buffer-overflow.exp: Remove "help gcore" test.
+	* gdb.base/gcore-relro-pie.exp: Likewise.
+	* gdb.base/gcore-relro.exp: Likewise.
+	* gdb.base/gcore.exp: Likewise.
+	* gdb.base/print-symbol-loading.exp: Likewise.
+	* gdb.threads/gcore-thread.exp: Likewise.
+	* lib/gdb.exp (gdb_gcore_cmd): Don't expect "Undefined command".
+
 2014-08-20  Pedro Alves  <palves@redhat.com>
 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
 
diff --git a/gdb/testsuite/gdb.base/gcore-buffer-overflow.exp b/gdb/testsuite/gdb.base/gcore-buffer-overflow.exp
index 1951301..f7a69ba 100644
--- a/gdb/testsuite/gdb.base/gcore-buffer-overflow.exp
+++ b/gdb/testsuite/gdb.base/gcore-buffer-overflow.exp
@@ -31,18 +31,6 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
 
 clean_restart ${binfile}
 
-# Does this gdb support gcore?
-gdb_test_multiple "help gcore" "help gcore" {
-    -re "Undefined command: .gcore.*$gdb_prompt $" {
-	# gcore command not supported -- nothing to test here.
-	unsupported "gdb does not support gcore on this target"
-	return -1
-    }
-    -re "Save a core file .*$gdb_prompt $" {
-	pass "help gcore"
-    }
-}
-
 gdb_test_no_output "set args ${pattern}"	\
     "Set buffer exceeding arguments"
 
diff --git a/gdb/testsuite/gdb.base/gcore-relro-pie.exp b/gdb/testsuite/gdb.base/gcore-relro-pie.exp
index e0c0de0..468e846 100644
--- a/gdb/testsuite/gdb.base/gcore-relro-pie.exp
+++ b/gdb/testsuite/gdb.base/gcore-relro-pie.exp
@@ -39,19 +39,6 @@ file attributes ${stripped_binfile} -permissions $perm
 
 clean_restart ${stripped_binfile}
 
-# Does this gdb support gcore?
-set test "help gcore"
-gdb_test_multiple $test $test {
-    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
-	# gcore command not supported -- nothing to test here.
-	unsupported "gdb does not support gcore on this target"
-	return -1
-    }
-    -re "Save a core file .*\r\n$gdb_prompt $" {
-	pass $test
-    }
-}
-
 # The binary is stripped of debug info, but not minsyms.
 if ![runto break_here] {
     fail "Can't run to break_here"
diff --git a/gdb/testsuite/gdb.base/gcore-relro.exp b/gdb/testsuite/gdb.base/gcore-relro.exp
index 8278de2..d5f4b69 100644
--- a/gdb/testsuite/gdb.base/gcore-relro.exp
+++ b/gdb/testsuite/gdb.base/gcore-relro.exp
@@ -38,19 +38,6 @@ set objfile [standard_output_file ${testfile}.o]
 clean_restart ${binfile}
 gdb_load_shlibs ${binfile_lib}
 
-# Does this gdb support gcore?
-set test "help gcore"
-gdb_test_multiple $test $test {
-    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
-	# gcore command not supported -- nothing to test here.
-	unsupported "gdb does not support gcore on this target"
-	return -1
-    }
-    -re "Save a core file .*\r\n$gdb_prompt $" {
-	pass $test
-    }
-}
-
 if ![runto lib] {
     return -1
 }
diff --git a/gdb/testsuite/gdb.base/gcore.exp b/gdb/testsuite/gdb.base/gcore.exp
index c28a9b3..15de624 100644
--- a/gdb/testsuite/gdb.base/gcore.exp
+++ b/gdb/testsuite/gdb.base/gcore.exp
@@ -24,18 +24,6 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
     return -1
 }
 
-# Does this gdb support gcore?
-gdb_test_multiple "help gcore" "help gcore" {
-    -re "Undefined command: .gcore.*$gdb_prompt $" {
-	# gcore command not supported -- nothing to test here.
-	unsupported "gdb does not support gcore on this target"
-	return -1
-    }
-    -re "Save a core file .*$gdb_prompt $" {
-	pass "help gcore"
-    }
-}
-
 if { ! [ runto_main ] } then {
     untested gcore.exp
     return -1
diff --git a/gdb/testsuite/gdb.base/print-symbol-loading.exp b/gdb/testsuite/gdb.base/print-symbol-loading.exp
index 1abfa2a..0686d0e 100644
--- a/gdb/testsuite/gdb.base/print-symbol-loading.exp
+++ b/gdb/testsuite/gdb.base/print-symbol-loading.exp
@@ -40,19 +40,6 @@ if { [gdb_compile ${objfile} ${binfile} executable $opts] != "" } {
 clean_restart ${binfile}
 gdb_load_shlibs ${binfile_lib}
 
-# Does this gdb support gcore?
-set test "help gcore"
-gdb_test_multiple $test $test {
-    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
-	# gcore command not supported -- nothing to test here.
-	unsupported "gdb does not support gcore on this target"
-	return -1
-    }
-    -re "Save a core file .*\r\n$gdb_prompt $" {
-	pass $test
-    }
-}
-
 if ![runto lib] {
     return -1
 }
diff --git a/gdb/testsuite/gdb.threads/gcore-thread.exp b/gdb/testsuite/gdb.threads/gcore-thread.exp
index 6e0e81e..3014de9 100644
--- a/gdb/testsuite/gdb.threads/gcore-thread.exp
+++ b/gdb/testsuite/gdb.threads/gcore-thread.exp
@@ -55,17 +55,6 @@ set nl "\[\r\n\]+"
 
 set timeout 30
 
-gdb_test_multiple "help gcore" "help gcore" {
-    -re "Undefined command: .gcore.*$gdb_prompt $" {
-	# gcore command not supported -- nothing to test here.
-	unsupported "gdb does not support gcore on this target"
-	return -1
-    }
-    -re "Save a core file .*$gdb_prompt $" {
-	pass "help gcore"
-    }
-}
-
 if { ! [ runto_main ] } then {
     untested gcore-thread.exp
     return -1
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 92069c9..61e1614 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3401,12 +3401,6 @@ proc gdb_gcore_cmd {core test} {
 	    pass $test
 	    set result 1
 	}
-
-	-re "Undefined command.*$gdb_prompt $" {
-	    unsupported $test
-	    verbose -log "'gcore' command undefined in gdb_gcore_cmd"
-	}
-
 	-re "(?:Can't create a corefile|Target does not support core file generation\\.)\[\r\n\]+$gdb_prompt $" {
 	    unsupported $test
 	}
-- 
1.9.3

Re: [patch+7.8?] Fix 'gcore' with exited threads

[Pedro Alves]

On 08/17/2014 10:16 PM, Jan Kratochvil wrote:
> On Mon, 23 Jun 2014 17:06:37 +0200, Pedro Alves wrote:
>> > On 06/09/2014 09:30 PM, Jan Kratochvil wrote:

> 
>>> > > Without the TRY_CATCH protection testsuite FAILs for:
>>> > > 	FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
>>> > > 	FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)
>> > 
>> > What does the log show ?
> gcore .../gdb/testsuite/gdb.threads/gcore-thread0.test^M
> Cannot find new threads: debugger service failed^M
> (gdb) FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
> +
> core .../gdb/testsuite/gdb.threads/gcore-thread0.test^M
> ".../gdb/testsuite/gdb.threads/gcore-thread0.test" is not a core dump: File format not recognized^M
> (gdb) FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)

Thanks.  It'd be good to have this in the commit log.

> gdb/
> 2014-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* linux-tdep.c (linux_corefile_thread_callback): Ignore THREAD_EXITED.
> 	(linux_make_corefile_notes): call update_thread_list, protected against
> 	exceptions.
> 
> gdb/testsuite/
> 2014-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.threads/gcore-stale-thread.c: New file.
> 	* gdb.threads/gcore-stale-thread.exp: New file.
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index d0f1106..875f32e 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -1194,6 +1194,11 @@ linux_corefile_thread_callback (struct thread_info *info, void *data)
>  {
>    struct linux_corefile_thread_data *args = data;
>  
> +  /* It can be current thread in non-stop mode
> +     which cannot be removed by update_thread_list.  */

Please remove the "in non-stop mode" bit.  The predicate GDB
currently chooses to decide whether to switch to another thread
or not is "non-stop", but that's bogus and will change (it should
be foreground vs background command), and then this comment will
most likely end up stale.

> +# This file was written by Michael Snyder (msnyder@redhat.com)
> +# This is a test for the gdb command "generate-core-file".

This paragraph is stale.

Otherwise OK.

Thanks!

IMO, this is good for 7.8.

Pedro Alves

Re: [PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads)

[Jan Kratochvil]

On Thu, 21 Aug 2014 12:41:12 +0200, Pedro Alves wrote:
> On 08/17/2014 10:16 PM, Jan Kratochvil wrote:
> >> > I believe this is is stale -- gcore.o has been in COMMON_OBS for a while
> >> > now.  I think the actual error will be whatever the default for the target
> >> > method throws.
> > Likewise two paragraphs above.  Not sure what I should do with it:
> >  * If I remove the test from this my testcase and someone later fixes
> >    the 6 existing instances of this test this only test will remain unfixed.
> >  * If I waste the time reading the sources what should be the expected output
> >    I risk the test will not work anyway as I read it wrongly.
> >    And I will probably wrongly modify 6 other testsuite cases.
> >  * I can try to find a platform without gcore but in most cases in the past my
> >    attempts to build GDB on any non-Linux platforms failed.
> 
> No no, you misunderstand, we should just delete it completely.
> It's a useless test.  See patch below.

I understood it perfectly.  I just did not want to invest time into writing
the off-topic patch and writing its ChangeLog for the patch you wrote below.
It is now solved and I can remove the gcore test also from my patch, thanks.


Jan

[commit+7.8] [patch+7.8?] Fix 'gcore' with exited threads

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On Thu, 21 Aug 2014 13:32:57 +0200, Pedro Alves wrote:
> On 08/17/2014 10:16 PM, Jan Kratochvil wrote:
> > On Mon, 23 Jun 2014 17:06:37 +0200, Pedro Alves wrote:
> >> > What does the log show ?
> > gcore .../gdb/testsuite/gdb.threads/gcore-thread0.test^M
> > Cannot find new threads: debugger service failed^M
> > (gdb) FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
> > +
> > core .../gdb/testsuite/gdb.threads/gcore-thread0.test^M
> > ".../gdb/testsuite/gdb.threads/gcore-thread0.test" is not a core dump: File format not recognized^M
> > (gdb) FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)
> 
> Thanks.  It'd be good to have this in the commit log.

I have put it to the commit log.
For the record as discussed in the past and not to repeat myself I just do not
agree with this statement.


> Otherwise OK.

Checked in: 22fd09ae995556cc1b898afe3d5901eb161d1102

> IMO, this is good for 7.8.

Checked in: 14adc931130880d75eccc023cbaec68790960235


Thanks,
Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 7481 bytes --]

commit 22fd09ae995556cc1b898afe3d5901eb161d1102
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Thu Aug 21 20:36:20 2014 +0200

    Fix 'gcore' with exited threads
    
    Program received signal SIGABRT, Aborted.
    [...]
    (gdb) gcore foobar
    Couldn't get registers: No such process.
    (gdb) info threads
    [...]
    (gdb) gcore foobar
    Saved corefile foobar
    (gdb)
    
    gcore tries to access the exited thread:
    [Thread 0x7ffff7fce700 (LWP 6895) exited]
    ptrace(PTRACE_GETREGS, 6895, 0, 0x7fff18167dd0) = -1 ESRCH (No such process)
    
    Without the TRY_CATCH protection testsuite FAILs for:
    	gcore .../gdb/testsuite/gdb.threads/gcore-thread0.test
    	Cannot find new threads: debugger service failed
    	(gdb) FAIL: gdb.threads/gcore-thread.exp: save a zeroed-threads corefile
    	+
    	core .../gdb/testsuite/gdb.threads/gcore-thread0.test
    	".../gdb/testsuite/gdb.threads/gcore-thread0.test" is not a core dump: File format not recognized
    	(gdb) FAIL: gdb.threads/gcore-thread.exp: core0file: re-load generated corefile (bad file format)
    Maybe the TRY_CATCH could be more inside update_thread_list().
    
    Similar update_thread_list() call is IMO missing in procfs_make_note_section()
    but I do not have where to verify that change.
    
    gdb/ChangeLog
    2014-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* linux-tdep.c (linux_corefile_thread_callback): Ignore THREAD_EXITED.
    	(linux_make_corefile_notes): call update_thread_list, protected against
    	exceptions.
    
    gdb/testsuite/ChangeLog
    2014-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* gdb.threads/gcore-stale-thread.c: New file.
    	* gdb.threads/gcore-stale-thread.exp: New file.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bf7f618..e8f652f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* linux-tdep.c (linux_corefile_thread_callback): Ignore THREAD_EXITED.
+	(linux_make_corefile_notes): call update_thread_list, protected against
+	exceptions.
+
 2014-08-21  Pedro Alves  <palves@redhat.com>
 
 	* infcmd.c (attach_command): Remove comment.
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d0f1106..dae59c5 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1194,6 +1194,11 @@ linux_corefile_thread_callback (struct thread_info *info, void *data)
 {
   struct linux_corefile_thread_data *args = data;
 
+  /* It can be current thread
+     which cannot be removed by update_thread_list.  */
+  if (info->state == THREAD_EXITED)
+    return 0;
+
   if (ptid_get_pid (info->ptid) == args->pid)
     {
       struct cleanup *old_chain;
@@ -1445,6 +1450,7 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
   char *note_data = NULL;
   gdb_byte *auxv;
   int auxv_len;
+  volatile struct gdb_exception e;
 
   if (linux_fill_prpsinfo (&prpsinfo))
     {
@@ -1468,6 +1474,12 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
     }
 
   /* Thread register information.  */
+  TRY_CATCH (e, RETURN_MASK_ERROR)
+    {
+      update_thread_list ();
+    }
+  if (e.reason < 0)
+    exception_print (gdb_stderr, e);
   thread_args.gdbarch = gdbarch;
   thread_args.pid = ptid_get_pid (inferior_ptid);
   thread_args.obfd = obfd;
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 31c64ff..8689b5e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.threads/gcore-stale-thread.c: New file.
+	* gdb.threads/gcore-stale-thread.exp: New file.
+
 2014-08-21  Pedro Alves  <palves@redhat.com>
 
 	* gdb.base/gcore-buffer-overflow.exp: Remove "help gcore" test.
diff --git a/gdb/testsuite/gdb.threads/gcore-stale-thread.c b/gdb/testsuite/gdb.threads/gcore-stale-thread.c
new file mode 100644
index 0000000..944acba
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/gcore-stale-thread.c
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 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 <pthread.h>
+#include <assert.h>
+
+static pthread_t main_thread;
+
+static void *
+start (void *arg)
+{
+  int i;
+
+  i = pthread_join (main_thread, NULL);
+  assert (i == 0);
+
+  return arg; /* break-here */
+}
+
+int
+main (void)
+{
+  pthread_t thread;
+  int i;
+
+  main_thread = pthread_self ();
+
+  i = pthread_create (&thread, NULL, start, NULL);
+  assert (i == 0);
+
+  pthread_exit (NULL);
+  assert (0);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/gcore-stale-thread.exp b/gdb/testsuite/gdb.threads/gcore-stale-thread.exp
new file mode 100644
index 0000000..291c56f
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/gcore-stale-thread.exp
@@ -0,0 +1,57 @@
+# Copyright 2014 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/>.
+
+standard_testfile
+set corefile [standard_output_file ${testfile}.core]
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable debug] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test_no_output "set non-stop on"
+
+if { ! [ runto_main ] } then {
+    return -1
+}
+
+gdb_test_multiple "info threads" "threads are supported" {
+    -re ".* main .*\r\n$gdb_prompt $" {
+	# OK, threads are supported.
+    }
+    -re "\r\n$gdb_prompt $" {
+	unsupported "gdb does not support threads on this target"
+	return -1
+    }
+}
+
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "break-here"]
+# gdb_continue_to_breakpoint does not work as it uses "$gdb_prompt $" regex
+# which does not work due to the output: (gdb) [Thread ... exited]
+set name "continue to breakpoint: break-here"
+gdb_test_multiple "continue" $name {
+    -re "Breakpoint .* (at|in) .* break-here .*\r\n$gdb_prompt " {
+	pass $name
+    }
+}
+
+gdb_gcore_cmd "$corefile" "save a corefile"
+
+# Do not run "info threads" before "gcore" as it could workaround the bug
+# by discarding non-current exited threads.
+gdb_test "info threads" \
+         {The current thread <Thread ID 1> has terminated\.  See `help thread'\.} \
+	 "exited thread is current due to non-stop"