From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7638 invoked by alias); 30 Oct 2008 03:08:13 -0000 Received: (qmail 7482 invoked by uid 22791); 30 Oct 2008 03:08:12 -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; Thu, 30 Oct 2008 03:08:09 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id A2D5D2A96A3; Wed, 29 Oct 2008 23:08:07 -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 58hMzDWgwB5g; Wed, 29 Oct 2008 23:08:07 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 55DCB2A96A1; Wed, 29 Oct 2008 23:08:07 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 2D197E7ACD; Wed, 29 Oct 2008 20:08:05 -0700 (PDT) Date: Thu, 30 Oct 2008 03:34:00 -0000 From: Joel Brobecker To: David Daney Cc: gdb-patches@sourceware.org Subject: Re: [Patch] Use resume instead of target_resume when stepping over watchpoint. Message-ID: <20081030030805.GC3635@adacore.com> References: <48C71565.3050601@avtrex.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <48C71565.3050601@avtrex.com> 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/msg00697.txt.bz2 Pedro, Others, What do you think of this patch? Personally, I have pretty much convinced myself that it shouldn't do any harm, but I really wished that "resume" would take a ptid as an argument. Except that this is not trivial to do, and I think that the current "resume" would need to be split a bit, to remove the code that determines what to resume. Anyway, I don't see anything wrong with this patch, but I'd love for someone to take a look as well. This is a pretty delicate part of the debugger. Do we really need the gdb_assert thought? On Tue, Sep 09, 2008 at 05:31:33PM -0700, David Daney wrote: > In handle_inferior_event() when stepping over a watch point currently we > issue target_resume(). This only works on architectures that have > hardware single step support. For gdbarch_software_single_step_p() > systems (like MIPS), we need to insert a single step breakpoint instead. > > The fix is to call resume() as it does the right thing already. I also > added an assert that inferior_ptid == ecs->ptid to be sure that resume() > was stepping the proper thread. > > This is essentially the change requested by Daniel in: > http://sourceware.org/ml/gdb-patches/2008-04/msg00443.html > > This change is a prerequisite for my forthcoming MIPS hardware watch patch. > > Tested on x86_64-pc-linux-gnu as well as mipsel-linux (in conjunction > with the MIPS hardware watch patch). > > OK to commit? > > 2008-09-09 David Daney > > * infrun.c (handle_inferior_event): Call resume instead of > target_resume when stepping over watchpoint. > > > Index: infrun.c > =================================================================== > RCS file: /cvs/src/src/gdb/infrun.c,v > retrieving revision 1.316 > diff -u -p -r1.316 infrun.c > --- infrun.c 8 Sep 2008 22:10:20 -0000 1.316 > +++ infrun.c 9 Sep 2008 23:37:09 -0000 > @@ -2472,7 +2472,8 @@ targets should add new threads to the th > if (!HAVE_STEPPABLE_WATCHPOINT) > remove_breakpoints (); > registers_changed (); > - target_resume (ecs->ptid, 1, TARGET_SIGNAL_0); /* Single step */ > + gdb_assert (ptid_equal (inferior_ptid, ecs->ptid)); > + resume (1, TARGET_SIGNAL_0); /* Single step */ > waiton_ptid = ecs->ptid; > if (HAVE_STEPPABLE_WATCHPOINT) > infwait_state = infwait_step_watch_state; -- Joel