Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Resizing the to_sections target vector field.
@ 1999-09-22 15:00 James Ingham
  1999-09-22 17:16 ` James Ingham
  0 siblings, 1 reply; 2+ messages in thread
From: James Ingham @ 1999-09-22 15:00 UTC (permalink / raw)
  To: gdb-patches

Hi, all...

Gdb crashes when you attach & detach a few times on Solaris native.
The bug was in a bit of code that was roughly cut & pasted around in 5
places in gdb - the bug was fixed in two places, but existed in the
others.  So I made it a function & fixed the bug there.

Look okay? 

Jim

Index: target.h
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/target.h,v
retrieving revision 1.82
diff -p -r1.82 target.h
*** target.h	1999/08/31 22:23:00	1.82
--- target.h	1999/09/22 21:01:27
*************** extern struct target_ops *find_run_targe
*** 1266,1271 ****
--- 1266,1274 ----
  
  extern struct target_ops *
    find_core_target PARAMS ((void));
+ 
+ int
+ target_resize_to_sections PARAMS ((struct target_ops *target, int num_added));
  \f
  /* Stuff that should be shared among the various remote targets.  */
  
Index: target.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/target.c,v
retrieving revision 1.105
diff -p -r1.105 target.c
*** target.c	1999/08/31 22:22:59	1.105
--- target.c	1999/09/22 21:01:28
*************** return_one ()
*** 1113,1118 ****
--- 1113,1168 ----
    return 1;
  }
  
+ /*
+  * Resize the to_sections pointer.  Also make sure that anyone that
+  * was holding on to an old value of it gets updated.
+  * Returns the old size.
+  */
+ 
+ int
+ target_resize_to_sections (struct target_ops *target, int num_added)
+ {
+   struct target_ops **t;
+   struct section_table *old_value;
+   int old_count;
+ 
+   old_value = target->to_sections;
+ 
+   if (target->to_sections)
+     {
+       old_count = target->to_sections_end - target->to_sections;
+       target->to_sections = (struct section_table *)
+ 	xrealloc ((char *) target->to_sections,
+ 		  (sizeof (struct section_table)) * (num_added + old_count));
+     }
+   else
+     {
+       old_count = 0;
+       target->to_sections = (struct section_table *)
+ 	xmalloc ((sizeof (struct section_table)) * num_added);
+     }
+   target->to_sections_end = target->to_sections + (num_added + old_count);
+ 
+   /* Check to see if anyone else was pointing to this structure.
+      If old_value was null, then no one was. */
+      
+   if (old_value)
+     {
+       for (t = target_structs; t < target_structs + target_struct_size;
+ 	   ++t)
+ 	{
+ 	  if ((*t)->to_sections == old_value)
+ 	    {
+ 	      (*t)->to_sections = target->to_sections;
+ 	      (*t)->to_sections_end = target->to_sections_end;
+ 	    }
+ 	}
+     }
+   
+   return old_count;
+ 
+ }
+ 
  /* Find a single runnable target in the stack and return it.  If for
     some reason there is more than one, return NULL.  */
  
Index: solib.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/solib.c,v
retrieving revision 1.129
diff -p -r1.129 solib.c
*** solib.c	1999/08/30 09:04:32	1.129
--- solib.c	1999/09/22 21:01:28
*************** solib_add (arg_string, from_tty, target)
*** 1174,1180 ****
  
  #endif SVR4_SHARED_LIBS
  
!   if ((re_err = re_comp (arg_string ? arg_string : ".")) != NULL)
      {
        error ("Invalid regexp: %s", re_err);
      }
--- 1174,1180 ----
  
  #endif SVR4_SHARED_LIBS
  
!   if ((re_err = re_comp (arg_string? arg_string : ".")) != NULL)
      {
        error ("Invalid regexp: %s", re_err);
      }
*************** solib_add (arg_string, from_tty, target)
*** 1196,1233 ****
  
        if (count)
  	{
! 	  int update_coreops;
! 
! 	  /* We must update the to_sections field in the core_ops structure
! 	     here, otherwise we dereference a potential dangling pointer
! 	     for each call to target_read/write_memory within this routine.  */
! 	  update_coreops = core_ops.to_sections == target->to_sections;
! 
! 	  /* Reallocate the target's section table including the new size.  */
! 	  if (target->to_sections)
! 	    {
! 	      old = target->to_sections_end - target->to_sections;
! 	      target->to_sections = (struct section_table *)
! 		xrealloc ((char *) target->to_sections,
! 			  (sizeof (struct section_table)) * (count + old));
! 	    }
! 	  else
! 	    {
! 	      old = 0;
! 	      target->to_sections = (struct section_table *)
! 		xmalloc ((sizeof (struct section_table)) * count);
! 	    }
! 	  target->to_sections_end = target->to_sections + (count + old);
! 
! 	  /* Update the to_sections field in the core_ops structure
! 	     if needed.  */
! 	  if (update_coreops)
! 	    {
! 	      core_ops.to_sections = target->to_sections;
! 	      core_ops.to_sections_end = target->to_sections_end;
! 	    }
! 
  	  /* Add these section table entries to the target's table.  */
  	  while ((so = find_solib (so)) != NULL)
  	    {
  	      if (so->so_name[0])
--- 1196,1204 ----
  
        if (count)
  	{
! 	  
  	  /* Add these section table entries to the target's table.  */
+ 	  old = target_resize_to_sections (target, count);
  	  while ((so = find_solib (so)) != NULL)
  	    {
  	      if (so->so_name[0])
Index: somsolib.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/somsolib.c,v
retrieving revision 2.32
diff -p -r2.32 somsolib.c
*** somsolib.c	1999/07/07 23:52:05	2.32
--- somsolib.c	1999/09/22 21:01:28
*************** som_solib_load_symbols (so, name, from_t
*** 375,423 ****
    if (status != 0)
      {
        int old, new;
-       int update_coreops;
-       int update_execops;
  
-       /* We must update the to_sections field in the core_ops structure
-          here, otherwise we dereference a potential dangling pointer
-          for each call to target_read/write_memory within this routine.  */
-       update_coreops = core_ops.to_sections == target->to_sections;
- 
-       /* Ditto exec_ops (this was a bug).
-        */
-       update_execops = exec_ops.to_sections == target->to_sections;
- 
        new = so->sections_end - so->sections;
!       /* Add sections from the shared library to the core target.  */
!       if (target->to_sections)
! 	{
! 	  old = target->to_sections_end - target->to_sections;
! 	  target->to_sections = (struct section_table *)
! 	    xrealloc ((char *) target->to_sections,
! 		      ((sizeof (struct section_table)) * (old + new)));
! 	}
!       else
! 	{
! 	  old = 0;
! 	  target->to_sections = (struct section_table *)
! 	    xmalloc ((sizeof (struct section_table)) * new);
! 	}
!       target->to_sections_end = (target->to_sections + old + new);
! 
!       /* Update the to_sections field in the core_ops structure
!          if needed, ditto exec_ops.  */
!       if (update_coreops)
! 	{
! 	  core_ops.to_sections = target->to_sections;
! 	  core_ops.to_sections_end = target->to_sections_end;
! 	}
! 
!       if (update_execops)
! 	{
! 	  exec_ops.to_sections = target->to_sections;
! 	  exec_ops.to_sections_end = target->to_sections_end;
! 	}
! 
        /* Copy over the old data before it gets clobbered.  */
        memcpy ((char *) (target->to_sections + old),
  	      so->sections,
--- 375,385 ----
    if (status != 0)
      {
        int old, new;
  
        new = so->sections_end - so->sections;
!       
!       old = target_resize_to_sections (target, new);
!       
        /* Copy over the old data before it gets clobbered.  */
        memcpy ((char *) (target->to_sections + old),
  	      so->sections,
Index: rs6000-nat.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/rs6000-nat.c,v
retrieving revision 2.37
diff -p -r2.37 rs6000-nat.c
*** rs6000-nat.c	1999/09/01 00:16:03	2.37
--- rs6000-nat.c	1999/09/22 21:01:28
*************** xcoff_relocate_core (target)
*** 755,782 ****
           add our sections to the section table for the core target.  */
        if (vp != vmap)
  	{
- 	  int count;
  	  struct section_table *stp;
- 	  int update_coreops;
  
! 	  /* We must update the to_sections field in the core_ops structure
! 	     now to avoid dangling pointer dereferences.  */
! 	  update_coreops = core_ops.to_sections == target->to_sections;
! 
! 	  count = target->to_sections_end - target->to_sections;
! 	  count += 2;
! 	  target->to_sections = (struct section_table *)
! 	    xrealloc (target->to_sections,
! 		      sizeof (struct section_table) * count);
! 	  target->to_sections_end = target->to_sections + count;
! 
! 	  /* Update the to_sections field in the core_ops structure
! 	     if needed.  */
! 	  if (update_coreops)
! 	    {
! 	      core_ops.to_sections = target->to_sections;
! 	      core_ops.to_sections_end = target->to_sections_end;
! 	    }
  	  stp = target->to_sections_end - 2;
  
  	  stp->bfd = vp->bfd;
--- 755,763 ----
           add our sections to the section table for the core target.  */
        if (vp != vmap)
  	{
  	  struct section_table *stp;
  
! 	  target_resize_to_sections (target, 2);
  	  stp = target->to_sections_end - 2;
  
  	  stp->bfd = vp->bfd;
Index: irix5-nat.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/irix5-nat.c,v
retrieving revision 2.35
diff -p -r2.35 irix5-nat.c
*** irix5-nat.c	1999/08/08 19:59:57	2.35
--- irix5-nat.c	1999/09/22 21:01:28
*************** solib_add (arg_string, from_tty, target)
*** 908,944 ****
  
        if (count)
  	{
! 	  int update_coreops;
! 
! 	  /* We must update the to_sections field in the core_ops structure
! 	     here, otherwise we dereference a potential dangling pointer
! 	     for each call to target_read/write_memory within this routine.  */
! 	  update_coreops = core_ops.to_sections == target->to_sections;
! 
! 	  /* Reallocate the target's section table including the new size.  */
! 	  if (target->to_sections)
! 	    {
! 	      old = target->to_sections_end - target->to_sections;
! 	      target->to_sections = (struct section_table *)
! 		xrealloc ((char *) target->to_sections,
! 			  (sizeof (struct section_table)) * (count + old));
! 	    }
! 	  else
! 	    {
! 	      old = 0;
! 	      target->to_sections = (struct section_table *)
! 		xmalloc ((sizeof (struct section_table)) * count);
! 	    }
! 	  target->to_sections_end = target->to_sections + (count + old);
! 
! 	  /* Update the to_sections field in the core_ops structure
! 	     if needed.  */
! 	  if (update_coreops)
! 	    {
! 	      core_ops.to_sections = target->to_sections;
! 	      core_ops.to_sections_end = target->to_sections_end;
! 	    }
! 
  	  /* Add these section table entries to the target's table.  */
  	  while ((so = find_solib (so)) != NULL)
  	    {
--- 908,915 ----
  
        if (count)
  	{
! 	  target_resize_to_sections (target, count);
! 	  
  	  /* Add these section table entries to the target's table.  */
  	  while ((so = find_solib (so)) != NULL)
  	    {
Index: pa64solib.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/pa64solib.c,v
retrieving revision 2.7
diff -p -r2.7 pa64solib.c
*** pa64solib.c	1999/09/18 16:33:46	2.7
--- pa64solib.c	1999/09/22 21:01:28
*************** pa64_solib_load_symbols (so, name, from_
*** 357,405 ****
    status = target_read_memory (text_addr, buf, 4);
    if (status != 0)
      {
!       int old, new;
!       int update_coreops;
!       int update_execops;
! 
!       /* We must update the to_sections field in the core_ops structure
! 	 here, otherwise we dereference a potential dangling pointer
! 	 for each call to target_read/write_memory within this routine.  */
!       update_coreops = core_ops.to_sections == target->to_sections;
! 
!       /* Ditto exec_ops (this was a bug).  */
!       update_execops = exec_ops.to_sections == target->to_sections;
! 
        new = so->sections_end - so->sections;
-       /* Add sections from the shared library to the core target.  */
-       if (target->to_sections)
- 	{
- 	  old = target->to_sections_end - target->to_sections;
- 	  target->to_sections = (struct section_table *)
- 	    xrealloc ((char *) target->to_sections,
- 		      ((sizeof (struct section_table)) * (old + new)));
- 	}
-       else
- 	{
- 	  old = 0;
- 	  target->to_sections = (struct section_table *)
- 	    xmalloc ((sizeof (struct section_table)) * new);
- 	}
-       target->to_sections_end = (target->to_sections + old + new);
- 
-       /* Update the to_sections field in the core_ops structure
- 	 if needed, ditto exec_ops.  */
-       if (update_coreops)
- 	{
- 	  core_ops.to_sections = target->to_sections;
- 	  core_ops.to_sections_end = target->to_sections_end;
- 	}
- 
-       if (update_execops)
- 	{
- 	  exec_ops.to_sections = target->to_sections;
- 	  exec_ops.to_sections_end = target->to_sections_end;
- 	}
  
        /* Copy over the old data before it gets clobbered.  */
        memcpy ((char *) (target->to_sections + old),
  	      so->sections,
--- 357,368 ----
    status = target_read_memory (text_addr, buf, 4);
    if (status != 0)
      {
!       int new, old;
!       
        new = so->sections_end - so->sections;
  
+       old = target_resize_to_sections (target, new);
+       
        /* Copy over the old data before it gets clobbered.  */
        memcpy ((char *) (target->to_sections + old),
  	      so->sections,

-- 
++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++
Jim Ingham                                              jingham@cygnus.com
Cygnus Solutions Inc.



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Resizing the to_sections target vector field.
  1999-09-22 15:00 Resizing the to_sections target vector field James Ingham
@ 1999-09-22 17:16 ` James Ingham
  0 siblings, 0 replies; 2+ messages in thread
