From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24237 invoked by alias); 19 Jul 2010 18:02:08 -0000 Received: (qmail 23599 invoked by uid 22791); 19 Jul 2010 18:02:04 -0000 X-SWARE-Spam-Status: No, hits=-5.2 required=5.0 tests=AWL,BAYES_00,FAKE_REPLY_C,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Jul 2010 18:01:52 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o6JI1NX1005969 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 19 Jul 2010 14:01:23 -0400 Received: from host1.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o6JI1JxD023774 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 19 Jul 2010 14:01:22 -0400 Received: from host1.dyn.jankratochvil.net (localhost [127.0.0.1]) by host1.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id o6JI1Jn2029688; Mon, 19 Jul 2010 20:01:19 +0200 Received: (from jkratoch@localhost) by host1.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id o6JI1Hxv029682; Mon, 19 Jul 2010 20:01:17 +0200 Date: Mon, 19 Jul 2010 18:02:00 -0000 From: Jan Kratochvil To: Eli Zaretskii , Pedro Alves , brobecker@adacore.com Cc: mark.kettenis@xs4all.nl, gdb-patches@sourceware.org, emachado@linux.vnet.ibm.com Subject: Re: [patch] Forbid run with a core file loaded Message-ID: <20100719180117.GA25464@host1.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201007191537.20178.pedro@codesourcery.com> <83eifd7rv9.fsf@gnu.org> User-Agent: Mutt/1.5.20 (2009-12-10) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-07/txt/msg00289.txt.bz2 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 + + 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) : Remove. + (target_is_pushed): New declaration. + (find_core_target): Remove declaration. + * tracepoint.c (init_tfile_ops) : Remove comment. + * utils.c (do_unpush_target, make_cleanup_unpush_target): New functions. + 2010-07-19 Hui Zhu * 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; /* 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); } --- 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 + Eli Zaretskii + + Make core files the process_stratum. + * gdb.texinfo (Active Targets): Remove core_stratum. Include + record_stratum example. + 2010-07-13 Tom Tromey * 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 + + 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 * 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;