From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13631 invoked by alias); 3 Jul 2002 20:24:02 -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 13583 invoked from network); 3 Jul 2002 20:23:57 -0000 Received: from unknown (HELO potter.sfbay.redhat.com) (205.180.83.107) by sources.redhat.com with SMTP; 3 Jul 2002 20:23:57 -0000 Received: from localhost.localdomain (remus.sfbay.redhat.com [172.16.27.252]) by potter.sfbay.redhat.com (8.11.6/8.11.6) with ESMTP id g63KP3Q22334; Wed, 3 Jul 2002 13:25:03 -0700 From: "Martin M. Hunt" Organization: Red Hat Inc To: Andrew Cagney Subject: Re: [RFA] bug fixes for varobj.c Date: Wed, 03 Jul 2002 13:30:00 -0000 User-Agent: KMail/1.4.1 Cc: gdb-patches@sources.redhat.com References: <200206102308.40570.hunt@redhat.com> <3D080263.1020906@cygnus.com> In-Reply-To: <3D080263.1020906@cygnus.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="------------Boundary-00=_XMWOR1P792X1Q22PGLVQ" Message-Id: <200207031323.21559.hunt@redhat.com> X-SW-Source: 2002-07/txt/msg00064.txt.bz2 --------------Boundary-00=_XMWOR1P792X1Q22PGLVQ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8bit Content-length: 1906 I have fixed the patch and checked it in. Final version is attached. On Wednesday 12 June 2002 07:24 pm, Andrew Cagney wrote: > > Anything using varobj is randomly corrupting memory and will cause > > crashes in Insight or anything using mi varobjs. This patch fixes that > > and some other minor problems. > > Hmm, I'm not seeing MI test failures. Would you, by chance, have > something reproduceable for an MI testcase? Memory corruption bugs are not easy to reproduce. I never even had a reproduceable test case for Insight. The best I could do was a sequence of a dozen or so operations that crashed most of the time. I just used valgrind and tracked down all the offending code. > Anyway, separating out the changes: > > I consider the sprintf() -> xasprintf() transformations: > > (varobj_gen_name): Use xasprintf. > > (create_child): Use xasprintf. > > to be ``obvious'' and can, separatly, go straight in (only ~300 other > calls to go ...). OK. > The frame_id stuff: > > /* Save the selected stack frame, since we will need to change it > > in order to evaluate expressions. */ > > - old_fi = selected_frame; > > + get_frame_id (selected_frame, &old_fid); > > is fine except I'm not sure about: > > - var->root->frame = (CORE_ADDR) -1; > > + var->root->frame.base = (CORE_ADDR) -1; > > + var->root->frame.pc = (CORE_ADDR) -1; > > The function: > > frame_find_by_id (struct frame_id id) > > has: > > /* ZERO denotes the null frame, let the caller decide what to do > about it. Should it instead return get_current_frame()? */ > if (id.base == 0 && id.pc == 0) > return NULL; > > (see find_frame_addr_in_frame_chain for where this came from) so I think > zero would be better. Agreed. Changed in the patch. > > For the indentation changes, They were accidentally included and are removed from the patch. -- Martin Hunt GDB Engineer Red Hat, Inc. --------------Boundary-00=_XMWOR1P792X1Q22PGLVQ Content-Type: text/x-diff; charset="iso-8859-1"; name="p" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="p" Content-length: 5281 Index: varobj.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/src/src/gdb/varobj.c,v retrieving revision 1.29 diff -u -u -b -r1.29 varobj.c --- varobj.c 15 Jun 2002 18:45:31 -0000 1.29 +++ varobj.c 3 Jul 2002 20:12:26 -0000 @@ -52,7 +52,7 @@ struct block *valid_block; =20 /* The frame for this expression */ - CORE_ADDR frame; + struct frame_id frame; =20 /* If 1, "update" always recomputes the frame & valid block using the currently selected frame. */ @@ -456,7 +456,7 @@ Since select_frame is so benign, just call it for all cases. */ if (fi !=3D NULL) { - var->root->frame =3D FRAME_FP (fi); + get_frame_id (fi, &var->root->frame); old_fi =3D selected_frame; select_frame (fi); } @@ -514,13 +514,13 @@ varobj_gen_name (void) { static int id =3D 0; - char obj_name[31]; + char *obj_name; =20 /* generate a name for this object */ id++; - sprintf (obj_name, "var%d", id); + xasprintf (&obj_name, "var%d", id); =20 - return xstrdup (obj_name); + return obj_name; } =20 /* Given an "objname", returns the pointer to the corresponding varobj @@ -850,7 +850,8 @@ struct value *new; struct vstack *stack =3D NULL; struct vstack *result =3D NULL; - struct frame_info *old_fi; + struct frame_id old_fid; + struct frame_info *fi; =20 /* sanity check: have we been passed a pointer? */ if (changelist =3D=3D NULL) @@ -863,7 +864,7 @@ =20 /* Save the selected stack frame, since we will need to change it in order to evaluate expressions. */ - old_fi =3D selected_frame; + get_frame_id (selected_frame, &old_fid); =20 /* Update the root variable. value_of_root can return NULL if the variable is no longer around, i.e. we stepped out of @@ -983,7 +984,9 @@ } =20 /* Restore selected frame */ - select_frame (old_fi); + fi =3D frame_find_by_id (old_fid); + if (fi) + select_frame (fi); =20 if (type_changed) return -2; @@ -1214,10 +1217,7 @@ child->error =3D 1; child->parent =3D parent; child->root =3D parent->root; - childs_name =3D - (char *) xmalloc ((strlen (parent->obj_name) + strlen (name) + 2) * - sizeof (char)); - sprintf (childs_name, "%s.%s", parent->obj_name, name); + xasprintf (&childs_name, "%s.%s", parent->obj_name, name); child->obj_name =3D childs_name; install_variable (child); =20 @@ -1306,7 +1306,8 @@ var->root->lang =3D NULL; var->root->exp =3D NULL; var->root->valid_block =3D NULL; - var->root->frame =3D (CORE_ADDR) -1; + var->root->frame.base =3D 0; + var->root->frame.pc =3D 0; var->root->use_selected_frame =3D 0; var->root->rootvar =3D NULL; =20 @@ -1794,14 +1795,7 @@ switch (TYPE_CODE (type)) { case TYPE_CODE_ARRAY: - { - /* We never get here unless parent->num_children is greater than 0... */ - int len =3D 1; - while ((int) pow ((double) 10, (double) len) < index) - len++; - name =3D (char *) xmalloc (1 + len * sizeof (char)); - sprintf (name, "%d", index); - } + xasprintf (&name, "%d", index); break; =20 case TYPE_CODE_STRUCT: @@ -1820,9 +1814,7 @@ break; =20 default: - name =3D - (char *) xmalloc ((strlen (parent->name) + 2) * sizeof (char)); - sprintf (name, "*%s", parent->name); + xasprintf (&name, "*%s", parent->name); break; } break; @@ -1855,10 +1847,7 @@ else { reinit_frame_cache (); - - - fi =3D find_frame_addr_in_frame_chain (var->root->frame); - + fi =3D frame_find_by_id (var->root->frame); within_scope =3D fi !=3D NULL; /* FIXME: select_frame could fail */ if (within_scope) @@ -2026,12 +2015,10 @@ static char * c_value_of_variable (struct varobj *var) { - struct type *type; - /* BOGUS: if val_print sees a struct/class, it will print out its children instead of "{...}" */ - type =3D get_type (var); - switch (TYPE_CODE (type)) + + switch (TYPE_CODE (get_type (var))) { case TYPE_CODE_STRUCT: case TYPE_CODE_UNION: @@ -2040,19 +2027,14 @@ =20 case TYPE_CODE_ARRAY: { - char number[18]; - sprintf (number, "[%d]", var->num_children); - return xstrdup (number); + char *number; + xasprintf (&number, "[%d]", var->num_children); + return (number); } /* break; */ =20 default: { - long dummy; - struct ui_file *stb =3D mem_fileopen (); - struct cleanup *old_chain =3D make_cleanup_ui_file_delete (stb); - char *thevalue; - if (var->value =3D=3D NULL) { /* This can happen if we attempt to get the value of a struct @@ -2062,6 +2044,11 @@ } else { + long dummy; + struct ui_file *stb =3D mem_fileopen (); + struct cleanup *old_chain =3D make_cleanup_ui_file_delete (stb); + char *thevalue; + if (VALUE_LAZY (var->value)) gdb_value_fetch_lazy (var->value); val_print (VALUE_TYPE (var->value), @@ -2070,11 +2057,9 @@ format_code[(int) var->format], 1, 0, 0); thevalue =3D ui_file_xstrdup (stb, &dummy); do_cleanups (old_chain); - } - return thevalue; } - /* break; */ + } } } =0C --------------Boundary-00=_XMWOR1P792X1Q22PGLVQ--