From: James Ingham @ 1999-09-22 17:16 UTC (permalink / raw)
  To: gdb-patches

Oops, I hadn't saved all my Xemacs buffers before making the diff.  In
the osfsolib.c & irix5-nat.c files, the line

      target_resize_to_sections (target, count);

should really be:
  
      old = target_resize_to_sections (target, count);

Sorry 'bout that.

Jim

-- 
++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++==++
Jim Ingham                                              jingham@cygnus.com
Cygnus Solutions Inc.
From jimb@cygnus.com Thu Sep 23 13:18:00 1999
From: Jim Blandy <jimb@cygnus.com>
To: jtc@redback.com, David Taylor <taylor@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: patch to write_dollar_variable()
Date: Thu, 23 Sep 1999 13:18:00 -0000
Message-id: <npbtat8jsz.fsf@zwingli.cygnus.com>
References: <5maerlncv8.fsf@jtc.redbacknetworks.com>
X-SW-Source: 1999-q3/msg00264.html
Content-length: 5476

> As previously discussed on the gdb list, this patch is necessary.  At
> least until symbol table handling is greatly improved... :-)
> 
>         --jtc
> 
> 
> 1999-08-20  J.T. Conklin  <jtc@redback.com>
> 
> 	* parse.c (write_dollar_variable): If HPUXHPPA is not defined,
>  	don't search for $ variables in the symbol table.  Worst case
>  	symbol table lookup performance is extremely poor.  This causes
>  	GDB scripts that use convenience variables to execute so slowly
> 	to be almost unusable.

