From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10616 invoked by alias); 6 Oct 2006 00:50:17 -0000 Received: (qmail 10607 invoked by uid 22791); 6 Oct 2006 00:50:16 -0000 X-Spam-Check-By: sourceware.org Received: from nile.gnat.com (HELO nile.gnat.com) (205.232.38.5) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 06 Oct 2006 00:50:13 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-nile.gnat.com (Postfix) with ESMTP id 204C248CF7C for ; Thu, 5 Oct 2006 20:50:11 -0400 (EDT) Received: from nile.gnat.com ([127.0.0.1]) by localhost (nile.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 20551-01-4 for ; Thu, 5 Oct 2006 20:50:10 -0400 (EDT) Received: from takamaka.act-europe.fr (unknown [70.71.0.212]) by nile.gnat.com (Postfix) with ESMTP id 6398F48CE2F for ; Thu, 5 Oct 2006 20:50:10 -0400 (EDT) Received: by takamaka.act-europe.fr (Postfix, from userid 507) id 6DC4447F00; Thu, 5 Oct 2006 17:50:09 -0700 (PDT) Date: Fri, 06 Oct 2006 00:50:00 -0000 From: Joel Brobecker To: gdb-patches@sources.redhat.com Subject: [RFA] Fix frame-issue with watchpoints... Message-ID: <20061006005009.GA986@adacore.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="NzB8fVQJ5HfG6fxh" Content-Disposition: inline User-Agent: Mutt/1.4i Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-10/txt/msg00044.txt.bz2 --NzB8fVQJ5HfG6fxh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 4178 Hello, A coworker of mine noticed an issue with watchpoints. To reproduce, he provided the following Ada program (on x86-linux for instance): 1 procedure Watch is 2 3 procedure Foo (X : access Integer) is 4 begin 5 delay 1.0; 6 end Foo; 7 8 X : aliased Integer := 1; 9 10 begin 11 Foo (X'Access); 12 X := 2; -- BREAK 13 end Watch; This program needs to be compiled using the usual trivial command: % gnatmake -g watch This will produce a small program that can be debugged as follow: % gdb watch (gdb) b foo.adb:5 (gdb) b foo.adb:12 (gdb) run At this point, the debugger will stop inside procedure Foo, where we insert a watchpoint on X, before continuing to the next breakpoint on line 12: (gdb) watch x Hardware watchpoint 3: x (gdb) cont Continuing. No frame is currently executing in block watch.foo. Here is what happens: The first thing we do is notice that we just stopped at a location where we have a breakpoint, so we do a single-step, which worked out just fine, and then do the real continue. Just before resuming the execution of the inferior, we try to re-insert the breakpoints, and that means we re-evaluate all breakpoint locations, and this is where things start to break... The watchpoint location evaluation eventually leads us to value_of_variable, and in particular the error message we see: else if (symbol_read_needs_frame (var)) { frame = block_innermost_frame (b); if (!frame) { if (BLOCK_FUNCTION (b) && SYMBOL_PRINT_NAME (BLOCK_FUNCTION (b))) error (_("No frame is currently executing in block %s."), What happens is that block_innermost_frame(b) returned NULL, and the reason is fairly simple: frame = NULL; while (1) { frame = get_prev_frame (frame); There is a note from Andrew besides the part of get_prev_frame that says: if (this_frame == NULL) { /* NOTE: cagney/2002-11-09: There was a code segment here that would error out when CURRENT_FRAME was NULL. The comment that went with it made the claim ... ``This screws value_of_variable, which just wants a nice clean NULL return from block_innermost_frame if there are no frames. I don't think I've ever seen this message happen otherwise. And returning NULL here is a perfectly legitimate thing to do.'' Per the above, this code shouldn't even be called with a NULL THIS_FRAME. */ frame_debug_got_null_frame (gdb_stdlog, this_frame, "this_frame NULL"); return current_frame; } The issue in our case is that "current_frame" is NULL too, probably because we never needed it before in our case (just finished off single-stepping out of the breakpoint and immediately getting ready to resume) and therefore never set it to a proper value. One way to bandaid this, probably along the lines that Andrew was trying to do (try to recover from a situation that should not happen), is to replace "current_frame" by get_current_frame(). However, I'm thinking that the real culprit is block_innermost_frame which called this function with a NULL frame, which it's not supposed to do. So I rewrote the loop to start from the outermost frame instead of a NULL frame, and then work its way up the frame stack... This fixes the problem without introducing any regression. In addition to this patch, I think we should investigate the option of removing the check above and replace it with an assertion that THIS_FRAME is not null. Or maybe if we are afraid of breaking something, then at least change "current_frame" along the lines of what I suggested, and print an unconditional warning. Let's make it visible. 2006-10-05 Joel Brobecker * blockframe.c (block_innermost_frame): Rewrite frame search logic. Tested on x86-linux, no regression. A new testcase to be submitted soon. OK to apply? Thanks, -- Joel --NzB8fVQJ5HfG6fxh Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="blockframe.c.diff" Content-length: 774 Index: blockframe.c =================================================================== RCS file: /cvs/src/src/gdb/blockframe.c,v retrieving revision 1.110 diff -u -p -r1.110 blockframe.c --- blockframe.c 19 Jul 2006 02:17:23 -0000 1.110 +++ blockframe.c 6 Oct 2006 00:47:59 -0000 @@ -358,14 +358,15 @@ block_innermost_frame (struct block *blo start = BLOCK_START (block); end = BLOCK_END (block); - frame = NULL; - while (1) + frame = get_current_frame (); + while (frame != NULL) { - frame = get_prev_frame (frame); - if (frame == NULL) - return NULL; calling_pc = get_frame_address_in_block (frame); if (calling_pc >= start && calling_pc < end) return frame; + + frame = get_prev_frame (frame); } + + return NULL; } --NzB8fVQJ5HfG6fxh--