From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12108 invoked by alias); 23 Dec 2005 19:45:15 -0000 Received: (qmail 12095 invoked by uid 22791); 23 Dec 2005 19:45:14 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 23 Dec 2005 19:45:12 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id jBNJjAA3020976 for ; Fri, 23 Dec 2005 14:45:11 -0500 Received: from potter.sfbay.redhat.com (potter.sfbay.redhat.com [172.16.27.15]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id jBNJjA116222; Fri, 23 Dec 2005 14:45:10 -0500 Received: from [172.16.24.50] (bluegiant.sfbay.redhat.com [172.16.24.50]) by potter.sfbay.redhat.com (8.12.8/8.12.8) with ESMTP id jBNJj8Va017166; Fri, 23 Dec 2005 14:45:08 -0500 Message-ID: <43AC53C3.3020902@redhat.com> Date: Sat, 24 Dec 2005 15:25:00 -0000 From: Michael Snyder User-Agent: Mozilla Thunderbird (X11/20050322) MIME-Version: 1.0 To: Daniel Jacobowitz CC: Michael Snyder , gdb-patches@sources.redhat.com Subject: Re: [RFA] Linux Checkpoint/Restart, take 2 References: <43A777EE.50404@cisco.com> <20051223142940.GA24997@nevyn.them.org> In-Reply-To: <20051223142940.GA24997@nevyn.them.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2005-12/txt/msg00279.txt.bz2 Daniel Jacobowitz wrote: > You only enable this for i386-linux. Please enable it everywhere that > the current implementation is expected to work, i.e. everywhere that uses > linux-nat.c. In fact you'd better do that, because otherwise you've > broken the build of all other native GNU/Linux targets :-) OK, although I have only tested it on x86. There's really nothing architecture-dependant in it. > The implementation is almost entirely contained in linux-nat.c and > linux-fork.c. Maybe we should discuss the target vector interface to > this before it goes in? For instance, there's no reason a Linux-hosted > gdbserver shouldn't be able to implement exactly the same thing. No, and I fervently hope that other targets will implement something like it too (I may work on some of them). However, the current patch is for native linux only, and does not *prevent* any other targets from implementing checkpoints. That's why I defined all the commands in a linux-only module. I'd like to check in what I have now, and do the target-vectorization as needed. Having this implementation to play with will help us define what target vectors we need. > I totally don't understand what the clobber_regs bits are for. You're > using fork as a backend; each saved fork had better have both its own > registers, right? Is it just a GDB internal bookkeeping thing? If so, > why do you need to do it for saved forks and not for the existing > follow-fork bits? No, it actually was needed, and it's a little obscure, so I'll give you an explanation as a footnote, down below. ;-) > I was somewhat amazed to find out that the same is not true of file > descriptor offsets. In fact I had to go write a test program for it. Me too. ;-) But that's what the man page says.... > Could you make a pass through comment formatting, please? Not a lot, but > enough variety that it jumped out at me, e.g. */ shouldn't generally be > on its own line, missing final period and two spaces. Also some > unnecessary braces for single statements. OK. > There's a bunch of FIXMEs. I'm not really happy about committing a > patch this big with FIXMEs in it; can we try to straighten them out > first? Ah -- there's really only three, but one of them (in a testsuite) got cut-n-pasted numerous times. I'll take a look, some of 'em are probably obsolete. It's hard to remember to take out the FIXME once you've fixed something. ;-) > All the calls to error, and most of the other output calls, should be > _("") wrapped in our current world order. OK, will do. > We know this is going to fall down badly with threads. Can we do > something friendly about that? Maybe you already do, I didn't look too > hard. Ummm... reasonable point, let me think about it. Thanks for the (preliminary) feedback. > "GNU/Linux" please. Check. > Why have we got both "info checkpoints" and "info forks"? Right now > they're aliases to each other and some of the fork commands redirect > you to info checkpoints. Well, because sometimes you'll be using checkpoints, and sometimes you'll be debugging forks. The underlying functionality is identical, but users aren't gonna know or care about that -- from their point of view, they are two entirely different tasks. > How do you feel about calling this "switch-fork"? If I see a debugger > command named "fork", my first reaction is going to be that it's an > active command (creates a fork) instead of a passive one (focuses our > attention on a different one). I see what you mean -- I was just basing it off of the "thread" command by analogy. I'm not attached to it, though, and would be happy to hear what others think. > Getting at the PC this way is probably going to bite you; the two that > jump out at me are you've bypassed ADDR_BITS_REMOVE, and you're forcing > unsigned (which is wrong for MIPS and might result in some strange > looking PCs). No, I don't have a better idea. OK, well... since I have a full-fledged copy of the regcache, there *must* be a legitimate way to do it. If not, there should be, and maybe I'll have to add it. ;-) > You've gotta use cleanups for this sort of thing. e.g. what > if the inferior takes a signal during the call_function_by_hand? Very > easy to arrange; we're coming from a user prompt here, and > call_function_by_hand is a stopped -> running transition. Ah, you mean for save_detach_fork? You're right, I meant to come back to that. Thanks for catching it. > That definitely won't work. I realize it's just a debugging aid, but > PTRACE_PEEKUSER doesn't necessarily get at all registers (on lots of > targets), and doesn't necessarily work at all (the only Linux example I > know of offhand is sparc64). Right, I'll just take it out. It was just for development. > Please include "gdb_string.h" (if you really wanted string.h, it would > be ); and please put it up at the top of the file with all > the other includes. Got it, thanks. ;-)