Testing HPUXHPPA is a mess, though.  How about this?

1999-09-23  Jim Blandy  <jimb@zwingli.cygnus.com>

	* parse.c (write_dollar_variable): Don't check the symbol table
	for identifiers beginning with `$' unless
	IDENTIFIERS_CAN_START_WITH_DOLLAR is #defined.
	* config/pa/tm-hppa.h (IDENTIFIERS_CAN_START_WITH_DOLLAR): Define.
	* doc/gdbint.texinfo (IDENTIFIERS_CAN_START_WITH_DOLLAR): Document.

Index: parse.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/parse.c,v
retrieving revision 2.67
diff -c -r2.67 parse.c
*** parse.c	1999/09/01 20:50:45	2.67
--- parse.c	1999/09/23 20:14:52
***************
*** 460,468 ****
    /* Handle the tokens $digits; also $ (short for $0) and $$ (short for $$1)
       and $$digits (equivalent to $<-digits> if you could type that). */
  
-   struct symbol *sym = NULL;
-   struct minimal_symbol *msym = NULL;
- 
    int negate = 0;
    int i = 1;
    /* Double dollar means negate the number and add -1 as well.
--- 460,465 ----
***************
*** 496,523 ****
    if (i >= 0)
      goto handle_register;
  
!   /* On HP-UX, certain system routines (millicode) have names beginning
!      with $ or $$, e.g. $$dyncall, which handles inter-space procedure
!      calls on PA-RISC. Check for those, first. */
  
!   sym = lookup_symbol (copy_name (str), (struct block *) NULL,
! 		       VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
!   if (sym)
!     {
!       write_exp_elt_opcode (OP_VAR_VALUE);
!       write_exp_elt_block (block_found);	/* set by lookup_symbol */
!       write_exp_elt_sym (sym);
!       write_exp_elt_opcode (OP_VAR_VALUE);
!       return;
!     }
!   msym = lookup_minimal_symbol (copy_name (str), NULL, NULL);
!   if (msym)
!     {
!       write_exp_msymbol (msym,
! 			 lookup_function_type (builtin_type_int),
! 			 builtin_type_int);
!       return;
!     }
  
    /* Any other names starting in $ are debugger internal variables.  */
  
--- 493,530 ----
    if (i >= 0)
      goto handle_register;
  
! #if defined(IDENTIFIERS_CAN_START_WITH_DOLLAR)
!   {
!     struct symbol *sym = NULL;
!     struct minimal_symbol *msym = NULL;
  
!     /* On HP-UX, certain system routines (millicode) have names beginning
!        with $ or $$, e.g. $$dyncall, which handles inter-space procedure
!        calls on PA-RISC. Check for those, first. */
! 
!     /* This code is not enabled on non HP-UX systems, since worst case 
!        symbol table lookup performance is awful, to put it mildly. */
! 
!     sym = lookup_symbol (copy_name (str), (struct block *) NULL,
! 			 VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
!     if (sym)
!       {
! 	write_exp_elt_opcode (OP_VAR_VALUE);
! 	write_exp_elt_block (block_found);	/* set by lookup_symbol */
! 	write_exp_elt_sym (sym);
! 	write_exp_elt_opcode (OP_VAR_VALUE);
! 	return;
!       }
!     msym = lookup_minimal_symbol (copy_name (str), NULL, NULL);
!     if (msym)
!       {
! 	write_exp_msymbol (msym,
! 			   lookup_function_type (builtin_type_int),
! 			   builtin_type_int);
! 	return;
!       }
!   }
! #endif
  
    /* Any other names starting in $ are debugger internal variables.  */
  
Index: config/pa/tm-hppa.h
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/config/pa/tm-hppa.h,v
retrieving revision 1.76
diff -c -r1.76 tm-hppa.h
*** tm-hppa.h	1999/09/17 16:12:32	1.76
--- tm-hppa.h	1999/09/23 20:14:53
***************
*** 799,801 ****
--- 799,807 ----
  /* Here's how to step off a permanent breakpoint.  */
  #define SKIP_PERMANENT_BREAKPOINT (hppa_skip_permanent_breakpoint)
  extern void hppa_skip_permanent_breakpoint (void);
+ 
+ /* On HP-UX, certain system routines (millicode) have names beginning
+    with $ or $$, e.g. $$dyncall, which handles inter-space procedure
+    calls on PA-RISC.  Tell the expression parser to check for those
+    when parsing tokens that begin with "$".  */
+ #define IDENTIFIERS_CAN_START_WITH_DOLLAR
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/doc/gdbint.texinfo,v
retrieving revision 1.136
diff -c -r1.136 gdbint.texinfo
*** gdbint.texinfo	1999/09/15 21:27:39	1.136
--- gdbint.texinfo	1999/09/23 20:14:58
***************
*** 1444,1449 ****
--- 1444,1458 ----
  feature-specific macros.  It was introduced in haste and we are
  repenting at leisure.
  
+ @item IDENTIFIERS_CAN_START_WITH_DOLLAR
+ Some systems have routines whose names start with @samp{$}.  Defining
+ this macro tells GDB's expression parser to check for such routines when
+ parsing tokens that begin with @samp{$}.
+ 
+ On HP-UX, certain system routines (millicode) have names beginning with
+ @samp{$} or @samp{$$}.  For example, @code{$$dyncall} is a millicode
+ routine that handles inter-space procedure calls on PA-RISC.
+ 
  @item IEEE_FLOAT
  Define this if the target system uses IEEE-format floating point numbers.
  
From shebs@cygnus.com Thu Sep 23 13:53:00 1999
From: Stan Shebs <shebs@cygnus.com>
To: jimb@cygnus.com
Cc: jtc@redback.com, taylor@cygnus.com, gdb-patches@sourceware.cygnus.com
Subject: Re: patch to write_dollar_variable()
Date: Thu, 23 Sep 1999 13:53:00 -0000
Message-id: <199909232052.NAA02680@andros.cygnus.com>
References: <npbtat8jsz.fsf@zwingli.cygnus.com>
X-SW-Source: 1999-q3/msg00265.html
Content-length: 1612

   From: Jim Blandy <jimb@cygnus.com>
   Date: 23 Sep 1999 15:17:00 -0500

   > As previously discussed on the gdb list, this patch is necessary.  At
   > least until symbol table handling is greatly improved... :-)
   > 
   >         --jtc
   > 
   > 
   > 1999-08-20  J.T. Conklin  <jtc@redback.com>
   > 
   > 	* parse.c (write_dollar_variable): If HPUXHPPA is not defined,
   >  	don't search for $ variables in the symbol table.  Worst case
   >  	symbol table lookup performance is extremely poor.  This causes
   >  	GDB scripts that use convenience variables to execute so slowly
   > 	to be almost unusable.

   Testing HPUXHPPA is a mess, though.  How about this?

   1999-09-23  Jim Blandy  <jimb@zwingli.cygnus.com>

	   * parse.c (write_dollar_variable): Don't check the symbol table
	   for identifiers beginning with `$' unless
	   IDENTIFIERS_CAN_START_WITH_DOLLAR is #defined.
	   * config/pa/tm-hppa.h (IDENTIFIERS_CAN_START_WITH_DOLLAR): Define.
	   * doc/gdbint.texinfo (IDENTIFIERS_CAN_START_WITH_DOLLAR): Document.

