From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20536 invoked by alias); 31 Jan 2007 19:07:47 -0000 Received: (qmail 20527 invoked by uid 22791); 31 Jan 2007 19:07:45 -0000 X-Spam-Check-By: sourceware.org Received: from lon-del-04.spheriq.net (HELO lon-del-04.spheriq.net) (195.46.50.101) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 31 Jan 2007 19:07:37 +0000 Received: from lon-out-02.spheriq.net ([195.46.50.130]) by lon-del-04.spheriq.net with ESMTP id l0VJ7Wdd025664 for ; Wed, 31 Jan 2007 19:07:32 GMT Received: from lon-cus-01.spheriq.net (lon-cus-01.spheriq.net [195.46.50.37]) by lon-out-02.spheriq.net with ESMTP id l0VJ7UY9002222 for ; Wed, 31 Jan 2007 19:07:32 GMT Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by lon-cus-01.spheriq.net with ESMTP id l0VJ7Uw1027835 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=OK); Wed, 31 Jan 2007 19:07:30 GMT Received: from zeta.dmz-eu.st.com (ns2.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 4888110522; Wed, 31 Jan 2007 15:05:40 +0000 (GMT) Received: from mail1.cro.st.com (mail1.cro.st.com [164.129.40.131]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id DCC7C4740A; Wed, 31 Jan 2007 15:05:39 +0000 (GMT) Received: from [164.129.44.95] (crx595.cro.st.com [164.129.44.95]) by mail1.cro.st.com (MOS 3.7.5a-GA) with ESMTP id CJP70347 (AUTH "denis pilat"); Wed, 31 Jan 2007 16:05:38 +0100 (CET) Message-ID: <45C0B042.9040308@st.com> Date: Wed, 31 Jan 2007 19:07:00 -0000 From: Denis PILAT User-Agent: Thunderbird 1.5.0.9 (X11/20061206) MIME-Version: 1.0 To: Nick Roberts , gdb-patches Subject: Re: [RFC] varobj deletion after the binary has changed References: <45B60056.6030704@st.com> <20070123124457.GA1600@nevyn.them.org> <45B63A49.4010609@st.com> <45B8E8A8.9040904@st.com> <17849.12231.246980.478169@kahikatea.snap.net.nz> <20070125232731.GA30178@nevyn.them.org> <45BDEAEC.1050006@st.com> <17854.28971.170898.231523@kahikatea.snap.net.nz> In-Reply-To: <17854.28971.170898.231523@kahikatea.snap.net.nz> Content-Type: multipart/mixed; boundary="------------010206020205060903070900" 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: 2007-01/txt/msg00620.txt.bz2 This is a multi-part message in MIME format. --------------010206020205060903070900 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 599 Nick Roberts wrote: > > Looks good. Maybe varobj_update could use an enum: > > enum varobj_update_values > { > SCOPE_FALSE = -1, > TYPE_CHANGED, > SCOPE_INVALID > } > Yes, it's better and more readable. As you'll see in my patch, the varobj_update function could return more than these case of error, it's the caller that decides how to deal with these errors. > > (It looks like we could remove the return value of varobj_update_one as it > doesn't seem to be used.) > You're right, I removed it as well. Attached is the new implementation plus the new exp file. Denis --------------010206020205060903070900 Content-Type: text/plain; name="varobj.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="varobj.patch" Content-length: 10307 2007-01-31 Denis Pilat * varobj.c (struct varobj_root): Add is_valid member. (varobj_get_type): Check for invalid varobj. (varobj_get_attributes): Likewise. (variable_editable):Likewise. (varobj_update): Likewise plus use an enum for returned error values. (new_root_variable): Set root varobj as valid by default. (varobj_invalidate): New function. * varobj.h (enum varobj_update_error): New enum. (varobj_invalidate): New function. * symfile.c (clear_symtab_users): Use varobj_invalidate. * mi/mi-cmd-var.c (varobj_update_one): Change returned type to void and use of new enum varobj_update_error to deal with errors. Index: varobj.c =================================================================== --- varobj.c (revision 553) +++ varobj.c (working copy) @@ -71,6 +71,10 @@ struct varobj_root using the currently selected frame. */ int use_selected_frame; + /* Flag that indicates validity: set to 0 when this varobj_root refers + to symbols that do not exist anymore. */ + int is_valid; + /* Language info for this variable and its children */ struct language_specific *lang; @@ -742,8 +746,9 @@ varobj_get_type (struct varobj *var) long length; /* For the "fake" variables, do not return a type. (It's type is - NULL, too.) */ - if (CPLUS_FAKE_CHILD (var)) + NULL, too.) + Do not return a type for invalid variables as well. */ + if (CPLUS_FAKE_CHILD (var) || !var->root->is_valid) return NULL; stb = mem_fileopen (); @@ -778,7 +783,7 @@ varobj_get_attributes (struct varobj *va { int attributes = 0; - if (variable_editable (var)) + if (var->root->is_valid && variable_editable (var)) /* FIXME: define masks for attributes */ attributes |= 0x00000001; /* Editable */ @@ -1018,16 +1023,15 @@ install_new_value (struct varobj *var, s expression to see if it's changed. Then go all the way through its children, reconstructing them and noting if they've changed. - Return value: - -1 if there was an error updating the varobj - -2 if the type changed - Otherwise it is the number of children + parent changed + Return value: + < 0 for error values, see varobj.h. + Otherwise it is the number of children + parent changed. Only root variables can be updated... NOTE: This function may delete the caller's varobj. If it - returns -2, then it has done this and VARP will be modified - to point to the new varobj. */ + returns TYPE_CHANGED, then it has done this and VARP will be modified + to point to the new varobj. */ int varobj_update (struct varobj **varp, struct varobj ***changelist) @@ -1048,12 +1052,15 @@ varobj_update (struct varobj **varp, str /* sanity check: have we been passed a pointer? */ if (changelist == NULL) - return -1; + return WRONG_PARAM; /* Only root variables can be updated... */ if (!is_root_p (*varp)) /* Not a root var */ - return -1; + return WRONG_PARAM; + + if (!(*varp)->root->is_valid) + return INVALID; /* Save the selected stack frame, since we will need to change it in order to evaluate expressions. */ @@ -1090,7 +1097,7 @@ varobj_update (struct varobj **varp, str /* This means the varobj itself is out of scope. Report it. */ VEC_free (varobj_p, result); - return -1; + return NOT_IN_SCOPE; } VEC_safe_push (varobj_p, stack, *varp); @@ -1140,7 +1147,7 @@ varobj_update (struct varobj **varp, str *cv = 0; if (type_changed) - return -2; + return TYPE_CHANGED; else return changed; } @@ -1409,6 +1416,7 @@ new_root_variable (void) var->root->frame = null_frame_id; var->root->use_selected_frame = 0; var->root->rootvar = NULL; + var->root->is_valid = 1; return var; } @@ -1692,7 +1700,10 @@ variable_editable (struct varobj *var) static char * my_value_of_variable (struct varobj *var) { - return (*var->root->lang->value_of_variable) (var); + if (var->root->is_valid) + return (*var->root->lang->value_of_variable) (var); + else + return NULL; } static char * @@ -2504,3 +2515,44 @@ When non-zero, varobj debugging is enabl show_varobjdebug, &setlist, &showlist); } + +/* Invalidate the varobjs that are tied to locals and re-create the ones that + are defined on globals. + Invalidated varobjs will be always printed in_scope="false". */ +void +varobj_invalidate (void) +{ + struct varobj **all_rootvarobj; + struct varobj **varp; + + if (varobj_list (&all_rootvarobj) > 0) + { + varp = all_rootvarobj; + while (*varp != NULL) + { + /* global var must be re-evaluated. */ + if ((*varp)->root->valid_block == NULL) + { + struct varobj *tmp_var; + + /* Try to create a varobj with same expression. If we succeed replace + the old varobj, otherwise invalidate it. */ + tmp_var = varobj_create (NULL, (*varp)->name, (CORE_ADDR) 0, USE_CURRENT_FRAME); + if (tmp_var != NULL) + { + tmp_var->obj_name = xstrdup ((*varp)->obj_name); + varobj_delete (*varp, NULL, 0); + install_variable (tmp_var); + } + else + (*varp)->root->is_valid = 0; + } + else /* locals must be invalidated. */ + (*varp)->root->is_valid = 0; + + varp++; + } + xfree (all_rootvarobj); + } + return; +} Index: varobj.h =================================================================== --- varobj.h (revision 553) +++ varobj.h (working copy) @@ -38,7 +38,16 @@ enum varobj_type USE_CURRENT_FRAME, /* Use the current frame */ USE_SELECTED_FRAME /* Always reevaluate in selected frame */ }; - + +/* Error return values for varobj_update function. */ +enum varobj_update_error + { + NOT_IN_SCOPE = -1, /* varobj not in scope, can not be updated. */ + TYPE_CHANGED = -2, /* varobj type has changed. */ + INVALID = -3, /* varobj is not valid anymore. */ + WRONG_PARAM = -4 /* function is called with wrong arguments. */ + }; + /* String representations of gdb's format codes (defined in varobj.c) */ extern char *varobj_format_string[]; @@ -99,4 +108,6 @@ extern int varobj_list (struct varobj ** extern int varobj_update (struct varobj **varp, struct varobj ***changelist); +extern void varobj_invalidate (void); + #endif /* VAROBJ_H */ Index: symfile.c =================================================================== --- symfile.c (revision 553) +++ symfile.c (working copy) @@ -52,6 +52,7 @@ #include "observer.h" #include "exec.h" #include "parser-defs.h" +#include "varobj.h" #include #include @@ -2602,6 +2603,10 @@ clear_symtab_users (void) between expressions and which ought to be reset each time. */ expression_context_block = NULL; innermost_block = NULL; + + /* Varobj may refer to old symbols, perform a cleanup. */ + varobj_invalidate (); + } static void Index: mi/mi-cmd-var.c =================================================================== --- mi/mi-cmd-var.c (revision 553) +++ mi/mi-cmd-var.c (working copy) @@ -34,9 +34,9 @@ const char mi_no_values[] = "--no-values const char mi_simple_values[] = "--simple-values"; const char mi_all_values[] = "--all-values"; -extern int varobjdebug; /* defined in varobj.c */ +extern int varobjdebug; /* defined in varobj.c. */ -static int varobj_update_one (struct varobj *var, +static void varobj_update_one (struct varobj *var, enum print_values print_values); static int mi_print_value_p (struct type *type, enum print_values print_values); @@ -535,11 +535,9 @@ mi_cmd_var_update (char *command, char * return MI_CMD_DONE; } -/* Helper for mi_cmd_var_update() Returns 0 if the update for - the variable fails (usually because the variable is out of - scope), and 1 if it succeeds. */ +/* Helper for mi_cmd_var_update(). */ -static int +static void varobj_update_one (struct varobj *var, enum print_values print_values) { struct varobj **changelist; @@ -549,37 +547,39 @@ varobj_update_one (struct varobj *var, e nc = varobj_update (&var, &changelist); - /* nc == 0 means that nothing has changed. - nc == -1 means that an error occured in updating the variable. - nc == -2 means the variable has changed type. */ + /* nc >= 0 represents the number of changes reported into changelist. + nc < 0 means that an error occured or the the variable has + changed type (TYPE_CHANGED). */ if (nc == 0) - return 1; - else if (nc == -1) + return; + else if (nc < 0) { if (mi_version (uiout) > 1) cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL); ui_out_field_string (uiout, "name", varobj_get_objname(var)); - ui_out_field_string (uiout, "in_scope", "false"); - if (mi_version (uiout) > 1) - do_cleanups (cleanup); - return -1; - } - else if (nc == -2) - { - if (mi_version (uiout) > 1) - cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL); - ui_out_field_string (uiout, "name", varobj_get_objname (var)); - ui_out_field_string (uiout, "in_scope", "true"); - ui_out_field_string (uiout, "new_type", varobj_get_type(var)); - ui_out_field_int (uiout, "new_num_children", - varobj_get_num_children(var)); + + switch (nc) + { + case NOT_IN_SCOPE: + case WRONG_PARAM: + ui_out_field_string (uiout, "in_scope", "false"); + break; + case INVALID: + ui_out_field_string (uiout, "in_scope", "invalid"); + break; + case TYPE_CHANGED: + ui_out_field_string (uiout, "in_scope", "true"); + ui_out_field_string (uiout, "new_type", varobj_get_type(var)); + ui_out_field_int (uiout, "new_num_children", + varobj_get_num_children(var)); + break; + } if (mi_version (uiout) > 1) do_cleanups (cleanup); } else { - cc = changelist; while (*cc != NULL) { @@ -595,7 +595,5 @@ varobj_update_one (struct varobj *var, e cc++; } xfree (changelist); - return 1; } - return 1; } --------------010206020205060903070900 Content-Type: text/plain; name="mi-var-invalidate.exp" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="mi-var-invalidate.exp" Content-length: 3821 # Copyright 1999, 2000, 2001, 2002, 2004, 2005 Free Software # Foundation, Inc. # # This Program Is Free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by # the Free Software Foundation; either version 2 of the License, or # (at your option) any later version. # # This program is distributed in the hope that it will be useful, # but WITHOUT ANY WARRANTY; without even the implied warranty of # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. # # You should have received a copy of the GNU General Public License # along with this program; if not, write to the Free Software # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. # Test essential Machine interface (MI) operations # # Verify that once binary file has changed, GDB correctly handles # previously defined MI variables. # load_lib mi-support.exp set MIFLAGS "-i=mi" gdb_exit if [mi_gdb_start] { continue } set testfile "var-cmd" set srcfile ${testfile}.c set binfile ${objdir}/${subdir}/${testfile} if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DFAKEARGV}] != "" } { untested mi-var-invalidate.exp return -1 } # Just change the output binary. set binfile_bis ${objdir}/${subdir}/${testfile}_bis if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile_bis}" executable {debug additional_flags=-DFAKEARGV}] != "" } { untested mi-var-invalidate.exp return -1 } set testfile2 "basics" set srcfile2 ${testfile2}.c set binfile2 ${objdir}/${subdir}/${testfile2} if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug additional_flags=-DFAKEARGV}] != "" } { untested mi-var-invalidate.exp return -1 } mi_delete_breakpoints mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load ${binfile} # Desc: Create global variable mi_gdb_test "-var-create global_simple * global_simple" \ "\\^done,name=\"global_simple\",numchild=\"6\",type=\"simpleton\"" \ "create global variable" mi_runto do_locals_tests # Desc: create local variables mi_gdb_test "-var-create linteger * linteger" \ "\\^done,name=\"linteger\",numchild=\"0\",type=\"int\"" \ "create local variable linteger" # # reload the same binary # global variable must be keeped, other invalidated # mi_delete_breakpoints mi_gdb_load ${binfile_bis} mi_runto main # Check local variable are "invalid" mi_gdb_test "-var-update linteger" \ "\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"invalid\"\}\\\]" \ "linteger not anymore in scope due to binary changes" mi_gdb_test "-var-info-type linteger" \ "\\^done,type=\"\"" \ "not type for invalid variable linteger" # Check global variable are still correct. mi_gdb_test "-var-update global_simple" \ "\\^done,changelist=\\\[\]" \ "global_simple still alive" mi_gdb_test "-var-info-type global_simple" \ "\\^done,type=\"simpleton\"" \ "type simpleton for valid variable global_simple" # # load an other binary # all variables must be invalidated # mi_delete_breakpoints mi_gdb_load ${binfile2} # Check local variable are "invalid" mi_gdb_test "-var-update linteger" \ "\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"invalid\"\}\\\]" \ "linteger not valid anymore due to binary changes" mi_gdb_test "-var-info-type linteger" \ "\\^done,type=\"\"" \ "not type for invalid variable linteger" # Check global variable are still correct. mi_gdb_test "-var-update global_simple" \ "\\^done,changelist=\\\[\{name=\"global_simple\",in_scope=\"invalid\"\}\\\]" \ "global_simple not anymore in scope due to binary changes" mi_gdb_test "-var-info-type global_simple" \ "\\^done,type=\"\"" \ "not type for invalid variable global_simple" mi_gdb_exit return 0 --------------010206020205060903070900--