From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21101 invoked by alias); 3 Oct 2008 06:07:10 -0000 Received: (qmail 20797 invoked by uid 22791); 3 Oct 2008 06:07:09 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 03 Oct 2008 06:06:34 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 5E29E2A96C8; Fri, 3 Oct 2008 02:06:32 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id onHal6c3sQvc; Fri, 3 Oct 2008 02:06:32 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id F32EC2A9655; Fri, 3 Oct 2008 02:06:31 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id E0B4BE7ACD; Thu, 2 Oct 2008 23:06:29 -0700 (PDT) Date: Fri, 03 Oct 2008 06:07:00 -0000 From: Joel Brobecker To: =?iso-8859-1?Q?S=E9rgio_Durigan_J=FAnior?= Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part Message-ID: <20081003060629.GQ3665@adacore.com> References: <1222798409.30389.23.camel@miki> <20081002211256.GO3665@adacore.com> <1223001252.9858.11.camel@miki> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1223001252.9858.11.camel@miki> User-Agent: Mutt/1.4.2.2i 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: 2008-10/txt/msg00079.txt.bz2 Hi Sergio, > If I understood correctly, you think it's better to create a "generic" > bptype so that not only "catch syscall" can use it, right? That's correct. The short answer is that, if we make your catchpoint use a more generic type and base the actual implementation of breakpoint_ops methods, then, later on, when someone decides to implement a new kind of catchpoint with similar functionality, then all he should have to do is create a new breakpoint_ops vector with appropriate methods, and voila. This is pretty much what happened when I implemented catchpoints on Ada exception. If you had a look at my patch, you'll notice that I didn't have to sprinkle "case bp_my_new_kind" everywhere to make it work. To me, this makes me so much more confident that I didn't miss an update somewhere. But my situation was simpler than yours, however, because my catchpoints were barely more than just an elaborated breakpoint. The longer answer is that I have been wondering about making the breakpoint structure a little more object-oriented. I have this Vision (no mushrooms involved so far) where there are really two types of catchpoints: (a): Those that are implemented using one or more breakpoints (b): Those that are implemented without the use of breakpoins Either way, I would like to see these breakpoints as objects, with methods that we would call: (1) insert: Insert the physical breakpoint, or activate the associated trace, or... (2) remove: The opposite of insert (3) is_mind: Return true if an event is recognized by the catchpoint (3) print_stop_action, print_one, print_mention. (the methods already provided by the breakpoint_ops) (4) any other method? So, when the debugger needs to insert breakpoints, he doesn't have to know how syscall catchpoints are implemented. It just calls the associated "insert" method. Same for removing. When we receive an event, we could rely on the "is_mine" method to determine that we stopped due to this catchpoint, and thus call the associated methods to tell the user about them. Etc. The thing is, I haven't designed this part of the code, and I don't know if the breakpoint_ops were meant to be used such that we could make the core code completely abstract of which breakpoint was hit. Nor am I certain that it is actually possible. But it's definitely something that I would like to try. Now, going back to your patch, I think it's important not to let best be the enemy of good either. I think that the feature that you are proposing is interesting and would like to make sure that we don't lose your work. I am about to go away on a trip, so will have little time to review the details of your patch, but looks like Michael has started on that. I would love for you to investigate my (ahem) Vision (double-ahem), and I'm ready to guide you as much as I can. But I am equally happy to incorporate your approach if another maintainer looks at your code and OKays it. > The way I see, there's no problem to implement "catch syscall" using > another way. However, and I think you already know that, the "catch > fork", "catch vfork" and "catch exec" are implemented using an entry on > "enum bptype" (and that's specific why I've chosen to do this). Do you > think we should change this, too? Not as part of your patch. But in the long term, I would like to migrate them to the breakpoint_ops approach if possible. That's if we have determined that it's doable, that is. > I'm not sure if I understood this part correctly. Maybe my GDB-fu isn't > good enought yet :-). I'll take a look at breakpoint_ops to see if I can > understand better what you want. Hopefully the explainations above might have clarified a little bit the context. Let me know if there are parts that you'd like me to expand more. -- Joel