This is going in the right direction, but to keep in line with our
future, I'd prefer to see a runtime test:

	if (IDENTIFIERS_CAN_START_WITH_DOLLAR)

that is defaulted to 0 somewhere convenient (like at the top of
parse.c).  This shouldn't cost any more, because GCC can optimize out
the whole block on non-HPs, and will always optimize in on HPs.  If
this ends up going through a gdbarch op in the future, there will be a
small extra cost, although I'm sure that there are many better
opportunities for script optimization before this test becomes an
issue.

								Stan
From jimb@cygnus.com Thu Sep 23 16:46:00 1999
From: Jim Blandy <jimb@cygnus.com>
To: Stan Shebs <shebs@cygnus.com>
Cc: jtc@redback.com, taylor@cygnus.com, gdb-patches@sourceware.cygnus.com
Subject: Re: patch to write_dollar_variable()
Date: Thu, 23 Sep 1999 16:46:00 -0000
Message-id: <np905x8a79.fsf@zwingli.cygnus.com>
References: <199909232052.NAA02680@andros.cygnus.com>
X-SW-Source: 1999-q3/msg00266.html
Content-length: 6101

> This is going in the right direction, but to keep in line with our
> future, I'd prefer to see a runtime test:
> 
> 	if (IDENTIFIERS_CAN_START_WITH_DOLLAR)
> 
> that is defaulted to 0 somewhere convenient (like at the top of
> parse.c).

