From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28477 invoked by alias); 4 May 2011 19:12:54 -0000 Received: (qmail 28468 invoked by uid 22791); 4 May 2011 19:12:52 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL,BAYES_00,SPF_SOFTFAIL,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e3.ny.us.ibm.com (HELO e3.ny.us.ibm.com) (32.97.182.143) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 04 May 2011 19:12:38 +0000 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by e3.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p44Ip3HB032452 for ; Wed, 4 May 2011 14:51:03 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p44JCYmN725178 for ; Wed, 4 May 2011 15:12:34 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p44JCW1n031012 for ; Wed, 4 May 2011 16:12:33 -0300 Received: from [9.12.228.54] ([9.12.228.54]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p44JCUS9030604; Wed, 4 May 2011 16:12:30 -0300 Subject: Re: [RFA 2/3] Demote to sw watchpoint only in update_watchpoint From: Thiago Jung Bauermann To: Eli Zaretskii Cc: uweigand@de.ibm.coma, gdb-patches@sourceware.org In-Reply-To: References: <201104291726.p3THQVaC029608@d06av02.portsmouth.uk.ibm.com> <1304398546.2245.80.camel@hactar> Content-Type: text/plain; charset="UTF-8" Date: Wed, 04 May 2011 19:12:00 -0000 Message-ID: <1304536344.19357.218.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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: 2011-05/txt/msg00108.txt.bz2 Hi Eli, On Tue, 2011-05-03 at 02:05 -0400, Eli Zaretskii wrote: > It was originally designed and implemented for a single platform (which > we no longer support, btw), and doesn't scale well, or not at all, to > other platforms, not even to x86. > > The problem with the meaning of the other_type_used argument is the > telltale sign: it only made sense for that single platform for which > hardware watchpoints were originally implemented in GDB. It no longer > makes any sense with today's platforms, and actually cannot be used at > all with most of them (at least those I'm familiar with), for a very > simple reason: a single `int' parameter doesn't provide a way to > report enough information about the consumption of related resources. > It doesn't matter if that parameter is a boolean or not; it simply > isn't up to this job. Much more information is needed about the > related resources to decide whether another watchpoint can or cannot > be set. This was hard for me to evaluate as I didn't find anywhere what other_type_used was ever meant to be. Not even in the patch submission that introduced it. Thanks for the insight. > The fundamental design problem here is this: there's no way GDB > application level can know whether the target can accommodate an > additional hardware watchpoint of a given type and attributes. Only > the target itself can tell that accurately, because the information > needed to answer that question is extremely target-dependent. OTOH, > it is bad UI design to let the user set a hardware watchpoint, then > demote it to a software watchpoint (or even refuse to insert a > watchpoint at all) at "resume" time. So on the one hand, we _want_ > the GDB application level to know if a hardware watchpoint is > possible, to tell the user it cannot, and OTOH there's a problem > knowing this at that level. GDB doesn't have a command to create a hardware watchpoint or a software watchpoint. It will decide which will be created based on what it thinks can be done. Do you think this should be changed and there should be an hwatch command which would fail if an hw watchpoint is not possible? Or just that a watchpoint should never be converted between sw and hw after they are created? > > But that's out of the scope of this patch series. > > I say, let's stop postponing this to some other patch series. I've > been to that movie enough to know that it's a polite form of saying > "never". In the particular case of this patch, I think this reasoning is misplaced. It may not have been clear in my previous message, but what I was saying was that I tried to attack the problem in a minimally intrusive way (i.e., a kludge :-) ) but gave up and left the code basically untouched. I said that fixing the problem was out of the scope of this patch series meaning "I was developing the feature and I noticed this problem. I tried to solve it while I was at it but things got involved". So I don't think it's fair to say "You tried to address that problem and even mentioned in your patch submission, now you have to fix it!" This is GDB's behaviour without this patch: (gdb) awatch a Hardware access (read/write) watchpoint 6: a (gdb) rwatch c Hardware read watchpoint 7: c (gdb) watch d Hardware watchpoint 8: d (gdb) i b Num Type Disp Enb Address What 6 acc watchpoint keep y a 7 read watchpoint keep y c 8 hw watchpoint keep y d (gdb) c Continuing. Unexpected error setting breakpoint or watchpoint: No space left on device. (gdb) and this is the behaviour with the patch: (gdb) awatch a Hardware access (read/write) watchpoint 2: a (gdb) rwatch c Hardware read watchpoint 3: c (gdb) watch d Hardware watchpoint 4: d (gdb) i b Num Type Disp Enb Address What 2 acc watchpoint keep y a 3 read watchpoint keep y c 4 hw watchpoint keep y d (gdb) c Continuing. Warning: Could not insert hardware watchpoint 3. Could not insert hardware watchpoint 4. Could not insert hardware breakpoints: You may have requested too many hardware breakpoints/watchpoints. (gdb) So the problem is still there. It's not better, but not worse either. The output is slightly different because of the better error handling this patch introduces. Thus, IMHO overhauling resources handling is not a prerequisite or even directly related to this patch (or the next one in the series). The acceptance or rejection of this patch is orthogonal to that. Having said that, I agree that it's a shame that GDB can't keep track of debug hardware availability, something that one would expect to be among the basic tasks of a debugger. So if we can agree on how GDB should deal with the problem, I'm willing to work on it on a best effort basis (meaning that I wouldn't have much time to dedicate to this, but would eventually make progress). > Sorry for being so blunt, but I think this problem is well past the > time we should have solved it for good. No need to apologize. I understand your frustration, especially since I read the discussion you and Cagney had in 2002 about this very topic, and nothing actually changed since then... -- []'s Thiago Jung Bauermann IBM Linux Technology Center