From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5061 invoked by alias); 7 Jun 2010 11:21:10 -0000 Received: (qmail 5050 invoked by uid 22791); 7 Jun 2010 11:21:09 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 07 Jun 2010 11:21:04 +0000 Received: (qmail 25269 invoked from network); 7 Jun 2010 11:21:02 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 7 Jun 2010 11:21:02 -0000 From: Pedro Alves To: Jan Kratochvil Subject: Re: [patch] Forbid run with a core file loaded Date: Mon, 07 Jun 2010 11:21:00 -0000 User-Agent: KMail/1.13.2 (Linux/2.6.32-22-generic; KDE/4.4.2; x86_64; ; ) Cc: Eli Zaretskii , mark.kettenis@xs4all.nl, brobecker@adacore.com, gdb-patches@sourceware.org References: <20100606195033.GA9710@host0.dyn.jankratochvil.net> In-Reply-To: <20100606195033.GA9710@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201006071220.58289.pedro@codesourcery.com> 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-06/txt/msg00167.txt.bz2 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; ; 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