Duh.  I knew that...

1999-09-23  Jim Blandy  <jimb@zwingli.cygnus.com>

	* parse.c (IDENTIFIERS_CAN_START_WITH_DOLLAR): New macro,
	whose value can be overridden by target files.
	(write_dollar_variable): Don't check the symbol table for
	identifiers beginning with `$' unless
	IDENTIFIERS_CAN_START_WITH_DOLLAR is non-zero.
	* config/pa/tm-hppa.h (IDENTIFIERS_CAN_START_WITH_DOLLAR): Define.
	* doc/gdbint.texinfo (IDENTIFIERS_CAN_START_WITH_DOLLAR): Document.

Index: parse.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/parse.c,v
retrieving revision 2.67
diff -c -r2.67 parse.c
*** parse.c	1999/09/01 20:50:45	2.67
--- parse.c	1999/09/23 23:39:00
***************
*** 44,49 ****
--- 44,64 ----
  #include "gdbcmd.h"
  #include "symfile.h"		/* for overlay functions */
  \f
+ /* Symbols which architectures can redefine.  */
+ 
+ /* Some systems have routines whose names start with `$'.  Giving this
+    macro a non-zero value tells GDB's expression parser to check for
+    such routines when parsing tokens that begin with `$'.
+ 
+    On HP-UX, certain system routines (millicode) have names beginning
+    with `$' or `$$'.  For example, `$$dyncall' is a millicode routine
+    that handles inter-space procedure calls on PA-RISC.  */
+ #ifndef IDENTIFIERS_CAN_START_WITH_DOLLAR
+ #define IDENTIFIERS_CAN_START_WITH_DOLLAR (0)
+ #endif
+ 
+ 
+ \f
  /* Global variables declared in parser-defs.h (and commented there).  */
  struct expression *expout;
  int expout_size;
***************
*** 460,468 ****
    /* Handle the tokens $digits; also $ (short for $0) and $$ (short for $$1)
       and $$digits (equivalent to $<-digits> if you could type that). */
  
-   struct symbol *sym = NULL;
-   struct minimal_symbol *msym = NULL;
- 
    int negate = 0;
    int i = 1;
    /* Double dollar means negate the number and add -1 as well.
--- 475,480 ----
***************
*** 496,522 ****
    if (i >= 0)
      goto handle_register;
  
!   /* On HP-UX, certain system routines (millicode) have names beginning
!      with $ or $$, e.g. $$dyncall, which handles inter-space procedure
!      calls on PA-RISC. Check for those, first. */
! 
!   sym = lookup_symbol (copy_name (str), (struct block *) NULL,
! 		       VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
!   if (sym)
!     {
!       write_exp_elt_opcode (OP_VAR_VALUE);
!       write_exp_elt_block (block_found);	/* set by lookup_symbol */
!       write_exp_elt_sym (sym);
!       write_exp_elt_opcode (OP_VAR_VALUE);
!       return;
!     }
!   msym = lookup_minimal_symbol (copy_name (str), NULL, NULL);
!   if (msym)
      {
!       write_exp_msymbol (msym,
! 			 lookup_function_type (builtin_type_int),
! 			 builtin_type_int);
!       return;
      }
  
    /* Any other names starting in $ are debugger internal variables.  */
--- 508,543 ----
    if (i >= 0)
      goto handle_register;
  
!   if (IDENTIFIERS_CAN_START_WITH_DOLLAR)
      {
!       struct symbol *sym = NULL;
!       struct minimal_symbol *msym = NULL;
! 
!       /* On HP-UX, certain system routines (millicode) have names beginning
! 	 with $ or $$, e.g. $$dyncall, which handles inter-space procedure
! 	 calls on PA-RISC. Check for those, first. */
! 
!       /* This code is not enabled on non HP-UX systems, since worst case 
! 	 symbol table lookup performance is awful, to put it mildly. */
! 
!       sym = lookup_symbol (copy_name (str), (struct block *) NULL,
! 			   VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
!       if (sym)
! 	{
! 	  write_exp_elt_opcode (OP_VAR_VALUE);
! 	  write_exp_elt_block (block_found);	/* set by lookup_symbol */
! 	  write_exp_elt_sym (sym);
! 	  write_exp_elt_opcode (OP_VAR_VALUE);
! 	  return;
! 	}
!       msym = lookup_minimal_symbol (copy_name (str), NULL, NULL);
!       if (msym)
! 	{
! 	  write_exp_msymbol (msym,
! 			     lookup_function_type (builtin_type_int),
! 			     builtin_type_int);
! 	  return;
! 	}
      }
  
    /* Any other names starting in $ are debugger internal variables.  */
Index: config/pa/tm-hppa.h
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/config/pa/tm-hppa.h,v
retrieving revision 1.76
diff -c -r1.76 tm-hppa.h
*** tm-hppa.h	1999/09/17 16:12:32	1.76
--- tm-hppa.h	1999/09/23 23:39:01
***************
*** 799,801 ****
--- 799,807 ----
  /* Here's how to step off a permanent breakpoint.  */
  #define SKIP_PERMANENT_BREAKPOINT (hppa_skip_permanent_breakpoint)
  extern void hppa_skip_permanent_breakpoint (void);
+ 
+ /* On HP-UX, certain system routines (millicode) have names beginning
+    with $ or $$, e.g. $$dyncall, which handles inter-space procedure
+    calls on PA-RISC.  Tell the expression parser to check for those
+    when parsing tokens that begin with "$".  */
+ #define IDENTIFIERS_CAN_START_WITH_DOLLAR
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/doc/gdbint.texinfo,v
retrieving revision 1.136
diff -c -r1.136 gdbint.texinfo
*** gdbint.texinfo	1999/09/15 21:27:39	1.136
--- gdbint.texinfo	1999/09/23 23:39:05
***************
*** 1444,1449 ****
--- 1444,1458 ----
  feature-specific macros.  It was introduced in haste and we are
  repenting at leisure.
  
+ @item IDENTIFIERS_CAN_START_WITH_DOLLAR
+ Some systems have routines whose names start with @samp{$}.  Giving this
+ macro a non-zero value tells GDB's expression parser to check for such
+ routines when parsing tokens that begin with @samp{$}.
+ 
+ On HP-UX, certain system routines (millicode) have names beginning with
+ @samp{$} or @samp{$$}.  For example, @code{$$dyncall} is a millicode
+ routine that handles inter-space procedure calls on PA-RISC.
+ 
  @item IEEE_FLOAT
  Define this if the target system uses IEEE-format floating point numbers.
  
From shebs@cygnus.com Thu Sep 23 19:44:00 1999
From: Stan Shebs <shebs@cygnus.com>
To: kevinb@cygnus.com
Cc: jimb@cygnus.com, msnyder@cygnus.com, gdb-patches@sourceware.cygnus.com
Subject: Re: RFA: breakpoint.c and infrun.c changes
Date: Thu, 23 Sep 1999 19:44:00 -0000
Message-id: <199909240241.TAA03085@andros.cygnus.com>
References: <990917194445.ZM3796@ocotillo.lan>
X-SW-Source: 1999-q3/msg00267.html
Content-length: 1665

   Date: Fri, 17 Sep 1999 12:44:45 -0700
   From: Kevin Buettner <kevinb@cygnus.com>

   My patch for fixing this bug is below.  The changes to breakpoint.c
   alone fix the bug (on the i386 anyway); the changes to infrun.c are
   cleanup.

A very nice piece of analysis!  I had to read through it about four
times to understand it all, but the reasoning seems sound.  As we
discussed on the phone, testing on a small assortment of relevant
platforms is still worthwhile, just in case practice is trickier than
theory. :-)

This will certainly help to shorten and straighten out the execution
control code.  As people have accreted hacks over the years, it becomes
harder and harder to know why something is there, and one descends into
voodoo-fixing (as old ChangeLog entries attest).

I'd also like us to look into JimB's suggestion that the PC could be
pre-adjusted according to DECR_PC_AFTER_BREAK before anything in
execution control looks at it.  The analysis for that would involve
looking at all the places where the PC is saved away, there are quite
a few of them.

In any case, assuming testing doesn't report any regression, I'd like
to get this patch in soon so I can do my next phase of infrun
simplification - there are just three more gotos left in
handle_inferior_event, but so far I can't see how to evaporate them
validly without breaking the function into a number of smaller pieces.

Also, it would be really great to add a testsuite case for temporary
breakpoints and stepping that will always fail without your changes,
and will pass otherwise, since it seems that there is currently no
test to detect the failure you're fixing.

								Stan
From kevinb@cygnus.com Fri Sep 24 10:40:00 1999
From: Kevin Buettner <kevinb@cygnus.com>
To: Stan Shebs <shebs@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com, jimb@cygnus.com, msnyder@cygnus.com
Subject: Re: RFA: breakpoint.c and infrun.c changes
Date: Fri, 24 Sep 1999 10:40:00 -0000
Message-id: <990924173707.ZM19747@ocotillo.lan>
References: <199909240241.TAA03085@andros.cygnus.com> <shebs@cygnus.com>
X-SW-Source: 1999-q3/msg00268.html
Content-length: 2507

On Sep 23,  7:41pm, Stan Shebs wrote:

> A very nice piece of analysis!  I had to read through it about four
> times to understand it all, but the reasoning seems sound.  As we
> discussed on the phone, testing on a small assortment of relevant
> platforms is still worthwhile, just in case practice is trickier than
> theory. :-)

As I told you on the phone, I've tested on i386 linux and saw no
regressions.

I've tested with the d10v simulator and saw no regressions.

I tried to test the i960, but the i960 simulator did not work well for
me.  I'll try against an actual board if you think it's important. 
(After looking at the nightly testing results for this target, I have
doubts about being able to get it to work properly.  The one recent
test that did work properly had 1034 failures.  But most of the recent
and even not so recent tests show that the testsuite did not complete
successfully.)

I've tested on the alpha (running linux), but I'm not sure what to
make of the results.  On the one hand, I do see *fewer* failures after
I apply my patch.  But I've run the test suite a number of times and
get different results each time I run it.  The good thing is that
there aren't any new failures in the breakpoint or single step tests. 
(I ran my tests on the machine called `dot'.)

