* [patch] Fix 'gcore' with exited threads @ 2014-06-09 20:30 Jan Kratochvil 2014-06-23 15:06 ` Pedro Alves 0 siblings, 1 reply; 7+ messages in thread From: Jan Kratochvil @ 2014-06-09 20:30 UTC (permalink / raw) To: gdb-patches [-- 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" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] Fix 'gcore' with exited threads 2014-06-09 20:30 [patch] Fix 'gcore' with exited threads Jan Kratochvil @ 2014-06-23 15:06 ` Pedro Alves 2014-08-17 21:16 ` [patch+7.8?] " Jan Kratochvil 0 siblings, 1 reply; 7+ messages in thread From: Pedro Alves @ 2014-06-23 15:06 UTC (permalink / raw) To: Jan Kratochvil, gdb-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [patch+7.8?] Fix 'gcore' with exited threads 2014-06-23 15:06 ` Pedro Alves @ 2014-08-17 21:16 ` Jan Kratochvil 2014-08-21 10:41 ` [PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads) Pedro Alves 2014-08-21 11:33 ` [patch+7.8?] Fix 'gcore' with exited threads Pedro Alves 0 siblings, 2 replies; 7+ messages in thread From: Jan Kratochvil @ 2014-08-17 21:16 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- 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" ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads) 2014-08-17 21:16 ` [patch+7.8?] " Jan Kratochvil @ 2014-08-21 10:41 ` Pedro Alves 2014-08-21 18:29 ` Jan Kratochvil 2014-08-21 11:33 ` [patch+7.8?] Fix 'gcore' with exited threads Pedro Alves 1 sibling, 1 reply; 7+ messages in thread From: Pedro Alves @ 2014-08-21 10:41 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads) 2014-08-21 10:41 ` [PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads) Pedro Alves @ 2014-08-21 18:29 ` Jan Kratochvil 0 siblings, 0 replies; 7+ messages in thread From: Jan Kratochvil @ 2014-08-21 18:29 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch+7.8?] Fix 'gcore' with exited threads 2014-08-17 21:16 ` [patch+7.8?] " Jan Kratochvil 2014-08-21 10:41 ` [PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads) Pedro Alves @ 2014-08-21 11:33 ` Pedro Alves 2014-08-21 18:45 ` [commit+7.8] " Jan Kratochvil 1 sibling, 1 reply; 7+ messages in thread From: Pedro Alves @ 2014-08-21 11:33 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [commit+7.8] [patch+7.8?] Fix 'gcore' with exited threads 2014-08-21 11:33 ` [patch+7.8?] Fix 'gcore' with exited threads Pedro Alves @ 2014-08-21 18:45 ` Jan Kratochvil 0 siblings, 0 replies; 7+ messages in thread From: Jan Kratochvil @ 2014-08-21 18:45 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- 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" ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-08-21 18:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-09 20:30 [patch] Fix 'gcore' with exited threads Jan Kratochvil 2014-06-23 15:06 ` Pedro Alves 2014-08-17 21:16 ` [patch+7.8?] " Jan Kratochvil 2014-08-21 10:41 ` [PUSHED] Remove useless gcore command detection (was: Re: [patch+7.8?] Fix 'gcore' with exited threads) Pedro Alves 2014-08-21 18:29 ` Jan Kratochvil 2014-08-21 11:33 ` [patch+7.8?] Fix 'gcore' with exited threads Pedro Alves 2014-08-21 18:45 ` [commit+7.8] " Jan Kratochvil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox