From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6556 invoked by alias); 12 Sep 2011 15:03:56 -0000 Received: (qmail 6547 invoked by uid 22791); 12 Sep 2011 15:03:55 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate1.uk.ibm.com (HELO mtagate1.uk.ibm.com) (194.196.100.161) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 12 Sep 2011 15:03:26 +0000 Received: from d06nrmr1806.portsmouth.uk.ibm.com (d06nrmr1806.portsmouth.uk.ibm.com [9.149.39.193]) by mtagate1.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p8CF3Lgt005343 for ; Mon, 12 Sep 2011 15:03:22 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1806.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p8CF3Kwd2195522 for ; Mon, 12 Sep 2011 16:03:21 +0100 Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p8CF3Kfx003527 for ; Mon, 12 Sep 2011 09:03:20 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p8CF3JCD003396; Mon, 12 Sep 2011 09:03:19 -0600 Message-Id: <201109121503.p8CF3JCD003396@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Mon, 12 Sep 2011 17:03:18 +0200 Subject: Watchpoint resource accounting broken (Re: [5/6] breakpoints_ops for all kinds of breakpoints: new watchpoints instance type) To: pedro@codesourcery.com (Pedro Alves) Date: Mon, 12 Sep 2011 15:06:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <201107221645.06432.pedro@codesourcery.com> from "Pedro Alves" at Jul 22, 2011 04:45:06 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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-09/txt/msg00192.txt.bz2 Pedro Alves wrote: > (watch_command_1): Allocate and initialize a struct watchpoint > instead of a struct breakpoint. Use install_breakpoint. > /* Now set up the breakpoint. */ > + > + w = XCNEW (struct watchpoint); > + b = &w->base; > if (use_mask) > - b = set_raw_breakpoint_without_location (NULL, bp_type, > - &masked_watchpoint_breakpoint_ops); > + init_raw_breakpoint_without_location (b, NULL, bp_type, > + &masked_watchpoint_breakpoint_ops); > else > - b = set_raw_breakpoint_without_location (NULL, bp_type, > - &watchpoint_breakpoint_ops); > + init_raw_breakpoint_without_location (b, NULL, bp_type, > + &watchpoint_breakpoint_ops); [snip] > @@ -9153,7 +9247,7 @@ watch_command_1 (char *arg, int accessfl > { > /* Finally update the new watchpoint. This creates the locations > that should be inserted. */ > - update_watchpoint (b, 1); > + update_watchpoint (w, 1); > } > if (e.reason < 0) > { > @@ -9161,15 +9255,7 @@ watch_command_1 (char *arg, int accessfl > throw_exception (e); > } > > - set_breakpoint_number (internal, b); > - > - /* Do not mention breakpoints with a negative number, but do > - notify observers. */ > - if (!internal) > - mention (b); > - observer_notify_breakpoint_created (b); > - > - update_global_location_list (1); > + install_breakpoint (internal, b); > } Unfortunately this change breaks watchpoint resource accounting. Note that in the old version, the watchpoint had already been added to the breakpoint list (by set_raw_breakpoint_without_location) *before* the call to update_watchpoint, while in the new version the watchpoint is added *after* that call (by install_breakpoint). However, update_watchpoint relies on having the watchpoint under investigation be on the breakpoint list; see the comment: /* We need to determine how many resources are already used for all other hardware watchpoints plus this one to see if we still have enough resources to also fit this watchpoint in as well. To guarantee the hw_watchpoint_used_count call below counts this watchpoint, make sure that it is marked as a hardware watchpoint. */ if (b->base.type == bp_watchpoint) b->base.type = bp_hardware_watchpoint; i = hw_watchpoint_used_count (b->base.type, &other_type_used); target_resources_ok = target_can_use_hardware_watchpoint (b->base.type, i, other_type_used); Note how just "i", the result of hw_watchpoint_used_count, is passed to target_can_use_hardware_watchpoint -- this works only if the current watchpoint is on the list that hw_watchpoint_used_count iterates over. (You cannot just consider the current watchpoint in addition to the result of hw_watchpoint_used_count either, because for other callers to update_watchpoint, the current watchpoint *is* on the list.) I'm sure sure how best to fix this; maybe go back to adding the watchpoint to the breakpoint list earlier? Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com