> I'd also like us to look into JimB's suggestion that the PC could be
> pre-adjusted according to DECR_PC_AFTER_BREAK before anything in
> execution control looks at it.  The analysis for that would involve
> looking at all the places where the PC is saved away, there are quite
> a few of them.

I'll make this a background task.

> In any case, assuming testing doesn't report any regression, I'd like
> to get this patch in soon so I can do my next phase of infrun
> simplification - there are just three more gotos left in
> handle_inferior_event, but so far I can't see how to evaporate them
> validly without breaking the function into a number of smaller pieces.

Okay.  Let me know what you want me to do about testing the i960. 
Also, I'd like Michael Snyder's blessing for the breakpoint.c changes.

> Also, it would be really great to add a testsuite case for temporary
> breakpoints and stepping that will always fail without your changes,
> and will pass otherwise, since it seems that there is currently no
> test to detect the failure you're fixing.

I'll construct such a test and have you take a look at it before
committing it.

Kevin

-- 
Kevin Buettner
kev@primenet.com, kevinb@cygnus.com
From shebs@cygnus.com Fri Sep 24 11:43:00 1999
From: Stan Shebs <shebs@cygnus.com>
To: kevinb@cygnus.com
Cc: gdb-patches@sourceware.cygnus.com, jimb@cygnus.com, msnyder@cygnus.com
Subject: Re: RFA: breakpoint.c and infrun.c changes
Date: Fri, 24 Sep 1999 11:43:00 -0000
Message-id: <199909241841.LAA03933@andros.cygnus.com>
References: <990924173707.ZM19747@ocotillo.lan>
X-SW-Source: 1999-q3/msg00269.html
Content-length: 1628

   Date: Fri, 24 Sep 1999 10:37:07 -0700
   From: Kevin Buettner <kevinb@cygnus.com>

   I've tested with the d10v simulator and saw no regressions.

