From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31281 invoked by alias); 22 Feb 2010 20:09:51 -0000 Received: (qmail 31257 invoked by uid 22791); 22 Feb 2010 20:09:49 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 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, 22 Feb 2010 20:09:43 +0000 Received: (qmail 2725 invoked from network); 22 Feb 2010 20:09:42 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 22 Feb 2010 20:09:42 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, Eli Zaretskii Subject: Re: read watchpoints don't work on targets that support read watchpoints Date: Mon, 22 Feb 2010 20:09:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-19-generic; KDE/4.3.2; x86_64; ; ) References: <201002180111.31520.pedro@codesourcery.com> <201002212147.44779.pedro@codesourcery.com> <83r5odm5un.fsf@gnu.org> In-Reply-To: <83r5odm5un.fsf@gnu.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201002222009.39573.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-02/txt/msg00553.txt.bz2 On Monday 22 February 2010 19:24:16, Eli Zaretskii wrote: > > From: Pedro Alves > > + /* If trying to set a read-watchpoint, and it turns out it's not > > + supported, try emulating one with an access watchpoint. */ > > + if (val == 1 && bpt->watchpoint_type == hw_read) > > + { > > + struct bp_location *loc, **loc_temp; > > + > > + /* But don't try to insert it, if there's already another > > + hw_access location that would be considered a duplicate > > + of this one. */ > > What does the last comment above mean for the use-case where I set a > read watchpoint and an access watchpoint at the same address (but with > different conditions)? I probably am missing something, because the > above seems to say this use will be impossible? Note that this is about locations, not high-level user breakpoints. E.g., two high-level (user) access watchpoints watching the same memory, can share the same watchpoint location on the target, even if the condition is different. E.g.: watch global if global == 3 watch global if global == 4 A simple low-level watchpoint is inserted on the target. This code is merely applying the same duplicates detection logic as the last loop of update_global_location_list, but this is reached after update_global_location_list has been called, so since we're changing the location's type (hw_read -> hw_access) very late (at insertion time), we rescan duplicates here. (with target accelarated watchpoint conditions, the duplicates detection will need to be updated to only consider duplicates locations that have the same condition expression, but, that change must be done in all places that do duplicate detection, not just here.) > > + if (bl->watchpoint_type == hw_read) > > + { > > + CORE_ADDR addr; > > + int res; > > + > > + /* A real read watchpoint. Check if we're also > > + trapping the same memory for writes. */ > > + > > + res = target_stopped_data_address (¤t_target, > > + &addr); > > + /* We can only find a read watchpoint triggering > > + if we know the address that trapped. We > > + shouldn't get here otherwise. */ > > + gdb_assert (res); > > + > > + ALL_BREAKPOINTS (b) > > + if (breakpoint_enabled (b) > > + && (b->type == bp_hardware_watchpoint > > + || b->type == bp_access_watchpoint)) > > Why do you need to scan ALL_BREAKPOINTS HERE? Can't you scan only the > list returned by watchpoints_triggered? Hmm, watchpoints_triggered doesn't return a list that would save us iterating over all breakpoints, it does exactly the same loop and check as below (in fact, I just copied it from there, comment and all), and stores the result in b->watchpoint_triggered (as watch_triggered_yes). So, I guess I could indeed iterate over all breakpoints as: if (bl->watchpoint_type == hw_read) { struct breakpoint *other_b; ALL_BREAKPOINTS (other_b) if ((other_b->type == bp_hardware_watchpoint || other_b->type == bp_access_watchpoint) && (other_b->watchpoint_triggered == watch_triggered_yes)) { other_write_watchpoint = 1; break; } } Should be a bit more efficient, and simpler, yes. Thanks. I've done this change, and checked the new test still passes OK on my hacked PPC gdb. > > > + /* Exact match not required. Within range > > + is sufficient. */ > > + for (loc = b->loc; loc; loc = loc->next) > > + if (target_watchpoint_addr_within_range > > + (¤t_target, > > + addr, > > + loc->address, > > + loc->length)) > > And why are we checking watchpoints that couldn't have triggered, > because they watch a different, albeit close, address? What would be > the use-case where such a watchpoint could be relevant to deciding > which watchpoint to announce? I'm not sure I understand this question. Does the confusion arise from the comment quoted above? This is the exact same check watchpoints_triggered does. ADDR is checked for being in the [loc->address, loc->address+loc->length) range. If it is, then watchpoint B is watching ADDR. Does this answer your questions? -- Pedro Alves