From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2156 invoked by alias); 7 Sep 2003 03:53:31 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 2141 invoked from network); 7 Sep 2003 03:53:27 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 7 Sep 2003 03:53:27 -0000 Received: from drow by nevyn.them.org with local (Exim 4.22 #1 (Debian)) id 19vqc9-0002bu-6H for ; Sat, 06 Sep 2003 23:53:25 -0400 Date: Sun, 07 Sep 2003 03:53:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sources.redhat.com Subject: Re: RFC: selected frame in read_var_value Message-ID: <20030907035325.GA9985@nevyn.them.org> Mail-Followup-To: gdb-patches@sources.redhat.com References: <20030801192951.GA2109@nevyn.them.org> <3F329038.5040708@redhat.com> <20030828195501.GB27550@nevyn.them.org> <3F58C35A.3060908@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3F58C35A.3060908@redhat.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-09/txt/msg00093.txt.bz2 On Fri, Sep 05, 2003 at 01:09:46PM -0400, Andrew Cagney wrote: > >On Thu, Aug 07, 2003 at 01:45:28PM -0400, Andrew Cagney wrote: > > > >>>Thoughts on this? I observed one case where the selected frame hadn't > >>been > >>>initialized yet: when checking watchpoint values. Oops! The condition > >>>matches the case in which get_current_frame () can be reasonably > >>expected >to > >>>succeed. There are a couple other places that need this same check, > >>mostly > >>>functions which will work on globals as well as locals. > > > >> > >>I think the caller, and not this code, should be the one deciding > >>if/when a get_selected_frame should be called. Otherwize the dependance > >>on global state remains. > > > > > >The callers don't know any more than we do. Take a look at some of > >them - several callers don't take frame arguments, and work both when > >the program is running and when it isn't. If you want to cut down on > >the global state dependence, I guess you'd have to push this up through > >every caller, so that it could eventually be pushed even higher? > > Yep, welcome to my world :-) > > >Me, I was just fixing the regression caused by lazily selecting a > >frame. I wasn't planning on redesigning interfaces all through the > >block and variable layers; and fixing this really does go quite far up. > >For instance it works back to block_innermost_frame, which can return > >NULL. > > What about something like this as a step sideways: > > >if (frame == NULL) > > frame = deprecated_safe_get_selected_frame (); > > where deprecated_safe_get_selected_frame() would contain those sanity > checks? > > Andrew Is this about what you wanted, and are my comments on the mark? If so, how do you feel about a mass replacement of the one deprecated construct (deprecated_selected_frame) with the new deprecated construct (deprecated_safe_get_selected_frame) in the places in GDB which use this "if frame arg is NULL, get selected frame" idiom? -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 2003-09-06 Daniel Jacobowitz * frame.c (deprecated_safe_get_selected_frame): New function. * frame.h (deprecated_safe_get_selected_frame): Add prototype. * findvar.c (read_var_value): Call it. Index: findvar.c =================================================================== RCS file: /cvs/src/src/gdb/findvar.c,v retrieving revision 1.62 diff -u -p -r1.62 findvar.c --- findvar.c 22 Jul 2003 15:41:59 -0000 1.62 +++ findvar.c 7 Sep 2003 03:46:36 -0000 @@ -404,8 +404,11 @@ read_var_value (register struct symbol * len = TYPE_LENGTH (type); + + /* FIXME drow/2003-09-06: this call to the selected frame should be + pushed upwards to the callers. */ if (frame == NULL) - frame = deprecated_selected_frame; + frame = deprecated_safe_get_selected_frame (); switch (SYMBOL_CLASS (var)) { Index: frame.c =================================================================== RCS file: /cvs/src/src/gdb/frame.c,v retrieving revision 1.137 diff -u -p -r1.137 frame.c --- frame.c 12 Aug 2003 17:45:13 -0000 1.137 +++ frame.c 7 Sep 2003 03:46:36 -0000 @@ -917,6 +917,18 @@ get_selected_frame (void) return deprecated_selected_frame; } +/* This is a variant of get_selected_frame which can be called when the + inferior is not running; in that case it will return NULL instead of + calling error (). */ + +struct frame_info * +deprecated_safe_get_selected_frame (void) +{ + if (!target_has_registers || !target_has_stack || !target_has_memory) + return NULL; + return get_selected_frame (); +} + /* Select frame FI (or NULL - to invalidate the current frame). */ void Index: frame.h =================================================================== RCS file: /cvs/src/src/gdb/frame.h,v retrieving revision 1.106 diff -u -p -r1.106 frame.h --- frame.h 11 Jul 2003 15:31:43 -0000 1.106 +++ frame.h 7 Sep 2003 03:46:36 -0000 @@ -639,6 +639,19 @@ extern void return_command (char *, int) extern struct frame_info *deprecated_selected_frame; +/* NOTE: drow/2003-09-06: + + This function is "a step sideways" for uses of deprecated_selected_frame. + They should be fixed as above, but meanwhile, we needed a solution for + cases where functions are called with a NULL frame meaning either "the + program is not running" or "use the selected frame". Lazy building of + deprecated_selected_frame confuses the situation, because now + deprecated_selected_frame can be NULL even when the inferior is running. + + This function calls get_selected_frame if the inferior is running, or + returns NULL otherwise. */ + +extern struct frame_info *deprecated_safe_get_selected_frame (void); /* Create a frame using the specified BASE and PC. */