Great!

   I tried to test the i960, but the i960 simulator did not work well for
   me.  I'll try against an actual board if you think it's important. 
   (After looking at the nightly testing results for this target, I have
   doubts about being able to get it to work properly.  The one recent
   test that did work properly had 1034 failures.  But most of the recent
   and even not so recent tests show that the testsuite did not complete
   successfully.)

I think we can safely skip over testing the i960 for this patch.  I
would like to know why the testing is having trouble, but that's not
your problem...

   I've tested on the alpha (running linux), but I'm not sure what to
   make of the results.  On the one hand, I do see *fewer* failures after
   I apply my patch.  But I've run the test suite a number of times and
   get different results each time I run it.  The good thing is that
   there aren't any new failures in the breakpoint or single step tests. 
   (I ran my tests on the machine called `dot'.)

Hmmm, when I ran tests on Alpha Linux a couple months ago I was
getting pretty consistent results.  Perhaps you could save a couple of
the varying logs and point me at them?

In any case, the results sound good, and the patch has been tested
with a wider variety of targets than most patches get, so there's no
reason not to check in now.  If you get it in today, then it can be in
Monday's snap for everybody to try out for themselves.

								Stan


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~1999-09-22 17:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-09-22 15:00 Resizing the to_sections target vector field James Ingham
1999-09-22 17:16 ` James Ingham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox