* [patch] Forbid run with a core file loaded @ 2010-05-21 14:47 Jan Kratochvil 2010-05-21 15:02 ` Mark Kettenis ` (2 more replies) 0 siblings, 3 replies; 26+ messages in thread From: Jan Kratochvil @ 2010-05-21 14:47 UTC (permalink / raw) To: gdb-patches Hi, there is already a protection against loading a core file when a program is running. But one still can now run a program when a core file is loaded. Moreover GDB then crashes on `quit'. (gdb) file sleep [...] (gdb) start [...] (gdb) core-file core.5841 A program is being debugged already. Kill it? (y or n) y [...] Program terminated with signal 11, Segmentation fault. [...] (gdb) start Starting program: /bin/sleep ^^^^^^^^^^^^^^^^ !!! [...] (gdb) quit A debugging session is active. Inferior 1 [process 13887] will be killed. Quit anyway? (y or n) y inferior.c:362: internal-error: find_inferior_pid: Assertion `pid != 0' failed. Forbid even the latter case. No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu. Thanks, Jan gdb/ 2010-05-21 Jan Kratochvil <jan.kratochvil@redhat.com> Forbid run with a core file loaded. * infcmd.c (run_command_1) <core_bfd>: New. gdb/testsuite/ 2010-05-21 Jan Kratochvil <jan.kratochvil@redhat.com> Forbid run with a core file loaded. * gdb.base/corefile.exp (load core again, start with core) (started with core): New tests. --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -483,6 +483,15 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) dont_repeat (); + if (core_bfd) + { + if (!from_tty + || query (_("Core file is already loaded. Unload it? "))) + core_file_command (NULL, from_tty); + if (core_bfd) + error (_("Core file not unloaded.")); + } + kill_if_already_running (from_tty); init_wait_for_inferior (); --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -180,3 +180,15 @@ gdb_load ${binfile} gdb_test "up" "#\[0-9\]* *\[0-9xa-fH'\]* in .* \\(.*\\).*" "up in corefile.exp (reinit)" gdb_test "core" "No core file now." + +# Test a run (start) command will clear any loaded core file. + +gdb_test "core-file $corefile" "Core was generated by .*" "load core again" + +set test "start with core" +gdb_test_multiple "start" $test { + -re {Core file is already loaded. Unload it[?] [(]y or n[)] } { + pass $test + } +} +gdb_test "y" {No core file now\..*reakpoint [0-9]+, main.*} "started with core" ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 14:47 [patch] Forbid run with a core file loaded Jan Kratochvil @ 2010-05-21 15:02 ` Mark Kettenis 2010-05-21 15:05 ` Joel Brobecker ` (2 more replies) 2010-05-21 15:04 ` Joel Brobecker 2010-05-21 15:06 ` Pedro Alves 2 siblings, 3 replies; 26+ messages in thread From: Mark Kettenis @ 2010-05-21 15:02 UTC (permalink / raw) To: jan.kratochvil; +Cc: gdb-patches > Date: Fri, 21 May 2010 15:47:19 +0200 > From: Jan Kratochvil <jan.kratochvil@redhat.com> > > Hi, > > there is already a protection against loading a core file when a program is > running. That makes sense. But what you are suggesting doesn't. I often start gdb and load a core file to investigate a problem. Then I set a breakpoint at some point before the crash and run the program again. This used to work just fine. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:02 ` Mark Kettenis @ 2010-05-21 15:05 ` Joel Brobecker 2010-05-21 15:40 ` Mark Kettenis 2010-05-21 15:05 ` Pedro Alves 2010-05-21 15:07 ` Jan Kratochvil 2 siblings, 1 reply; 26+ messages in thread From: Joel Brobecker @ 2010-05-21 15:05 UTC (permalink / raw) To: Mark Kettenis; +Cc: jan.kratochvil, gdb-patches > > there is already a protection against loading a core file when a program is > > running. > > That makes sense. But what you are suggesting doesn't. > > I often start gdb and load a core file to investigate a problem. Then > I set a breakpoint at some point before the crash and run the program > again. This used to work just fine. There might have been a poor choice of words in the subject - Based on the patch, Jan is not proposing to actually forbid that operation. Rather, he's suggesting we add a confirmation query before actually restarting the inferior. Would you agree to such a change? -- Joel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:05 ` Joel Brobecker @ 2010-05-21 15:40 ` Mark Kettenis 2010-05-23 19:38 ` Jan Kratochvil 0 siblings, 1 reply; 26+ messages in thread From: Mark Kettenis @ 2010-05-21 15:40 UTC (permalink / raw) To: brobecker; +Cc: jan.kratochvil, gdb-patches > Date: Fri, 21 May 2010 08:04:28 -0700 > From: Joel Brobecker <brobecker@adacore.com> > > > > there is already a protection against loading a core file when a > > > program is running. > > > > That makes sense. But what you are suggesting doesn't. > > > > I often start gdb and load a core file to investigate a problem. Then > > I set a breakpoint at some point before the crash and run the program > > again. This used to work just fine. > > There might have been a poor choice of words in the subject - Based on > the patch, Jan is not proposing to actually forbid that operation. > Rather, he's suggesting we add a confirmation query before actually > restarting the inferior. Would you agree to such a change? I'd think that would be pretty pointless. You don't really lose state if you use the "run" command. You can easily get back to looking at the core file by reloading it using the "core" command. In general these "Are you sure?" type questions are un-Unixy and rather annoying in my opinion. They make sense if the user is about to take an action that is going to lose state (such as loading a core file while a program is running), but I think we shouldn't add them in this case. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:40 ` Mark Kettenis @ 2010-05-23 19:38 ` Jan Kratochvil 2010-05-23 21:08 ` Eli Zaretskii 2010-05-23 21:16 ` Pedro Alves 0 siblings, 2 replies; 26+ messages in thread From: Jan Kratochvil @ 2010-05-23 19:38 UTC (permalink / raw) To: Mark Kettenis, Joel Brobecker, Pedro Alves; +Cc: gdb-patches On Fri, 21 May 2010 17:31:06 +0200, Mark Kettenis wrote: > In general these "Are you sure?" type questions are un-Unixy and > rather annoying in my opinion. They make sense if the user is about > to take an action that is going to lose state (such as loading a core > file while a program is running), but I think we shouldn't add them in > this case. That makes sense, removed the question. On Fri, 21 May 2010 17:02:31 +0200, Joel Brobecker wrote: > > +set test "start with core" > > +gdb_test_multiple "start" $test { > > + -re {Core file is already loaded. Unload it[?] [(]y or n[)] } { > > + pass $test > > + } > > +} > > We should not use the "start" command in testcases, as it does not > work with remote targets. I'm afraid we're going to have to rely on > the run command instead. OK, used runto_main now. Attach test does not run with gdbserver the same way gdb.base/attach.exp does not. Tested this testfile with runtest-gdbserver (only on localhost). On Fri, 21 May 2010 17:06:07 +0200, Pedro Alves wrote: > I don't think it makes that much sense nowadays to have a > distinct core_stratum vs process_stratum. OK, removed core_stratum. > It core_ops was a process_stratum, pushing linux-nat.c would auto discard > the core, as you can't have two targets of the same stratum on the stack. Followed it although some ordering adjustments were required. > The only main use for that stratum nowadays is for find_core_target, > I think. Replaced. Unaware if some 3rd patches were using secondary core_stratum, it probably could not work anyway. > > Forbid run with a core file loaded. > > * infcmd.c (run_command_1) <core_bfd>: New. > > What about "attach"? Forgot, implemented/fixed. No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu. Thanks, Jan gdb/ 2010-05-23 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * corefile.c (core_target): New variable. (core_file_command): Remove variable t, use core_target. * corelow.c (core_ops): Make it static. (init_core_ops): Change to process_stratum. Initialize CORE_TARGET. * gdbarch.sh (core_pid_to_str): Remove core_stratum from its comment. * gdbarch.h: Regenerate. * gdbcore.h (core_target): New declaration. * inf-ptrace.c (inf_ptrace_create_inferior): Move push_target before fork_inferior, comment it. (inf_ptrace_attach): Call pop_all_targets_above. * record.c (record_open): Replace core_stratum by a core_bfd check. * target.c (find_core_target): Remove. * target.h (enum strata) <core_stratum>: Remove. (find_core_target): Remove declaration. * tracepoint.c (init_tfile_ops) <to_stratum>: Remove comment. gdb/doc/ 2010-05-23 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * gdb.texinfo (Active Targets): Remove core_stratum. Include record_stratum example. gdb/testsuite/ 2010-05-23 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * gdb.base/corefile.exp (run: load core again) (run: sanity check we see the core file, run: with core) (run: core file is cleared, attach: load core again) (attach: sanity check we see the core file, attach: with core) (attach: core file is cleared): New tests. * gdb.base/coremaker.c (main): New parameters. Implement "sleep" argv. --- a/gdb/corefile.c +++ b/gdb/corefile.c @@ -59,6 +59,10 @@ static int exec_file_hook_count = 0; /* size of array */ /* Binary file diddling handle for the core file. */ bfd *core_bfd = NULL; + +/* corelow.c target (if included for this gdb target). */ + +struct target_ops *core_target; \f /* Backward compatability with old way of specifying core files. */ @@ -66,18 +70,15 @@ bfd *core_bfd = NULL; void core_file_command (char *filename, int from_tty) { - struct target_ops *t; - dont_repeat (); /* Either way, seems bogus. */ - t = find_core_target (); - if (t == NULL) + if (core_target == NULL) error (_("GDB can't read core files on this machine.")); if (!filename) - (t->to_detach) (t, filename, from_tty); + (core_target->to_detach) (core_target, filename, from_tty); else - (t->to_open) (filename, from_tty); + (core_target->to_open) (filename, from_tty); } \f --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -100,7 +100,7 @@ static void init_core_ops (void); void _initialize_corelow (void); -struct target_ops core_ops; +static struct target_ops core_ops; /* An arbitrary identifier for the core inferior. */ #define CORELOW_PID 1 @@ -911,11 +911,17 @@ init_core_ops (void) core_ops.to_thread_alive = core_thread_alive; core_ops.to_read_description = core_read_description; core_ops.to_pid_to_str = core_pid_to_str; - core_ops.to_stratum = core_stratum; + core_ops.to_stratum = process_stratum; core_ops.to_has_memory = core_has_memory; core_ops.to_has_stack = core_has_stack; core_ops.to_has_registers = core_has_registers; core_ops.to_magic = OPS_MAGIC; + + if (core_target) + internal_error (__FILE__, __LINE__, + _("init_core_ops: core target already exists (\"%s\")."), + core_target->to_longname); + core_target = &core_ops; } void --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -609,8 +609,7 @@ v:struct core_regset_section *:core_regset_sections:const char *name, int len::: # core file into buffer READBUF with length LEN. M:LONGEST:core_xfer_shared_libraries:gdb_byte *readbuf, ULONGEST offset, LONGEST len:readbuf, offset, len -# How the core_stratum layer converts a PTID from a core file to a -# string. +# How the core target converts a PTID from a core file to a string. M:char *:core_pid_to_str:ptid_t ptid:ptid # BFD target to use when generating a core file. --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -666,8 +666,7 @@ typedef LONGEST (gdbarch_core_xfer_shared_libraries_ftype) (struct gdbarch *gdba extern LONGEST gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, LONGEST len); extern void set_gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdbarch_core_xfer_shared_libraries_ftype *core_xfer_shared_libraries); -/* How the core_stratum layer converts a PTID from a core file to a - string. */ +/* How the core target converts a PTID from a core file to a string. */ extern int gdbarch_core_pid_to_str_p (struct gdbarch *gdbarch); --- a/gdb/gdbcore.h +++ b/gdb/gdbcore.h @@ -108,6 +108,8 @@ extern void specify_exec_file_hook (void (*hook) (char *filename)); extern bfd *core_bfd; +extern struct target_ops *core_target; + /* Whether to open exec and core files read-only or read-write. */ extern int write_files; --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -121,11 +121,12 @@ inf_ptrace_create_inferior (struct target_ops *ops, { int pid; + /* Clear possible core file with its process_stratum. */ + push_target (ops); + pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL, NULL, NULL); - push_target (ops); - /* On some targets, there must be some explicit synchronization between the parent and child processes after the debugger forks, and before the child execs the debuggee program. This @@ -194,6 +195,10 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); + /* target_pid_to_str already uses the target. Clear possible core file with + its process_stratum. */ + pop_all_targets_above (ops->to_stratum - 1, 0); + if (from_tty) { exec_file = get_exec_file (0); --- a/gdb/record.c +++ b/gdb/record.c @@ -958,7 +958,7 @@ record_open (char *name, int from_tty) record_beneath_to_stopped_by_watchpoint = tmp_to_stopped_by_watchpoint; record_beneath_to_stopped_data_address = tmp_to_stopped_data_address; - if (current_target.to_stratum == core_stratum) + if (core_bfd) record_core_open_1 (name, from_tty); else record_open_1 (name, from_tty); --- a/gdb/target.c +++ b/gdb/target.c @@ -2699,31 +2699,6 @@ find_run_target (void) return (count == 1 ? runable : NULL); } -/* Find a single core_stratum target in the list of targets and return it. - If for some reason there is more than one, return NULL. */ - -struct target_ops * -find_core_target (void) -{ - struct target_ops **t; - struct target_ops *runable = NULL; - int count; - - count = 0; - - for (t = target_structs; t < target_structs + target_struct_size; - ++t) - { - if ((*t)->to_stratum == core_stratum) - { - runable = *t; - ++count; - } - } - - return (count == 1 ? runable : NULL); -} - /* * Find the next target down the stack from the specified target. */ --- a/gdb/target.h +++ b/gdb/target.h @@ -65,8 +65,7 @@ enum strata { dummy_stratum, /* The lowest of the low */ file_stratum, /* Executable files, etc */ - core_stratum, /* Core dump files */ - process_stratum, /* Executing processes */ + process_stratum, /* Executing processes or core dump files */ thread_stratum, /* Executing threads */ record_stratum, /* Support record debugging */ arch_stratum /* Architecture overrides */ @@ -1496,8 +1495,6 @@ extern void find_default_create_inferior (struct target_ops *, extern struct target_ops *find_run_target (void); -extern struct target_ops *find_core_target (void); - extern struct target_ops *find_target_beneath (struct target_ops *); /* Read OS data object of type TYPE from the target, and return it in --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -4050,8 +4050,6 @@ init_tfile_ops (void) tfile_ops.to_get_trace_status = tfile_get_trace_status; tfile_ops.to_trace_find = tfile_trace_find; tfile_ops.to_get_trace_state_variable_value = tfile_get_trace_state_variable_value; - /* core_stratum might seem more logical, but GDB doesn't like having - more than one core_stratum vector. */ tfile_ops.to_stratum = process_stratum; tfile_ops.to_has_all_memory = tfile_has_all_memory; tfile_ops.to_has_memory = tfile_has_memory; --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -15024,33 +15024,21 @@ and @code{show architecture}. @cindex active targets @cindex multiple targets -There are three classes of targets: processes, core files, and -executable files. @value{GDBN} can work concurrently on up to three -active targets, one in each class. This allows you to (for example) -start a process and inspect its activity without abandoning your work on -a core file. - -For example, if you execute @samp{gdb a.out}, then the executable file -@code{a.out} is the only active target. If you designate a core file as -well---presumably from a prior run that crashed and coredumped---then -@value{GDBN} has two active targets and uses them in tandem, looking -first in the corefile target, then in the executable file, to satisfy -requests for memory addresses. (Typically, these two classes of target -are complementary, since core files contain only a program's -read-write memory---variables and so on---plus machine status, while -executable files contain only the program text and initialized data.) - -When you type @code{run}, your executable file becomes an active process -target as well. When a process target is active, all @value{GDBN} -commands requesting memory addresses refer to that target; addresses in -an active core file or executable file target are obscured while the -process target is active. - -Use the @code{core-file} and @code{exec-file} commands to select a new -core file or executable target (@pxref{Files, ,Commands to Specify -Files}). To specify as a target a process that is already running, use -the @code{attach} command (@pxref{Attach, ,Debugging an Already-running -Process}). +There are multiple classes of targets such as: processes, executable files or +recording sessions. Core files belong to the process class, there can be +active only one of a core or a running process. Otherwise @value{GDBN} can +work concurrently on multiple active targets, one in each class. This allows +you to (for example) start a process and inspect its activity while still +having access to the executable file after the process finishes. Or if you +start process recording (@pxref{Reverse Execution}) and @code{reverse-step} +there you are presented a virtual layer of the recording target while the +process target remains stopped at the chronologically last point of the process +execution. + +Use the @code{core-file} and @code{exec-file} commands to select a new core +file or executable target (@pxref{Files, ,Commands to Specify Files}). To +specify as a target a process that is already running, use the @code{attach} +command (@pxref{Attach, ,Debugging an Already-running Process}). @node Target Commands @section Commands for Managing Targets --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -180,3 +180,62 @@ gdb_load ${binfile} gdb_test "up" "#\[0-9\]* *\[0-9xa-fH'\]* in .* \\(.*\\).*" "up in corefile.exp (reinit)" gdb_test "core" "No core file now." + + +# Test a run (start) command will clear any loaded core file. + +gdb_test "core-file $corefile" "Core was generated by .*" "run: load core again" +gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "run: sanity check we see the core file" + +set test "run: with core" +if [runto_main] { + pass $test +} else { + fail $test +} + +set test "run: core file is cleared" +gdb_test_multiple "info files" $test { + "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + "\r\n$gdb_prompt $" { + pass $test + } +} + +gdb_exit + + +# Test an attach command will clear any loaded core file. + +if ![is_remote target] { + set test "attach: spawn sleep" + set res [remote_spawn host "$binfile sleep"]; + if { $res < 0 || $res == "" } { + perror "$test failed." + fail $test + return + } + set pid [exp_pid -i $res] + # We do not care of the startup phase where it will be caught. + + gdb_start + + gdb_test "core-file $corefile" "Core was generated by .*" "attach: load core again" + gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "attach: sanity check we see the core file" + + gdb_test "attach $pid" "Attaching to process $pid\r\n.*" "attach: with core" + + set test "attach: core file is cleared" + gdb_test_multiple "info files" $test { + "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + "\r\n$gdb_prompt $" { + pass $test + } + } + + gdb_exit +} --- a/gdb/testsuite/gdb.base/coremaker.c +++ b/gdb/testsuite/gdb.base/coremaker.c @@ -133,8 +133,14 @@ func1 () func2 (); } -int main () +int +main (int argc, char **argv) { + if (argc == 2 && strcmp (argv[1], "sleep") == 0) + { + sleep (60); + return 0; + } mmapdata (); func1 (); return 0; ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-23 19:38 ` Jan Kratochvil @ 2010-05-23 21:08 ` Eli Zaretskii 2010-06-06 19:51 ` Jan Kratochvil 2010-05-23 21:16 ` Pedro Alves 1 sibling, 1 reply; 26+ messages in thread From: Eli Zaretskii @ 2010-05-23 21:08 UTC (permalink / raw) To: Jan Kratochvil; +Cc: mark.kettenis, brobecker, pedro, gdb-patches > Date: Sun, 23 May 2010 20:54:40 +0200 > From: Jan Kratochvil <jan.kratochvil@redhat.com> > Cc: gdb-patches@sourceware.org > > +There are multiple classes of targets such as: processes, executable files or > +recording sessions. Core files belong to the process class, there can be > +active only one of a core or a running process. Something is wrong with the last sentence. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-23 21:08 ` Eli Zaretskii @ 2010-06-06 19:51 ` Jan Kratochvil 2010-06-06 23:08 ` Eli Zaretskii 2010-06-07 11:21 ` Pedro Alves 0 siblings, 2 replies; 26+ messages in thread From: Jan Kratochvil @ 2010-06-06 19:51 UTC (permalink / raw) To: Eli Zaretskii, Pedro Alves; +Cc: mark.kettenis, brobecker, gdb-patches On Sun, 23 May 2010 22:14:41 +0200, Eli Zaretskii wrote: > > +There are multiple classes of targets such as: processes, executable files or > > +recording sessions. Core files belong to the process class, there can be > > +active only one of a core or a running process. > > Something is wrong with the last sentence. Used now: Core files belong to the process class making core file and process mutually exclusive. On Sun, 23 May 2010 23:08:44 +0200, Pedro Alves wrote: > On Sunday 23 May 2010 19:54:40, Jan Kratochvil wrote: > > @@ -194,6 +195,10 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) > > + pop_all_targets_above (ops->to_stratum - 1, 0); > > Unfortunately, this would break multiprocess. For example, you may > be attaching to your second process (e.g., "start", "add-inferior", > "inferior 2", "attach $pid"), and the thread stratum needs to remain > pushed for the first inferior/process. OK, fixed it by unpush_target only. I hope you agree the target stack should be made specific for each inferior. Like I proposed to fix target_gdbarch making it specific for each inferior. [01/03] Update target_gdbarch for current inferior http://sourceware.org/ml/gdb-patches/2010-01/msg00228.html > > + /* Clear possible core file with its process_stratum. */ > > + push_target (ops); > > + > > pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL, > > NULL, NULL); > > > > - push_target (ops); > > This makes sense, as indeed fork_inferior touches the current > target, even today, but, can you check what happens if fork_inferior > throws an error? OK, I have placed there a cleanup. No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu. OK to check-in? Thanks, Jan gdb/ 2010-05-23 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * corefile.c (core_target): New variable. (core_file_command): Remove variable t, use core_target. * corelow.c (core_ops): Make it static. (init_core_ops): Change to process_stratum. Initialize CORE_TARGET. * defs.h (make_cleanup_unpush_target): New prototype. * gdbarch.sh (core_pid_to_str): Remove core_stratum from its comment. * gdbarch.h: Regenerate. * gdbcore.h (core_target): New declaration. * inf-ptrace.c (inf_ptrace_create_inferior): New variable back_to. Move push_target before fork_inferior, comment it. (inf_ptrace_attach): Call unpush_target for core_target. * record.c (record_open): Replace core_stratum by a core_bfd check. * target.c (find_core_target): Remove. * target.h (enum strata) <core_stratum>: Remove. (find_core_target): Remove declaration. * tracepoint.c (init_tfile_ops) <to_stratum>: Remove comment. * utils.c (do_unpush_target, make_cleanup_unpush_target): New functions. gdb/doc/ 2010-05-23 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * gdb.texinfo (Active Targets): Remove core_stratum. Include record_stratum example. gdb/testsuite/ 2010-05-23 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * gdb.base/corefile.exp (run: load core again) (run: sanity check we see the core file, run: with core) (run: core file is cleared, attach: load core again) (attach: sanity check we see the core file, attach: with core) (attach: core file is cleared): New tests. * gdb.base/coremaker.c (main): New parameters. Implement "sleep" argv. --- a/gdb/corefile.c +++ b/gdb/corefile.c @@ -59,6 +59,10 @@ static int exec_file_hook_count = 0; /* size of array */ /* Binary file diddling handle for the core file. */ bfd *core_bfd = NULL; + +/* corelow.c target (if included for this gdb target). */ + +struct target_ops *core_target; \f /* Backward compatability with old way of specifying core files. */ @@ -66,18 +70,15 @@ bfd *core_bfd = NULL; void core_file_command (char *filename, int from_tty) { - struct target_ops *t; - dont_repeat (); /* Either way, seems bogus. */ - t = find_core_target (); - if (t == NULL) + if (core_target == NULL) error (_("GDB can't read core files on this machine.")); if (!filename) - (t->to_detach) (t, filename, from_tty); + (core_target->to_detach) (core_target, filename, from_tty); else - (t->to_open) (filename, from_tty); + (core_target->to_open) (filename, from_tty); } \f --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -100,7 +100,7 @@ static void init_core_ops (void); void _initialize_corelow (void); -struct target_ops core_ops; +static struct target_ops core_ops; /* An arbitrary identifier for the core inferior. */ #define CORELOW_PID 1 @@ -911,11 +911,17 @@ init_core_ops (void) core_ops.to_thread_alive = core_thread_alive; core_ops.to_read_description = core_read_description; core_ops.to_pid_to_str = core_pid_to_str; - core_ops.to_stratum = core_stratum; + core_ops.to_stratum = process_stratum; core_ops.to_has_memory = core_has_memory; core_ops.to_has_stack = core_has_stack; core_ops.to_has_registers = core_has_registers; core_ops.to_magic = OPS_MAGIC; + + if (core_target) + internal_error (__FILE__, __LINE__, + _("init_core_ops: core target already exists (\"%s\")."), + core_target->to_longname); + core_target = &core_ops; } void --- a/gdb/defs.h +++ b/gdb/defs.h @@ -348,6 +348,9 @@ extern struct cleanup *make_cleanup_obstack_free (struct obstack *obstack); extern struct cleanup *make_cleanup_restore_integer (int *variable); +struct target_ops; +extern struct cleanup *make_cleanup_unpush_target (struct target_ops *ops); + extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_my_cleanup (struct cleanup **, --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -609,8 +609,7 @@ v:struct core_regset_section *:core_regset_sections:const char *name, int len::: # core file into buffer READBUF with length LEN. M:LONGEST:core_xfer_shared_libraries:gdb_byte *readbuf, ULONGEST offset, LONGEST len:readbuf, offset, len -# How the core_stratum layer converts a PTID from a core file to a -# string. +# How the core target converts a PTID from a core file to a string. M:char *:core_pid_to_str:ptid_t ptid:ptid # BFD target to use when generating a core file. --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -666,8 +666,7 @@ typedef LONGEST (gdbarch_core_xfer_shared_libraries_ftype) (struct gdbarch *gdba extern LONGEST gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, LONGEST len); extern void set_gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdbarch_core_xfer_shared_libraries_ftype *core_xfer_shared_libraries); -/* How the core_stratum layer converts a PTID from a core file to a - string. */ +/* How the core target converts a PTID from a core file to a string. */ extern int gdbarch_core_pid_to_str_p (struct gdbarch *gdbarch); --- a/gdb/gdbcore.h +++ b/gdb/gdbcore.h @@ -108,6 +108,8 @@ extern void specify_exec_file_hook (void (*hook) (char *filename)); extern bfd *core_bfd; +extern struct target_ops *core_target; + /* Whether to open exec and core files read-only or read-write. */ extern int write_files; --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -119,12 +119,17 @@ inf_ptrace_create_inferior (struct target_ops *ops, char *exec_file, char *allargs, char **env, int from_tty) { + struct cleanup *back_to; int pid; + /* Clear possible core file with its process_stratum. */ + push_target (ops); + back_to = make_cleanup_unpush_target (ops); + pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL, NULL, NULL); - push_target (ops); + discard_cleanups (back_to); /* On some targets, there must be some explicit synchronization between the parent and child processes after the debugger @@ -194,6 +199,12 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); + /* target_pid_to_str already uses the target. Clear possible core file with + its process_stratum. Targets above must remain in place as the target + stack is shared across multiple inferiors. */ + if (core_target) + unpush_target (core_target); + if (from_tty) { exec_file = get_exec_file (0); --- a/gdb/record.c +++ b/gdb/record.c @@ -958,7 +958,7 @@ record_open (char *name, int from_tty) record_beneath_to_stopped_by_watchpoint = tmp_to_stopped_by_watchpoint; record_beneath_to_stopped_data_address = tmp_to_stopped_data_address; - if (current_target.to_stratum == core_stratum) + if (core_bfd) record_core_open_1 (name, from_tty); else record_open_1 (name, from_tty); --- a/gdb/target.c +++ b/gdb/target.c @@ -2699,31 +2699,6 @@ find_run_target (void) return (count == 1 ? runable : NULL); } -/* Find a single core_stratum target in the list of targets and return it. - If for some reason there is more than one, return NULL. */ - -struct target_ops * -find_core_target (void) -{ - struct target_ops **t; - struct target_ops *runable = NULL; - int count; - - count = 0; - - for (t = target_structs; t < target_structs + target_struct_size; - ++t) - { - if ((*t)->to_stratum == core_stratum) - { - runable = *t; - ++count; - } - } - - return (count == 1 ? runable : NULL); -} - /* * Find the next target down the stack from the specified target. */ --- a/gdb/target.h +++ b/gdb/target.h @@ -65,8 +65,7 @@ enum strata { dummy_stratum, /* The lowest of the low */ file_stratum, /* Executable files, etc */ - core_stratum, /* Core dump files */ - process_stratum, /* Executing processes */ + process_stratum, /* Executing processes or core dump files */ thread_stratum, /* Executing threads */ record_stratum, /* Support record debugging */ arch_stratum /* Architecture overrides */ @@ -1496,8 +1495,6 @@ extern void find_default_create_inferior (struct target_ops *, extern struct target_ops *find_run_target (void); -extern struct target_ops *find_core_target (void); - extern struct target_ops *find_target_beneath (struct target_ops *); /* Read OS data object of type TYPE from the target, and return it in --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -4049,8 +4049,6 @@ init_tfile_ops (void) tfile_ops.to_get_trace_status = tfile_get_trace_status; tfile_ops.to_trace_find = tfile_trace_find; tfile_ops.to_get_trace_state_variable_value = tfile_get_trace_state_variable_value; - /* core_stratum might seem more logical, but GDB doesn't like having - more than one core_stratum vector. */ tfile_ops.to_stratum = process_stratum; tfile_ops.to_has_all_memory = tfile_has_all_memory; tfile_ops.to_has_memory = tfile_has_memory; --- a/gdb/utils.c +++ b/gdb/utils.c @@ -352,6 +352,24 @@ make_cleanup_restore_integer (int *variable) xfree); } +/* Helper for make_cleanup_unpush_target. */ + +static void +do_unpush_target (void *arg) +{ + struct target_ops *ops = arg; + + unpush_target (ops); +} + +/* Return a new cleanup that unpushes OPS. */ + +struct cleanup * +make_cleanup_unpush_target (struct target_ops *ops) +{ + return make_my_cleanup (&cleanup_chain, do_unpush_target, ops); +} + struct cleanup * make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function, void *arg, void (*free_arg) (void *)) --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -15027,33 +15027,20 @@ and @code{show architecture}. @cindex active targets @cindex multiple targets -There are three classes of targets: processes, core files, and -executable files. @value{GDBN} can work concurrently on up to three -active targets, one in each class. This allows you to (for example) -start a process and inspect its activity without abandoning your work on -a core file. - -For example, if you execute @samp{gdb a.out}, then the executable file -@code{a.out} is the only active target. If you designate a core file as -well---presumably from a prior run that crashed and coredumped---then -@value{GDBN} has two active targets and uses them in tandem, looking -first in the corefile target, then in the executable file, to satisfy -requests for memory addresses. (Typically, these two classes of target -are complementary, since core files contain only a program's -read-write memory---variables and so on---plus machine status, while -executable files contain only the program text and initialized data.) - -When you type @code{run}, your executable file becomes an active process -target as well. When a process target is active, all @value{GDBN} -commands requesting memory addresses refer to that target; addresses in -an active core file or executable file target are obscured while the -process target is active. - -Use the @code{core-file} and @code{exec-file} commands to select a new -core file or executable target (@pxref{Files, ,Commands to Specify -Files}). To specify as a target a process that is already running, use -the @code{attach} command (@pxref{Attach, ,Debugging an Already-running -Process}). +There are multiple classes of targets such as: processes, executable files or +recording sessions. Core files belong to the process class making core file +and process mutually exclusive. Otherwise @value{GDBN} can work concurrently +on multiple active targets, one in each class. This allows you to (for +example) start a process and inspect its activity while still having access to +the executable file after the process finishes. Or if you start process +recording (@pxref{Reverse Execution}) and @code{reverse-step} there you are +presented a virtual layer of the recording target while the process target +remains stopped at the chronologically last point of the process execution. + +Use the @code{core-file} and @code{exec-file} commands to select a new core +file or executable target (@pxref{Files, ,Commands to Specify Files}). To +specify as a target a process that is already running, use the @code{attach} +command (@pxref{Attach, ,Debugging an Already-running Process}). @node Target Commands @section Commands for Managing Targets --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -177,3 +177,62 @@ gdb_load ${binfile} gdb_test "up" "#\[0-9\]* *\[0-9xa-fH'\]* in .* \\(.*\\).*" "up in corefile.exp (reinit)" gdb_test "core" "No core file now." + + +# Test a run (start) command will clear any loaded core file. + +gdb_test "core-file $corefile" "Core was generated by .*" "run: load core again" +gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "run: sanity check we see the core file" + +set test "run: with core" +if [runto_main] { + pass $test +} else { + fail $test +} + +set test "run: core file is cleared" +gdb_test_multiple "info files" $test { + "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + "\r\n$gdb_prompt $" { + pass $test + } +} + +gdb_exit + + +# Test an attach command will clear any loaded core file. + +if ![is_remote target] { + set test "attach: spawn sleep" + set res [remote_spawn host "$binfile sleep"]; + if { $res < 0 || $res == "" } { + perror "$test failed." + fail $test + return + } + set pid [exp_pid -i $res] + # We do not care of the startup phase where it will be caught. + + gdb_start + + gdb_test "core-file $corefile" "Core was generated by .*" "attach: load core again" + gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "attach: sanity check we see the core file" + + gdb_test "attach $pid" "Attaching to process $pid\r\n.*" "attach: with core" + + set test "attach: core file is cleared" + gdb_test_multiple "info files" $test { + "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + "\r\n$gdb_prompt $" { + pass $test + } + } + + gdb_exit +} --- a/gdb/testsuite/gdb.base/coremaker.c +++ b/gdb/testsuite/gdb.base/coremaker.c @@ -133,8 +133,14 @@ func1 () func2 (); } -int main () +int +main (int argc, char **argv) { + if (argc == 2 && strcmp (argv[1], "sleep") == 0) + { + sleep (60); + return 0; + } mmapdata (); func1 (); return 0; ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-06-06 19:51 ` Jan Kratochvil @ 2010-06-06 23:08 ` Eli Zaretskii 2010-06-07 11:21 ` Pedro Alves 1 sibling, 0 replies; 26+ messages in thread From: Eli Zaretskii @ 2010-06-06 23:08 UTC (permalink / raw) To: Jan Kratochvil; +Cc: pedro, mark.kettenis, brobecker, gdb-patches > Date: Sun, 6 Jun 2010 21:50:34 +0200 > From: Jan Kratochvil <jan.kratochvil@redhat.com> > Cc: mark.kettenis@xs4all.nl, brobecker@adacore.com, gdb-patches@sourceware.org > > No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu. > > OK to check-in? The part for the manual is okay with me. Thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-06-06 19:51 ` Jan Kratochvil 2010-06-06 23:08 ` Eli Zaretskii @ 2010-06-07 11:21 ` Pedro Alves 2010-06-08 2:33 ` Tom Tromey 2010-07-08 17:17 ` Jan Kratochvil 1 sibling, 2 replies; 26+ messages in thread From: Pedro Alves @ 2010-06-07 11:21 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Eli Zaretskii, mark.kettenis, brobecker, gdb-patches On Sunday 06 June 2010 20:50:34, Jan Kratochvil wrote: > > Unfortunately, this would break multiprocess. For example, you may > > be attaching to your second process (e.g., "start", "add-inferior", > > "inferior 2", "attach $pid"), and the thread stratum needs to remain > > pushed for the first inferior/process. > > OK, fixed it by unpush_target only. > > I hope you agree the target stack should be made specific for each inferior. I've gone back and forth on that for a while with the multi-exec work. It's trickier than that. Consider the extended-remote target --- if you do "add-inferior; <inf 2 created>; inferior 2; run", if we simply had a target stack per inferior, then when you get to "run", you'd only have "exec" on the inferior 2's stack, but you wanted extended-remote to handle creating the inferior. > Like I proposed to fix target_gdbarch making it specific for each inferior. > [01/03] Update target_gdbarch for current inferior > http://sourceware.org/ml/gdb-patches/2010-01/msg00228.html Maybe, though we probably still need a target_gdbarch to represent the target interface's properties, instead of any particular inferior it it controling. The canonical example is the remote target, running with multi-process enabled. > OK, I have placed there a cleanup. Thanks. You'd need to make sure the cleanup doesn't pop the target if there are still live inferiors to debug, so to not break other inferiors in multi-process (see the have_inferiors checks in inf-ptrace.c). > > > + /* Clear possible core file with its process_stratum. */ > + push_target (ops); > + back_to = make_cleanup_unpush_target (ops); > + > pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL, > NULL, NULL); > > - push_target (ops); > + discard_cleanups (back_to); > > /* On some targets, there must be some explicit synchronization > between the parent and child processes after the debugger > @@ -194,6 +199,12 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) > if (pid == getpid ()) /* Trying to masturbate? */ > error (_("I refuse to debug myself!")); > > + /* target_pid_to_str already uses the target. Clear possible core file with > + its process_stratum. Targets above must remain in place as the target > + stack is shared across multiple inferiors. */ > + if (core_target) > + unpush_target (core_target); I'd rather either get rid of the target_pid_to_str calls before the push, or push earlier (with a cleanup), than adding knowledge about the core target here. You can even consider those target_pid_to_str calls buggy, for accessing the exec or process stratum target depending on when they are called. > + > +# Test an attach command will clear any loaded core file. > + > +if ![is_remote target] { > + set test "attach: spawn sleep" > + set res [remote_spawn host "$binfile sleep"]; > + if { $res < 0 || $res == "" } { > + perror "$test failed." > + fail $test Do we need both perror and fail? > + return > + } > + set pid [exp_pid -i $res] > + # We do not care of the startup phase where it will be caught. I don't understand this comment. What's "it"? Please rephrase. Did you mean "We don't care whether the program is still in the startup phase when we attach"? Other than that, it looks good to me, and is good to check in, if Mark or others don't have objections. -- Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-06-07 11:21 ` Pedro Alves @ 2010-06-08 2:33 ` Tom Tromey 2010-06-08 11:03 ` Pedro Alves 2010-07-08 17:17 ` Jan Kratochvil 1 sibling, 1 reply; 26+ messages in thread From: Tom Tromey @ 2010-06-08 2:33 UTC (permalink / raw) To: Pedro Alves Cc: Jan Kratochvil, Eli Zaretskii, mark.kettenis, brobecker, gdb-patches Jan> I hope you agree the target stack should be made specific for each Jan> inferior. Pedro> I've gone back and forth on that for a while with the multi-exec Pedro> work. It's trickier than that. How else would you propose letting people attach to multiple inferiors using multiple targets? This seems like a reasonable thing to want to do, e.g., debugging a distributed application of some kind. Pedro> Consider the extended-remote target --- if you do "add-inferior; Pedro> <inf 2 created>; inferior 2; run", if we simply had a target Pedro> stack per inferior, then when you get to "run", you'd only have Pedro> "exec" on the inferior 2's stack, but you wanted extended-remote Pedro> to handle creating the inferior. I don't really know enough about the target API, but this seems fixable. "Target stack per inferior" does not necessarily imply that the targets cannot be shared, just that conceptually each inferior carries its own. Tom ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-06-08 2:33 ` Tom Tromey @ 2010-06-08 11:03 ` Pedro Alves 0 siblings, 0 replies; 26+ messages in thread From: Pedro Alves @ 2010-06-08 11:03 UTC (permalink / raw) To: tromey Cc: Jan Kratochvil, Eli Zaretskii, mark.kettenis, brobecker, gdb-patches On Tuesday 08 June 2010 03:32:29, Tom Tromey wrote: > Jan> I hope you agree the target stack should be made specific for each > Jan> inferior. > > Pedro> I've gone back and forth on that for a while with the multi-exec > Pedro> work. It's trickier than that. > > How else would you propose letting people attach to multiple inferiors > using multiple targets? This seems like a reasonable thing to want to > do, e.g., debugging a distributed application of some kind. It is. It's the obvious next step. It's the missing step into being able to debug parallel MPI programs running on different machines. But that also obviously needs to consider user interface issues -- more than just an internal implementation detail. > Pedro> Consider the extended-remote target --- if you do "add-inferior; > Pedro> <inf 2 created>; inferior 2; run", if we simply had a target > Pedro> stack per inferior, then when you get to "run", you'd only have > Pedro> "exec" on the inferior 2's stack, but you wanted extended-remote > Pedro> to handle creating the inferior. > > I don't really know enough about the target API, but this seems fixable. > "Target stack per inferior" does not necessarily imply that the targets > cannot be shared, just that conceptually each inferior carries its own. Of course everything is fixable. tricker != impossible != undesirable. If you go the simply route of simply sharing the target stack between all inferiors that share the same process_stratum target, you have what we have today. None of that would make Jan's patch any different, because it would still need to be careful to not pop random targets off the stack when _one_ inferior is gone. If you say that you'd only share the _targets_ and not the _stack_ between inferiors, then you'd have to rethink things like the target->beneath pointer, as that is embedded within the target_ops object. -- Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-06-07 11:21 ` Pedro Alves 2010-06-08 2:33 ` Tom Tromey @ 2010-07-08 17:17 ` Jan Kratochvil 2010-07-08 18:28 ` Eli Zaretskii 2010-07-19 14:37 ` Pedro Alves 1 sibling, 2 replies; 26+ messages in thread From: Jan Kratochvil @ 2010-07-08 17:17 UTC (permalink / raw) To: Pedro Alves Cc: Eli Zaretskii, mark.kettenis, brobecker, gdb-patches, Edjunior Barbosa Machado On Mon, 07 Jun 2010 13:20:57 +0200, Pedro Alves wrote: > On Sunday 06 June 2010 20:50:34, Jan Kratochvil wrote: > > I hope you agree the target stack should be made specific for each inferior. > > I've gone back and forth on that for a while with > the multi-exec work. It's trickier than that. Consider the extended-remote > target --- if you do "add-inferior; <inf 2 created>; inferior 2; run", > if we simply had a target stack per inferior, then when you get to > "run", you'd only have "exec" on the inferior 2's stack, but you wanted > extended-remote to handle creating the inferior. I understand it just cannot be moved into `struct inferior' without any other changes. As the month passed I se I am not going to implement it soon. > > OK, I have placed there a cleanup. > > Thanks. You'd need to make sure the cleanup doesn't pop the > target if there are still live inferiors to debug, so to not break > other inferiors in multi-process (see the have_inferiors checks > in inf-ptrace.c). Rather chose the safest way to never do any change which is not required. > I'd rather either get rid of the target_pid_to_str calls before > the push, or push earlier (with a cleanup), OK, chose the latter (that is "push earlier"). > > +# Test an attach command will clear any loaded core file. > > + > > +if ![is_remote target] { > > + set test "attach: spawn sleep" > > + set res [remote_spawn host "$binfile sleep"]; > > + if { $res < 0 || $res == "" } { > > + perror "$test failed." > > + fail $test > > Do we need both perror and fail? Removed perror. ------------------------------------------------------------------------------ |In general we need, as "perror" is a different category than pass/fail/xfail. |perror is there to ignore later results if there are too many errors. |But there is "return" in that block again so the real perror functionality is |not used anyway. | |IMO it would be right to just call `xfail $test'. Unfortunately xfail is |a very quiet action commonly being ignored IMO contrary to this case which |means some severe testsuite environment failure. | |Anyway left there only "fail" now which is not too quiet. But it is also |wrong as it is not a proven gdb-tool failure. I miss some loudxfail. ------------------------------------------------------------------------------ > > + return > > + } > > + set pid [exp_pid -i $res] > > + # We do not care of the startup phase where it will be caught. > > I don't understand this comment. What's "it"? Please rephrase. > Did you mean "We don't care whether the program is still in the startup > phase when we attach"? Used your sentence > Other than that, it looks good to me, and is good to check in, It should probably go into 7.2. As there are some new changes I am asking for new approval. No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu. Thanks, Jan gdb/ 2010-07-08 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * corefile.c (core_target): New variable. (core_file_command): Remove variable t, use core_target. * corelow.c (core_ops): Make it static. (init_core_ops): Change to process_stratum. Initialize CORE_TARGET. * defs.h (make_cleanup_unpush_target): New prototype. * gdbarch.h: Regenerate. * gdbarch.sh (core_pid_to_str): Remove core_stratum from its comment. * gdbcore.h (core_target): New declaration. * inf-ptrace.c (inf_ptrace_create_inferior, inf_ptrace_attach): New variables ops_already_pushed and back_to. Use push_target, make_cleanup_unpush_target and discard_cleanups calls. * record.c (record_open): Replace core_stratum by a core_bfd check. * target.c (target_is_pushed): New function. (find_core_target): Remove. * target.h (enum strata) <core_stratum>: Remove. (target_is_pushed): New declaration. (find_core_target): Remove declaration. * tracepoint.c (init_tfile_ops) <to_stratum>: Remove comment. * utils.c (do_unpush_target, make_cleanup_unpush_target): New functions. gdb/doc/ 2010-07-08 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * gdb.texinfo (Active Targets): Remove core_stratum. Include record_stratum example. gdb/testsuite/ 2010-07-08 Jan Kratochvil <jan.kratochvil@redhat.com> Make core files the process_stratum. * gdb.base/corefile.exp (run: load core again) (run: sanity check we see the core file, run: with core) (run: core file is cleared, attach: load core again) (attach: sanity check we see the core file, attach: with core) (attach: core file is cleared): New tests. * gdb.base/coremaker.c (main): New parameters. Implement "sleep" argv. --- a/gdb/corefile.c +++ b/gdb/corefile.c @@ -59,6 +59,10 @@ static int exec_file_hook_count = 0; /* size of array */ /* Binary file diddling handle for the core file. */ bfd *core_bfd = NULL; + +/* corelow.c target (if included for this gdb target). */ + +struct target_ops *core_target; \f /* Backward compatability with old way of specifying core files. */ @@ -66,18 +70,15 @@ bfd *core_bfd = NULL; void core_file_command (char *filename, int from_tty) { - struct target_ops *t; - dont_repeat (); /* Either way, seems bogus. */ - t = find_core_target (); - if (t == NULL) + if (core_target == NULL) error (_("GDB can't read core files on this machine.")); if (!filename) - (t->to_detach) (t, filename, from_tty); + (core_target->to_detach) (core_target, filename, from_tty); else - (t->to_open) (filename, from_tty); + (core_target->to_open) (filename, from_tty); } \f --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -100,7 +100,7 @@ static void init_core_ops (void); void _initialize_corelow (void); -struct target_ops core_ops; +static struct target_ops core_ops; /* An arbitrary identifier for the core inferior. */ #define CORELOW_PID 1 @@ -911,11 +911,17 @@ init_core_ops (void) core_ops.to_thread_alive = core_thread_alive; core_ops.to_read_description = core_read_description; core_ops.to_pid_to_str = core_pid_to_str; - core_ops.to_stratum = core_stratum; + core_ops.to_stratum = process_stratum; core_ops.to_has_memory = core_has_memory; core_ops.to_has_stack = core_has_stack; core_ops.to_has_registers = core_has_registers; core_ops.to_magic = OPS_MAGIC; + + if (core_target) + internal_error (__FILE__, __LINE__, + _("init_core_ops: core target already exists (\"%s\")."), + core_target->to_longname); + core_target = &core_ops; } void --- a/gdb/defs.h +++ b/gdb/defs.h @@ -352,6 +352,9 @@ extern struct cleanup *make_cleanup_obstack_free (struct obstack *obstack); extern struct cleanup *make_cleanup_restore_integer (int *variable); +struct target_ops; +extern struct cleanup *make_cleanup_unpush_target (struct target_ops *ops); + extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_my_cleanup (struct cleanup **, --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -672,8 +672,7 @@ typedef LONGEST (gdbarch_core_xfer_shared_libraries_ftype) (struct gdbarch *gdba extern LONGEST gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, LONGEST len); extern void set_gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdbarch_core_xfer_shared_libraries_ftype *core_xfer_shared_libraries); -/* How the core_stratum layer converts a PTID from a core file to a - string. */ +/* How the core target converts a PTID from a core file to a string. */ extern int gdbarch_core_pid_to_str_p (struct gdbarch *gdbarch); --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -611,8 +611,7 @@ v:struct core_regset_section *:core_regset_sections:const char *name, int len::: # core file into buffer READBUF with length LEN. M:LONGEST:core_xfer_shared_libraries:gdb_byte *readbuf, ULONGEST offset, LONGEST len:readbuf, offset, len -# How the core_stratum layer converts a PTID from a core file to a -# string. +# How the core target converts a PTID from a core file to a string. M:char *:core_pid_to_str:ptid_t ptid:ptid # BFD target to use when generating a core file. --- a/gdb/gdbcore.h +++ b/gdb/gdbcore.h @@ -108,6 +108,8 @@ extern void specify_exec_file_hook (void (*hook) (char *filename)); extern bfd *core_bfd; +extern struct target_ops *core_target; + /* Whether to open exec and core files read-only or read-write. */ extern int write_files; --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -121,10 +121,23 @@ inf_ptrace_create_inferior (struct target_ops *ops, { int pid; + /* Do not change either targets above or the same target if already present. + The reason is the target stack is shared across multiple inferiors. */ + int ops_already_pushed = target_is_pushed (ops); + struct cleanup *back_to; + + if (! ops_already_pushed) + { + /* Clear possible core file with its process_stratum. */ + push_target (ops); + back_to = make_cleanup_unpush_target (ops); + } + pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL, NULL, NULL); - push_target (ops); + if (! ops_already_pushed) + discard_cleanups (back_to); /* On some targets, there must be some explicit synchronization between the parent and child processes after the debugger @@ -189,11 +202,24 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) pid_t pid; struct inferior *inf; + /* Do not change either targets above or the same target if already present. + The reason is the target stack is shared across multiple inferiors. */ + int ops_already_pushed = target_is_pushed (ops); + struct cleanup *back_to; + pid = parse_pid_to_attach (args); if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); + if (! ops_already_pushed) + { + /* target_pid_to_str already uses the target. Also clear possible core + file with its process_stratum. */ + push_target (ops); + back_to = make_cleanup_unpush_target (ops); + } + if (from_tty) { exec_file = get_exec_file (0); @@ -226,7 +252,8 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) target, it should decorate the ptid later with more info. */ add_thread_silent (inferior_ptid); - push_target(ops); + if (! ops_already_pushed) + discard_cleanups (back_to); } #ifdef PT_GET_PROCESS_STATE --- a/gdb/record.c +++ b/gdb/record.c @@ -962,7 +962,7 @@ record_open (char *name, int from_tty) record_beneath_to_stopped_by_watchpoint = tmp_to_stopped_by_watchpoint; record_beneath_to_stopped_data_address = tmp_to_stopped_data_address; - if (current_target.to_stratum == core_stratum) + if (core_bfd) record_core_open_1 (name, from_tty); else record_open_1 (name, from_tty); --- a/gdb/target.c +++ b/gdb/target.c @@ -1037,6 +1037,30 @@ pop_all_targets (int quitting) pop_all_targets_above (dummy_stratum, quitting); } +/* Return 1 if T is now pushed in the target stack. Return 0 otherwise. */ + +int +target_is_pushed (struct target_ops *t) +{ + struct target_ops **cur; + + /* Check magic number. If wrong, it probably means someone changed + the struct definition, but not all the places that initialize one. */ + if (t->to_magic != OPS_MAGIC) + { + fprintf_unfiltered (gdb_stderr, + "Magic number of %s target struct wrong\n", + t->to_shortname); + internal_error (__FILE__, __LINE__, _("failed internal consistency check")); + } + + for (cur = &target_stack; (*cur) != NULL; cur = &(*cur)->beneath) + if (*cur == t) + return 1; + + return 0; +} + /* Using the objfile specified in OBJFILE, find the address for the current thread's thread-local storage with offset OFFSET. */ CORE_ADDR @@ -2770,31 +2794,6 @@ find_run_target (void) return (count == 1 ? runable : NULL); } -/* Find a single core_stratum target in the list of targets and return it. - If for some reason there is more than one, return NULL. */ - -struct target_ops * -find_core_target (void) -{ - struct target_ops **t; - struct target_ops *runable = NULL; - int count; - - count = 0; - - for (t = target_structs; t < target_structs + target_struct_size; - ++t) - { - if ((*t)->to_stratum == core_stratum) - { - runable = *t; - ++count; - } - } - - return (count == 1 ? runable : NULL); -} - /* * Find the next target down the stack from the specified target. */ --- a/gdb/target.h +++ b/gdb/target.h @@ -68,8 +68,7 @@ enum strata { dummy_stratum, /* The lowest of the low */ file_stratum, /* Executable files, etc */ - core_stratum, /* Core dump files */ - process_stratum, /* Executing processes */ + process_stratum, /* Executing processes or core dump files */ thread_stratum, /* Executing threads */ record_stratum, /* Support record debugging */ arch_stratum /* Architecture overrides */ @@ -1485,6 +1484,8 @@ extern void pop_all_targets (int quitting); strictly above ABOVE_STRATUM. */ extern void pop_all_targets_above (enum strata above_stratum, int quitting); +extern int target_is_pushed (struct target_ops *t); + extern CORE_ADDR target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset); @@ -1546,8 +1547,6 @@ extern void find_default_create_inferior (struct target_ops *, extern struct target_ops *find_run_target (void); -extern struct target_ops *find_core_target (void); - extern struct target_ops *find_target_beneath (struct target_ops *); /* Read OS data object of type TYPE from the target, and return it in --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -4098,8 +4098,6 @@ init_tfile_ops (void) tfile_ops.to_get_trace_status = tfile_get_trace_status; tfile_ops.to_trace_find = tfile_trace_find; tfile_ops.to_get_trace_state_variable_value = tfile_get_trace_state_variable_value; - /* core_stratum might seem more logical, but GDB doesn't like having - more than one core_stratum vector. */ tfile_ops.to_stratum = process_stratum; tfile_ops.to_has_all_memory = tfile_has_all_memory; tfile_ops.to_has_memory = tfile_has_memory; --- a/gdb/utils.c +++ b/gdb/utils.c @@ -352,6 +352,24 @@ make_cleanup_restore_integer (int *variable) xfree); } +/* Helper for make_cleanup_unpush_target. */ + +static void +do_unpush_target (void *arg) +{ + struct target_ops *ops = arg; + + unpush_target (ops); +} + +/* Return a new cleanup that unpushes OPS. */ + +struct cleanup * +make_cleanup_unpush_target (struct target_ops *ops) +{ + return make_my_cleanup (&cleanup_chain, do_unpush_target, ops); +} + struct cleanup * make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function, void *arg, void (*free_arg) (void *)) --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -15343,33 +15343,20 @@ and @code{show architecture}. @cindex active targets @cindex multiple targets -There are three classes of targets: processes, core files, and -executable files. @value{GDBN} can work concurrently on up to three -active targets, one in each class. This allows you to (for example) -start a process and inspect its activity without abandoning your work on -a core file. - -For example, if you execute @samp{gdb a.out}, then the executable file -@code{a.out} is the only active target. If you designate a core file as -well---presumably from a prior run that crashed and coredumped---then -@value{GDBN} has two active targets and uses them in tandem, looking -first in the corefile target, then in the executable file, to satisfy -requests for memory addresses. (Typically, these two classes of target -are complementary, since core files contain only a program's -read-write memory---variables and so on---plus machine status, while -executable files contain only the program text and initialized data.) - -When you type @code{run}, your executable file becomes an active process -target as well. When a process target is active, all @value{GDBN} -commands requesting memory addresses refer to that target; addresses in -an active core file or executable file target are obscured while the -process target is active. - -Use the @code{core-file} and @code{exec-file} commands to select a new -core file or executable target (@pxref{Files, ,Commands to Specify -Files}). To specify as a target a process that is already running, use -the @code{attach} command (@pxref{Attach, ,Debugging an Already-running -Process}). +There are multiple classes of targets such as: processes, executable files or +recording sessions. Core files belong to the process class making core file +and process mutually exclusive. Otherwise @value{GDBN} can work concurrently +on multiple active targets, one in each class. This allows you to (for +example) start a process and inspect its activity while still having access to +the executable file after the process finishes. Or if you start process +recording (@pxref{Reverse Execution}) and @code{reverse-step} there you are +presented a virtual layer of the recording target while the process target +remains stopped at the chronologically last point of the process execution. + +Use the @code{core-file} and @code{exec-file} commands to select a new core +file or executable target (@pxref{Files, ,Commands to Specify Files}). To +specify as a target a process that is already running, use the @code{attach} +command (@pxref{Attach, ,Debugging an Already-running Process}). @node Target Commands @section Commands for Managing Targets --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -177,3 +177,62 @@ gdb_load ${binfile} gdb_test "up" "#\[0-9\]* *\[0-9xa-fH'\]* in .* \\(.*\\).*" "up in corefile.exp (reinit)" gdb_test "core" "No core file now." + + +# Test a run (start) command will clear any loaded core file. + +gdb_test "core-file $corefile" "Core was generated by .*" "run: load core again" +gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "run: sanity check we see the core file" + +set test "run: with core" +if [runto_main] { + pass $test +} else { + fail $test +} + +set test "run: core file is cleared" +gdb_test_multiple "info files" $test { + -re "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + -re "\r\n$gdb_prompt $" { + pass $test + } +} + +gdb_exit + + +# Test an attach command will clear any loaded core file. + +if ![is_remote target] { + set test "attach: spawn sleep" + set res [remote_spawn host "$binfile sleep"]; + if { $res < 0 || $res == "" } { + fail $test + return + } + set pid [exp_pid -i $res] + # We don't care whether the program is still in the startup phase when we + # attach. + + gdb_start + + gdb_test "core-file $corefile" "Core was generated by .*" "attach: load core again" + gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "attach: sanity check we see the core file" + + gdb_test "attach $pid" "Attaching to process $pid\r\n.*" "attach: with core" + + set test "attach: core file is cleared" + gdb_test_multiple "info files" $test { + -re "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + -re "\r\n$gdb_prompt $" { + pass $test + } + } + + gdb_exit +} --- a/gdb/testsuite/gdb.base/coremaker.c +++ b/gdb/testsuite/gdb.base/coremaker.c @@ -133,8 +133,14 @@ func1 () func2 (); } -int main () +int +main (int argc, char **argv) { + if (argc == 2 && strcmp (argv[1], "sleep") == 0) + { + sleep (60); + return 0; + } mmapdata (); func1 (); return 0; ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-07-08 17:17 ` Jan Kratochvil @ 2010-07-08 18:28 ` Eli Zaretskii 2010-07-19 18:02 ` Jan Kratochvil 2010-07-19 14:37 ` Pedro Alves 1 sibling, 1 reply; 26+ messages in thread From: Eli Zaretskii @ 2010-07-08 18:28 UTC (permalink / raw) To: Jan Kratochvil; +Cc: pedro, mark.kettenis, brobecker, gdb-patches, emachado > Date: Thu, 8 Jul 2010 19:16:48 +0200 > From: Jan Kratochvil <jan.kratochvil@redhat.com> > Cc: Eli Zaretskii <eliz@gnu.org>, mark.kettenis@xs4all.nl, > brobecker@adacore.com, gdb-patches@sourceware.org, > Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com> > > gdb/doc/ > 2010-07-08 Jan Kratochvil <jan.kratochvil@redhat.com> > > Make core files the process_stratum. > * gdb.texinfo (Active Targets): Remove core_stratum. Include > record_stratum example. This part is okay, but please fix the punctuation as shown below: There are multiple classes of targets such, as: processes, executable files or recording sessions. Core files belong to the process class, making core file and process mutually exclusive. Otherwise, @value{GDBN} can work concurrently on multiple active targets, one in each class. This allows you to (for example) start a process and inspect its activity, while still having access to the executable file after the process finishes. Or if you start process recording (@pxref{Reverse Execution}) and @code{reverse-step} there, you are presented a virtual layer of the recording target, while the process target remains stopped at the chronologically last point of the process execution. Thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-07-08 18:28 ` Eli Zaretskii @ 2010-07-19 18:02 ` Jan Kratochvil 2010-07-19 18:05 ` Eli Zaretskii 2010-07-27 16:00 ` Joel Brobecker 0 siblings, 2 replies; 26+ messages in thread From: Jan Kratochvil @ 2010-07-19 18:02 UTC (permalink / raw) To: Eli Zaretskii, Pedro Alves, brobecker Cc: mark.kettenis, gdb-patches, emachado On Thu, 08 Jul 2010 20:25:46 +0200, Eli Zaretskii wrote: > This part is okay, but please fix the punctuation as shown below: > > There are multiple classes of targets such, as: processes, executable files or ^^^^^^^^^^^ Isn't it a typo? But probably it is not, OK, thanks. > recording sessions. Core files belong to the process class, making core file [...] On Mon, 19 Jul 2010 16:37:19 +0200, Pedro Alves wrote: > This is okay, thanks. Checked-in. Joel, requesting approval for the 7.2 branch. There is also less intrusive patch http://cvs.fedoraproject.org/viewvc/rpms/gdb/F-13/gdb-bz594560-core-vs-process.patch?content-type=text%2Fplain&view=co which does not remove the process_stratum. Thanks, Jan http://sourceware.org/ml/gdb-cvs/2010-07/msg00107.html --- src/gdb/ChangeLog 2010/07/19 07:55:41 1.11998 +++ src/gdb/ChangeLog 2010/07/19 17:51:22 1.11999 @@ -1,3 +1,26 @@ +2010-07-19 Jan Kratochvil <jan.kratochvil@redhat.com> + + Make core files the process_stratum. + * corefile.c (core_target): New variable. + (core_file_command): Remove variable t, use core_target. + * corelow.c (core_ops): Make it static. + (init_core_ops): Change to process_stratum. Initialize CORE_TARGET. + * defs.h (make_cleanup_unpush_target): New prototype. + * gdbarch.h: Regenerate. + * gdbarch.sh (core_pid_to_str): Remove core_stratum from its comment. + * gdbcore.h (core_target): New declaration. + * inf-ptrace.c (inf_ptrace_create_inferior, inf_ptrace_attach): New + variables ops_already_pushed and back_to. Use push_target, + make_cleanup_unpush_target and discard_cleanups calls. + * record.c (record_open): Replace core_stratum by a core_bfd check. + * target.c (target_is_pushed): New function. + (find_core_target): Remove. + * target.h (enum strata) <core_stratum>: Remove. + (target_is_pushed): New declaration. + (find_core_target): Remove declaration. + * tracepoint.c (init_tfile_ops) <to_stratum>: Remove comment. + * utils.c (do_unpush_target, make_cleanup_unpush_target): New functions. + 2010-07-19 Hui Zhu <teawater@gmail.com> * breakpoint.c (single_step_breakpoints_inserted): New --- src/gdb/corefile.c 2010/05/13 23:53:32 1.58 +++ src/gdb/corefile.c 2010/07/19 17:51:23 1.59 @@ -59,6 +59,10 @@ /* Binary file diddling handle for the core file. */ bfd *core_bfd = NULL; + +/* corelow.c target (if included for this gdb target). */ + +struct target_ops *core_target; \f /* Backward compatability with old way of specifying core files. */ @@ -66,18 +70,15 @@ void core_file_command (char *filename, int from_tty) { - struct target_ops *t; - dont_repeat (); /* Either way, seems bogus. */ - t = find_core_target (); - if (t == NULL) + if (core_target == NULL) error (_("GDB can't read core files on this machine.")); if (!filename) - (t->to_detach) (t, filename, from_tty); + (core_target->to_detach) (core_target, filename, from_tty); else - (t->to_open) (filename, from_tty); + (core_target->to_open) (filename, from_tty); } \f --- src/gdb/corelow.c 2010/05/13 23:53:32 1.100 +++ src/gdb/corelow.c 2010/07/19 17:51:23 1.101 @@ -100,7 +100,7 @@ void _initialize_corelow (void); -struct target_ops core_ops; +static struct target_ops core_ops; /* An arbitrary identifier for the core inferior. */ #define CORELOW_PID 1 @@ -911,11 +911,17 @@ core_ops.to_thread_alive = core_thread_alive; core_ops.to_read_description = core_read_description; core_ops.to_pid_to_str = core_pid_to_str; - core_ops.to_stratum = core_stratum; + core_ops.to_stratum = process_stratum; core_ops.to_has_memory = core_has_memory; core_ops.to_has_stack = core_has_stack; core_ops.to_has_registers = core_has_registers; core_ops.to_magic = OPS_MAGIC; + + if (core_target) + internal_error (__FILE__, __LINE__, + _("init_core_ops: core target already exists (\"%s\")."), + core_target->to_longname); + core_target = &core_ops; } void --- src/gdb/defs.h 2010/06/26 06:44:47 1.275 +++ src/gdb/defs.h 2010/07/19 17:51:23 1.276 @@ -352,6 +352,9 @@ extern struct cleanup *make_cleanup_restore_integer (int *variable); +struct target_ops; +extern struct cleanup *make_cleanup_unpush_target (struct target_ops *ops); + extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_my_cleanup (struct cleanup **, --- src/gdb/gdbarch.h 2010/07/06 12:56:22 1.417 +++ src/gdb/gdbarch.h 2010/07/19 17:51:23 1.418 @@ -672,8 +672,7 @@ extern LONGEST gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, LONGEST len); extern void set_gdbarch_core_xfer_shared_libraries (struct gdbarch *gdbarch, gdbarch_core_xfer_shared_libraries_ftype *core_xfer_shared_libraries); -/* How the core_stratum layer converts a PTID from a core file to a - string. */ +/* How the core target converts a PTID from a core file to a string. */ extern int gdbarch_core_pid_to_str_p (struct gdbarch *gdbarch); --- src/gdb/gdbarch.sh 2010/07/06 12:56:23 1.513 +++ src/gdb/gdbarch.sh 2010/07/19 17:51:23 1.514 @@ -611,8 +611,7 @@ # core file into buffer READBUF with length LEN. M:LONGEST:core_xfer_shared_libraries:gdb_byte *readbuf, ULONGEST offset, LONGEST len:readbuf, offset, len -# How the core_stratum layer converts a PTID from a core file to a -# string. +# How the core target converts a PTID from a core file to a string. M:char *:core_pid_to_str:ptid_t ptid:ptid # BFD target to use when generating a core file. --- src/gdb/gdbcore.h 2010/01/01 07:31:32 1.38 +++ src/gdb/gdbcore.h 2010/07/19 17:51:23 1.39 @@ -108,6 +108,8 @@ extern bfd *core_bfd; +extern struct target_ops *core_target; + /* Whether to open exec and core files read-only or read-write. */ extern int write_files; --- src/gdb/inf-ptrace.c 2010/02/15 17:35:49 1.70 +++ src/gdb/inf-ptrace.c 2010/07/19 17:51:23 1.71 @@ -121,10 +121,23 @@ { int pid; + /* Do not change either targets above or the same target if already present. + The reason is the target stack is shared across multiple inferiors. */ + int ops_already_pushed = target_is_pushed (ops); + struct cleanup *back_to; + + if (! ops_already_pushed) + { + /* Clear possible core file with its process_stratum. */ + push_target (ops); + back_to = make_cleanup_unpush_target (ops); + } + pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL, NULL, NULL); - push_target (ops); + if (! ops_already_pushed) + discard_cleanups (back_to); /* On some targets, there must be some explicit synchronization between the parent and child processes after the debugger @@ -189,11 +202,24 @@ pid_t pid; struct inferior *inf; + /* Do not change either targets above or the same target if already present. + The reason is the target stack is shared across multiple inferiors. */ + int ops_already_pushed = target_is_pushed (ops); + struct cleanup *back_to; + pid = parse_pid_to_attach (args); if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); + if (! ops_already_pushed) + { + /* target_pid_to_str already uses the target. Also clear possible core + file with its process_stratum. */ + push_target (ops); + back_to = make_cleanup_unpush_target (ops); + } + if (from_tty) { exec_file = get_exec_file (0); @@ -226,7 +252,8 @@ target, it should decorate the ptid later with more info. */ add_thread_silent (inferior_ptid); - push_target(ops); + if (! ops_already_pushed) + discard_cleanups (back_to); } #ifdef PT_GET_PROCESS_STATE --- src/gdb/record.c 2010/07/19 07:55:43 1.51 +++ src/gdb/record.c 2010/07/19 17:51:23 1.52 @@ -962,7 +962,7 @@ record_beneath_to_stopped_by_watchpoint = tmp_to_stopped_by_watchpoint; record_beneath_to_stopped_data_address = tmp_to_stopped_data_address; - if (current_target.to_stratum == core_stratum) + if (core_bfd) record_core_open_1 (name, from_tty); else record_open_1 (name, from_tty); --- src/gdb/target.c 2010/07/16 20:04:41 1.260 +++ src/gdb/target.c 2010/07/19 17:51:23 1.261 @@ -1037,6 +1037,30 @@ pop_all_targets_above (dummy_stratum, quitting); } +/* Return 1 if T is now pushed in the target stack. Return 0 otherwise. */ + +int +target_is_pushed (struct target_ops *t) +{ + struct target_ops **cur; + + /* Check magic number. If wrong, it probably means someone changed + the struct definition, but not all the places that initialize one. */ + if (t->to_magic != OPS_MAGIC) + { + fprintf_unfiltered (gdb_stderr, + "Magic number of %s target struct wrong\n", + t->to_shortname); + internal_error (__FILE__, __LINE__, _("failed internal consistency check")); + } + + for (cur = &target_stack; (*cur) != NULL; cur = &(*cur)->beneath) + if (*cur == t) + return 1; + + return 0; +} + /* Using the objfile specified in OBJFILE, find the address for the current thread's thread-local storage with offset OFFSET. */ CORE_ADDR @@ -2770,31 +2794,6 @@ return (count == 1 ? runable : NULL); } -/* Find a single core_stratum target in the list of targets and return it. - If for some reason there is more than one, return NULL. */ - -struct target_ops * -find_core_target (void) -{ - struct target_ops **t; - struct target_ops *runable = NULL; - int count; - - count = 0; - - for (t = target_structs; t < target_structs + target_struct_size; - ++t) - { - if ((*t)->to_stratum == core_stratum) - { - runable = *t; - ++count; - } - } - - return (count == 1 ? runable : NULL); -} - /* * Find the next target down the stack from the specified target. */ --- src/gdb/target.h 2010/07/07 16:15:18 1.185 +++ src/gdb/target.h 2010/07/19 17:51:23 1.186 @@ -68,8 +68,7 @@ { dummy_stratum, /* The lowest of the low */ file_stratum, /* Executable files, etc */ - core_stratum, /* Core dump files */ - process_stratum, /* Executing processes */ + process_stratum, /* Executing processes or core dump files */ thread_stratum, /* Executing threads */ record_stratum, /* Support record debugging */ arch_stratum /* Architecture overrides */ @@ -1485,6 +1484,8 @@ strictly above ABOVE_STRATUM. */ extern void pop_all_targets_above (enum strata above_stratum, int quitting); +extern int target_is_pushed (struct target_ops *t); + extern CORE_ADDR target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset); @@ -1546,8 +1547,6 @@ extern struct target_ops *find_run_target (void); -extern struct target_ops *find_core_target (void); - extern struct target_ops *find_target_beneath (struct target_ops *); /* Read OS data object of type TYPE from the target, and return it in --- src/gdb/tracepoint.c 2010/07/01 15:36:17 1.191 +++ src/gdb/tracepoint.c 2010/07/19 17:51:23 1.192 @@ -4098,8 +4098,6 @@ tfile_ops.to_get_trace_status = tfile_get_trace_status; tfile_ops.to_trace_find = tfile_trace_find; tfile_ops.to_get_trace_state_variable_value = tfile_get_trace_state_variable_value; - /* core_stratum might seem more logical, but GDB doesn't like having - more than one core_stratum vector. */ tfile_ops.to_stratum = process_stratum; tfile_ops.to_has_all_memory = tfile_has_all_memory; tfile_ops.to_has_memory = tfile_has_memory; --- src/gdb/utils.c 2010/06/26 06:44:47 1.236 +++ src/gdb/utils.c 2010/07/19 17:51:23 1.237 @@ -352,6 +352,24 @@ xfree); } +/* Helper for make_cleanup_unpush_target. */ + +static void +do_unpush_target (void *arg) +{ + struct target_ops *ops = arg; + + unpush_target (ops); +} + +/* Return a new cleanup that unpushes OPS. */ + +struct cleanup * +make_cleanup_unpush_target (struct target_ops *ops) +{ + return make_my_cleanup (&cleanup_chain, do_unpush_target, ops); +} + struct cleanup * make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function, void *arg, void (*free_arg) (void *)) --- src/gdb/doc/ChangeLog 2010/07/13 20:51:34 1.1086 +++ src/gdb/doc/ChangeLog 2010/07/19 17:51:24 1.1087 @@ -1,3 +1,10 @@ +2010-07-19 Jan Kratochvil <jan.kratochvil@redhat.com> + Eli Zaretskii <eliz@gnu.org> + + Make core files the process_stratum. + * gdb.texinfo (Active Targets): Remove core_stratum. Include + record_stratum example. + 2010-07-13 Tom Tromey <tromey@redhat.com> * gdb.texinfo (Index Files): New node. --- src/gdb/doc/gdb.texinfo 2010/07/13 20:51:34 1.740 +++ src/gdb/doc/gdb.texinfo 2010/07/19 17:51:24 1.741 @@ -15391,33 +15391,20 @@ @cindex active targets @cindex multiple targets -There are three classes of targets: processes, core files, and -executable files. @value{GDBN} can work concurrently on up to three -active targets, one in each class. This allows you to (for example) -start a process and inspect its activity without abandoning your work on -a core file. - -For example, if you execute @samp{gdb a.out}, then the executable file -@code{a.out} is the only active target. If you designate a core file as -well---presumably from a prior run that crashed and coredumped---then -@value{GDBN} has two active targets and uses them in tandem, looking -first in the corefile target, then in the executable file, to satisfy -requests for memory addresses. (Typically, these two classes of target -are complementary, since core files contain only a program's -read-write memory---variables and so on---plus machine status, while -executable files contain only the program text and initialized data.) - -When you type @code{run}, your executable file becomes an active process -target as well. When a process target is active, all @value{GDBN} -commands requesting memory addresses refer to that target; addresses in -an active core file or executable file target are obscured while the -process target is active. - -Use the @code{core-file} and @code{exec-file} commands to select a new -core file or executable target (@pxref{Files, ,Commands to Specify -Files}). To specify as a target a process that is already running, use -the @code{attach} command (@pxref{Attach, ,Debugging an Already-running -Process}). +There are multiple classes of targets such, as: processes, executable files or +recording sessions. Core files belong to the process class, making core file +and process mutually exclusive. Otherwise, @value{GDBN} can work concurrently +on multiple active targets, one in each class. This allows you to (for +example) start a process and inspect its activity, while still having access to +the executable file after the process finishes. Or if you start process +recording (@pxref{Reverse Execution}) and @code{reverse-step} there, you are +presented a virtual layer of the recording target, while the process target +remains stopped at the chronologically last point of the process execution. + +Use the @code{core-file} and @code{exec-file} commands to select a new core +file or executable target (@pxref{Files, ,Commands to Specify Files}). To +specify as a target a process that is already running, use the @code{attach} +command (@pxref{Attach, ,Debugging an Already-running Process}). @node Target Commands @section Commands for Managing Targets --- src/gdb/testsuite/ChangeLog 2010/07/14 14:54:58 1.2381 +++ src/gdb/testsuite/ChangeLog 2010/07/19 17:51:24 1.2382 @@ -1,3 +1,13 @@ +2010-07-19 Jan Kratochvil <jan.kratochvil@redhat.com> + + Make core files the process_stratum. + * gdb.base/corefile.exp (run: load core again) + (run: sanity check we see the core file, run: with core) + (run: core file is cleared, attach: load core again) + (attach: sanity check we see the core file, attach: with core) + (attach: core file is cleared): New tests. + * gdb.base/coremaker.c (main): New parameters. Implement "sleep" argv. + 2010-07-14 Ken Werner <ken.werner@de.ibm.com> * gdb.arch/altivec-abi.exp: New tests. --- src/gdb/testsuite/gdb.base/corefile.exp 2010/05/24 22:03:59 1.21 +++ src/gdb/testsuite/gdb.base/corefile.exp 2010/07/19 17:51:25 1.22 @@ -177,3 +177,62 @@ gdb_test "up" "#\[0-9\]* *\[0-9xa-fH'\]* in .* \\(.*\\).*" "up in corefile.exp (reinit)" gdb_test "core" "No core file now." + + +# Test a run (start) command will clear any loaded core file. + +gdb_test "core-file $corefile" "Core was generated by .*" "run: load core again" +gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "run: sanity check we see the core file" + +set test "run: with core" +if [runto_main] { + pass $test +} else { + fail $test +} + +set test "run: core file is cleared" +gdb_test_multiple "info files" $test { + -re "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + -re "\r\n$gdb_prompt $" { + pass $test + } +} + +gdb_exit + + +# Test an attach command will clear any loaded core file. + +if ![is_remote target] { + set test "attach: spawn sleep" + set res [remote_spawn host "$binfile sleep"]; + if { $res < 0 || $res == "" } { + fail $test + return + } + set pid [exp_pid -i $res] + # We don't care whether the program is still in the startup phase when we + # attach. + + gdb_start + + gdb_test "core-file $corefile" "Core was generated by .*" "attach: load core again" + gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "attach: sanity check we see the core file" + + gdb_test "attach $pid" "Attaching to process $pid\r\n.*" "attach: with core" + + set test "attach: core file is cleared" + gdb_test_multiple "info files" $test { + -re "\r\nLocal core dump file:\r\n.*\r\n$gdb_prompt $" { + fail $test + } + -re "\r\n$gdb_prompt $" { + pass $test + } + } + + gdb_exit +} --- src/gdb/testsuite/gdb.base/coremaker.c 2010/01/01 07:32:00 1.8 +++ src/gdb/testsuite/gdb.base/coremaker.c 2010/07/19 17:51:25 1.9 @@ -133,8 +133,14 @@ func2 (); } -int main () +int +main (int argc, char **argv) { + if (argc == 2 && strcmp (argv[1], "sleep") == 0) + { + sleep (60); + return 0; + } mmapdata (); func1 (); return 0; ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-07-19 18:02 ` Jan Kratochvil @ 2010-07-19 18:05 ` Eli Zaretskii 2010-07-19 18:16 ` Jan Kratochvil 2010-07-27 16:00 ` Joel Brobecker 1 sibling, 1 reply; 26+ messages in thread From: Eli Zaretskii @ 2010-07-19 18:05 UTC (permalink / raw) To: Jan Kratochvil; +Cc: pedro, brobecker, mark.kettenis, gdb-patches, emachado > Date: Mon, 19 Jul 2010 20:01:17 +0200 > From: Jan Kratochvil <jan.kratochvil@redhat.com> > Cc: mark.kettenis@xs4all.nl, gdb-patches@sourceware.org, > emachado@linux.vnet.ibm.com > > On Thu, 08 Jul 2010 20:25:46 +0200, Eli Zaretskii wrote: > > This part is okay, but please fix the punctuation as shown below: > > > > There are multiple classes of targets such, as: processes, executable files or > ^^^^^^^^^^^ > Isn't it a typo? But probably it is not, OK, thanks. If you mean the comma after "such", then yes, it's better be removed. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-07-19 18:05 ` Eli Zaretskii @ 2010-07-19 18:16 ` Jan Kratochvil 0 siblings, 0 replies; 26+ messages in thread From: Jan Kratochvil @ 2010-07-19 18:16 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pedro, brobecker, mark.kettenis, gdb-patches, emachado On Mon, 19 Jul 2010 20:03:55 +0200, Eli Zaretskii wrote: > > On Thu, 08 Jul 2010 20:25:46 +0200, Eli Zaretskii wrote: > > > This part is okay, but please fix the punctuation as shown below: > > > > > > There are multiple classes of targets such, as: processes, executable files or > > ^^^^^^^^^^^ > > Isn't it a typo? But probably it is not, OK, thanks. > > If you mean the comma after "such", then yes, it's better be removed. Checked-in this one. Thanks, Jan http://sourceware.org/ml/gdb-cvs/2010-07/msg00108.html --- src/gdb/doc/ChangeLog 2010/07/19 17:51:24 1.1087 +++ src/gdb/doc/ChangeLog 2010/07/19 18:11:31 1.1088 @@ -1,4 +1,8 @@ 2010-07-19 Jan Kratochvil <jan.kratochvil@redhat.com> + + * gdb.texinfo (Active Targets): Fix wrong comma placement. + +2010-07-19 Jan Kratochvil <jan.kratochvil@redhat.com> Eli Zaretskii <eliz@gnu.org> Make core files the process_stratum. --- src/gdb/doc/gdb.texinfo 2010/07/19 17:51:24 1.741 +++ src/gdb/doc/gdb.texinfo 2010/07/19 18:11:31 1.742 @@ -15391,7 +15391,7 @@ @cindex active targets @cindex multiple targets -There are multiple classes of targets such, as: processes, executable files or +There are multiple classes of targets such as: processes, executable files or recording sessions. Core files belong to the process class, making core file and process mutually exclusive. Otherwise, @value{GDBN} can work concurrently on multiple active targets, one in each class. This allows you to (for ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-07-19 18:02 ` Jan Kratochvil 2010-07-19 18:05 ` Eli Zaretskii @ 2010-07-27 16:00 ` Joel Brobecker 1 sibling, 0 replies; 26+ messages in thread From: Joel Brobecker @ 2010-07-27 16:00 UTC (permalink / raw) To: Jan Kratochvil Cc: Eli Zaretskii, Pedro Alves, mark.kettenis, gdb-patches, emachado > Joel, requesting approval for the 7.2 branch. > There is also less intrusive patch > http://cvs.fedoraproject.org/viewvc/rpms/gdb/F-13/gdb-bz594560-core-vs-process.patch?content-type=text%2Fplain&view=co > which does not remove the process_stratum. I am inclined hold off this patch off the branch (patch is too intrusive this late in the game, problem is too minor [IMO]). But Pedro has a better understanding of the patch, and if he thinks it's safe to put in, then that's good enough to go in 7.2. -- Joel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-07-08 17:17 ` Jan Kratochvil 2010-07-08 18:28 ` Eli Zaretskii @ 2010-07-19 14:37 ` Pedro Alves 1 sibling, 0 replies; 26+ messages in thread From: Pedro Alves @ 2010-07-19 14:37 UTC (permalink / raw) To: Jan Kratochvil Cc: Eli Zaretskii, mark.kettenis, brobecker, gdb-patches, Edjunior Barbosa Machado On Thursday 08 July 2010 18:16:48, Jan Kratochvil wrote: > gdb/ > 2010-07-08 Jan Kratochvil <jan.kratochvil@redhat.com> > > Make core files the process_stratum. > * corefile.c (core_target): New variable. > (core_file_command): Remove variable t, use core_target. > * corelow.c (core_ops): Make it static. > (init_core_ops): Change to process_stratum. Initialize CORE_TARGET. > * defs.h (make_cleanup_unpush_target): New prototype. > * gdbarch.h: Regenerate. > * gdbarch.sh (core_pid_to_str): Remove core_stratum from its comment. > * gdbcore.h (core_target): New declaration. > * inf-ptrace.c (inf_ptrace_create_inferior, inf_ptrace_attach): New > variables ops_already_pushed and back_to. Use push_target, > make_cleanup_unpush_target and discard_cleanups calls. > * record.c (record_open): Replace core_stratum by a core_bfd check. > * target.c (target_is_pushed): New function. > (find_core_target): Remove. > * target.h (enum strata) <core_stratum>: Remove. > (target_is_pushed): New declaration. > (find_core_target): Remove declaration. > * tracepoint.c (init_tfile_ops) <to_stratum>: Remove comment. > * utils.c (do_unpush_target, make_cleanup_unpush_target): New functions. This is okay, thanks. > gdb/doc/ > 2010-07-08 Jan Kratochvil <jan.kratochvil@redhat.com> > > Make core files the process_stratum. > * gdb.texinfo (Active Targets): Remove core_stratum. Include > record_stratum example. -- Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-23 19:38 ` Jan Kratochvil 2010-05-23 21:08 ` Eli Zaretskii @ 2010-05-23 21:16 ` Pedro Alves 1 sibling, 0 replies; 26+ messages in thread From: Pedro Alves @ 2010-05-23 21:16 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Mark Kettenis, Joel Brobecker, gdb-patches On Sunday 23 May 2010 19:54:40, Jan Kratochvil wrote: > On Fri, 21 May 2010 17:06:07 +0200, Pedro Alves wrote: > > I don't think it makes that much sense nowadays to have a > > distinct core_stratum vs process_stratum. > > OK, removed core_stratum. Thanks. > @@ -194,6 +195,10 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) > if (pid == getpid ()) /* Trying to masturbate? */ > error (_("I refuse to debug myself!")); > > + /* target_pid_to_str already uses the target. Clear possible core file with > + its process_stratum. */ > + pop_all_targets_above (ops->to_stratum - 1, 0); > + Unfortunately, this would break multiprocess. For example, you may be attaching to your second process (e.g., "start", "add-inferior", "inferior 2", "attach $pid"), and the thread stratum needs to remain pushed for the first inferior/process. > + /* Clear possible core file with its process_stratum. */ > + push_target (ops); > + > pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL, > NULL, NULL); > > - push_target (ops); This makes sense, as indeed fork_inferior touches the current target, even today, but, can you check what happens if fork_inferior throws an error? I worry that if, for example, exec fails, we still end up with the target pushed. Probably not a big issue for linux-nat.c, which since it support multi-process, handles being pushed with inferior_ptid == null_ptid, but other ptrace targets don't. > On Fri, 21 May 2010 17:02:31 +0200, Joel Brobecker wrote: > > > +set test "start with core" > > > +gdb_test_multiple "start" $test { > > > + -re {Core file is already loaded. Unload it[?] [(]y or n[)] } { > > > + pass $test > > > + } > > > +} > > > > We should not use the "start" command in testcases, as it does not > > work with remote targets. I'm afraid we're going to have to rely on > > the run command instead. There's some confusion here. "start" doesn't work with "target remote", just as much as "run" doesn't work. In other words, if "run" would work, so would "start", as the latter is just the former plus an internal temporary breakpoint on `main'. > OK, used runto_main now. Attach test does not run with gdbserver the same way > gdb.base/attach.exp does not. Tested this testfile with runtest-gdbserver > (only on localhost). -- Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:02 ` Mark Kettenis 2010-05-21 15:05 ` Joel Brobecker @ 2010-05-21 15:05 ` Pedro Alves 2010-05-21 15:54 ` Mark Kettenis 2010-05-21 15:07 ` Jan Kratochvil 2 siblings, 1 reply; 26+ messages in thread From: Pedro Alves @ 2010-05-21 15:05 UTC (permalink / raw) To: gdb-patches; +Cc: Mark Kettenis, jan.kratochvil On Friday 21 May 2010 15:47:07, Mark Kettenis wrote: > I often start gdb and load a core file to investigate a problem. Then > I set a breakpoint at some point before the crash and run the program > again. This used to work just fine. If you're refering to getting back to debugging the core when the running program exits, it never worked correctly. You'd get a better experience if your core had no threads at all. If you didn't trip on an assertion, and weird problems for having gdb consult things in the process target, then the core target (which wouldn't make sense for the running program), then the exec target, when you'd get back to debugging the core, you'd find your threads had disapeared. That's just an example. Here are others <http://sourceware.org/ml/gdb-patches/2008-08/msg00290.html>. For this to work correctly, we'd need to rethink the single target-stack, into maybe multiple target stacks, or something else radically different. -- Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:05 ` Pedro Alves @ 2010-05-21 15:54 ` Mark Kettenis 2010-05-21 16:08 ` Pedro Alves 0 siblings, 1 reply; 26+ messages in thread From: Mark Kettenis @ 2010-05-21 15:54 UTC (permalink / raw) To: pedro; +Cc: gdb-patches, jan.kratochvil > From: Pedro Alves <pedro@codesourcery.com> > Date: Fri, 21 May 2010 16:05:01 +0100 > > On Friday 21 May 2010 15:47:07, Mark Kettenis wrote: > > I often start gdb and load a core file to investigate a problem. Then > > I set a breakpoint at some point before the crash and run the program > > again. This used to work just fine. > > If you're refering to getting back to debugging the core when > the running program exits, it never worked correctly. You'd get a better > experience if your core had no threads at all. If you didn't trip on an > assertion, and weird problems for having gdb consult things in the process > target, then the core target (which wouldn't make sense for the running > program), then the exec target, when you'd get back to debugging > the core, you'd find your threads had disapeared. That's just an example. > Here are others <http://sourceware.org/ml/gdb-patches/2008-08/msg00290.html>. I don't think I go back again to the core file very often, but it seems to work fine with gdb 6.3 for a single-threaded process. > For this to work correctly, we'd need to rethink the single target-stack, > into maybe multiple target stacks, or something else radically different. Dunno. Isn't it enough to pop the core layer when you run? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:54 ` Mark Kettenis @ 2010-05-21 16:08 ` Pedro Alves 0 siblings, 0 replies; 26+ messages in thread From: Pedro Alves @ 2010-05-21 16:08 UTC (permalink / raw) To: gdb-patches; +Cc: Mark Kettenis, jan.kratochvil On Friday 21 May 2010 16:37:44, Mark Kettenis wrote: > > From: Pedro Alves <pedro@codesourcery.com> > > Date: Fri, 21 May 2010 16:05:01 +0100 > > > > On Friday 21 May 2010 15:47:07, Mark Kettenis wrote: > > > I often start gdb and load a core file to investigate a problem. Then > > > I set a breakpoint at some point before the crash and run the program > > > again. This used to work just fine. > > > > If you're refering to getting back to debugging the core when > > the running program exits, it never worked correctly. You'd get a better > > experience if your core had no threads at all. If you didn't trip on an > > assertion, and weird problems for having gdb consult things in the process > > target, then the core target (which wouldn't make sense for the running > > program), then the exec target, when you'd get back to debugging > > the core, you'd find your threads had disapeared. That's just an example. > > Here are others <http://sourceware.org/ml/gdb-patches/2008-08/msg00290.html>. > > I don't think I go back again to the core file very often, but it > seems to work fine with gdb 6.3 for a single-threaded process. Yes, that's the case I said would most likely work for the basic things in the other mail. > > > For this to work correctly, we'd need to rethink the single target-stack, > > into maybe multiple target stacks, or something else radically different. > > Dunno. Isn't it enough to pop the core layer when you run? Errr, if you pop it, then what are you getting back to? I was talking about a design that allows correct debugging of the process, while still debugging the core. poping the core layer, clearing all current inferior state, and later reload the core works around it, of course... but that's not the same as leaving the core target on that stack, pushing the process statum, allowing debugging it, and once that is poped, assume the core target and its inferior are in a sane gdb internal state, which is what gdb assumes today. -- Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:02 ` Mark Kettenis 2010-05-21 15:05 ` Joel Brobecker 2010-05-21 15:05 ` Pedro Alves @ 2010-05-21 15:07 ` Jan Kratochvil 2 siblings, 0 replies; 26+ messages in thread From: Jan Kratochvil @ 2010-05-21 15:07 UTC (permalink / raw) To: Mark Kettenis; +Cc: gdb-patches On Fri, 21 May 2010 16:47:07 +0200, Mark Kettenis wrote: > > Date: Fri, 21 May 2010 15:47:19 +0200 > > From: Jan Kratochvil <jan.kratochvil@redhat.com> > > > > Hi, > > > > there is already a protection against loading a core file when a program is > > running. > > That makes sense. But what you are suggesting doesn't. > > I often start gdb and load a core file to investigate a problem. Then > I set a breakpoint at some point before the crash and run the program > again. This used to work just fine. What do you suggest otherwise? If core-and-then-run should work then we must make working also run-and-then-core (fine with that). With both core file and running process the core file memory gets ignored. So I find unloading a core file as just making the situation more simple. (a) Do you find the "Unload it?" question too distrurbing? (b) Do you find essential to see the core file memory again after the process terminates? Thanks, Jan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 14:47 [patch] Forbid run with a core file loaded Jan Kratochvil 2010-05-21 15:02 ` Mark Kettenis @ 2010-05-21 15:04 ` Joel Brobecker 2010-05-21 15:06 ` Pedro Alves 2 siblings, 0 replies; 26+ messages in thread From: Joel Brobecker @ 2010-05-21 15:04 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches > (gdb) start > Starting program: /bin/sleep > ^^^^^^^^^^^^^^^^ !!! > [...] Hah! I was convinced we would ask confirmation to the user in this case... I agree we should. > 2010-05-21 Jan Kratochvil <jan.kratochvil@redhat.com> > > Forbid run with a core file loaded. > * infcmd.c (run_command_1) <core_bfd>: New. This looks fine to me (but let's wait for Mark to confirm that there was a misunderstanding - I'll followup separately). > gdb/testsuite/ > 2010-05-21 Jan Kratochvil <jan.kratochvil@redhat.com> > > Forbid run with a core file loaded. > * gdb.base/corefile.exp (load core again, start with core) > (started with core): New tests. > +set test "start with core" > +gdb_test_multiple "start" $test { > + -re {Core file is already loaded. Unload it[?] [(]y or n[)] } { > + pass $test > + } > +} We should not use the "start" command in testcases, as it does not work with remote targets. I'm afraid we're going to have to rely on the run command instead. -- Joel ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 14:47 [patch] Forbid run with a core file loaded Jan Kratochvil 2010-05-21 15:02 ` Mark Kettenis 2010-05-21 15:04 ` Joel Brobecker @ 2010-05-21 15:06 ` Pedro Alves 2010-05-21 15:15 ` Joel Brobecker 2 siblings, 1 reply; 26+ messages in thread From: Pedro Alves @ 2010-05-21 15:06 UTC (permalink / raw) To: gdb-patches; +Cc: Jan Kratochvil On Friday 21 May 2010 14:47:19, Jan Kratochvil wrote: > Hi, > > there is already a protection against loading a core file when a program is > running. > > But one still can now run a program when a core file is loaded. Moreover GDB > then crashes on `quit'. > > (gdb) file sleep > [...] > (gdb) start > [...] > (gdb) core-file core.5841 > A program is being debugged already. Kill it? (y or n) y > [...] > Program terminated with signal 11, Segmentation fault. > [...] > (gdb) start > Starting program: /bin/sleep > ^^^^^^^^^^^^^^^^ !!! That's sort of expected. From gdb.texinfo: "There are three classes of targets: processes, core files, and executable files. @value{GDBN} can work concurrently on up to three active targets, one in each class. This allows you to (for example) start a process and inspect its activity without abandoning your work on a core file." This doesn't, and I think never has worked 100% correctly, except maybe on single-threaded targets, on older GDBs. Nowadays, it's really not expected and supported to have the core_ops target remain on the target stack simultaneously and below a process_stratum target (linux-nat.c, in your case). I don't think it makes that much sense nowadays to have a distinct core_stratum vs process_stratum. It core_ops was a process_stratum, pushing linux-nat.c would auto discard the core, as you can't have two targets of the same stratum on the stack. The only main use for that stratum nowadays is for find_core_target, I think. > [...] > (gdb) quit > A debugging session is active. > Inferior 1 [process 13887] will be killed. > Quit anyway? (y or n) y > inferior.c:362: internal-error: find_inferior_pid: Assertion `pid != 0' failed. > > gdb/ > 2010-05-21 Jan Kratochvil <jan.kratochvil@redhat.com> > > Forbid run with a core file loaded. > * infcmd.c (run_command_1) <core_bfd>: New. What about "attach"? > > gdb/testsuite/ > 2010-05-21 Jan Kratochvil <jan.kratochvil@redhat.com> > > Forbid run with a core file loaded. > * gdb.base/corefile.exp (load core again, start with core) > (started with core): New tests. > > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -483,6 +483,15 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) > > dont_repeat (); > > + if (core_bfd) > + { > + if (!from_tty > + || query (_("Core file is already loaded. Unload it? "))) > + core_file_command (NULL, from_tty); > + if (core_bfd) > + error (_("Core file not unloaded.")); > + } > + I wish this would query a target property instead of poking core_bfd directly, but I don't see what. Several targets call target_preopen to clean up the previous different target, which does more cleanup than just getting rid of the core target, but it looks a bit messy to harmonise this. So I'm fine with going this route. "is already loaded" sort of implies (to my ears) that you are trying to load another core. "load a core --- what do yo mean ? I _already_ have one loaded." Taking from attach_command, how about: "A core file is being debugged. Unload it? " Anyway, that's just a suggestion. Ignore that one if you don't like it. -- Pedro Alves ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [patch] Forbid run with a core file loaded 2010-05-21 15:06 ` Pedro Alves @ 2010-05-21 15:15 ` Joel Brobecker 0 siblings, 0 replies; 26+ messages in thread From: Joel Brobecker @ 2010-05-21 15:15 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches, Jan Kratochvil > Taking from attach_command, how about: > > "A core file is being debugged. Unload it? " I like that one. -- Joel ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2010-07-27 16:00 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-05-21 14:47 [patch] Forbid run with a core file loaded Jan Kratochvil 2010-05-21 15:02 ` Mark Kettenis 2010-05-21 15:05 ` Joel Brobecker 2010-05-21 15:40 ` Mark Kettenis 2010-05-23 19:38 ` Jan Kratochvil 2010-05-23 21:08 ` Eli Zaretskii 2010-06-06 19:51 ` Jan Kratochvil 2010-06-06 23:08 ` Eli Zaretskii 2010-06-07 11:21 ` Pedro Alves 2010-06-08 2:33 ` Tom Tromey 2010-06-08 11:03 ` Pedro Alves 2010-07-08 17:17 ` Jan Kratochvil 2010-07-08 18:28 ` Eli Zaretskii 2010-07-19 18:02 ` Jan Kratochvil 2010-07-19 18:05 ` Eli Zaretskii 2010-07-19 18:16 ` Jan Kratochvil 2010-07-27 16:00 ` Joel Brobecker 2010-07-19 14:37 ` Pedro Alves 2010-05-23 21:16 ` Pedro Alves 2010-05-21 15:05 ` Pedro Alves 2010-05-21 15:54 ` Mark Kettenis 2010-05-21 16:08 ` Pedro Alves 2010-05-21 15:07 ` Jan Kratochvil 2010-05-21 15:04 ` Joel Brobecker 2010-05-21 15:06 ` Pedro Alves 2010-05-21 15:15 ` Joel Brobecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).