only force symbol lookups with local variables on hppa

only force symbol lookups with local variables on hppa

[Mike Frysinger]

[-- Attachment #1.1: Type: text/plain, Size: 293 bytes --]

as a follow up to the slow script thread:
http://sources.redhat.com/ml/gdb/2006-08/msg00099.html

attached patch will change the behavior so symbols with dollar signs in them 
($) are only looked up when a specific arch says to do so (with hppa being 
the only one at the moment)
-mike

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: gdb-hppa-dollar-lookups.patch --]
[-- Type: text/x-diff, Size: 920 bytes --]

2006-08-20  Mike Frysinger  <vapier@gentoo.org>

	* parse.c (write_dollar_variable): Check LOOKUP_DOLLAR_SYMBOLS.
	* config/pa/tm-hppa.h [LOOKUP_DOLLAR_SYMBOLS]: Define.

--- gdb/parse.c
+++ gdb/parse.c
@@ -489,8 +489,12 @@ write_dollar_variable (struct stoken str
   /* On some systems, such as HP-UX and hppa-linux, certain system routines 
      have names beginning with $ or $$.  Check for those, first. */
 
+#ifdef LOOKUP_DOLLAR_SYMBOLS
   sym = lookup_symbol (copy_name (str), (struct block *) NULL,
 		       VAR_DOMAIN, (int *) NULL, (struct symtab **) NULL);
+#else
+  sym = NULL;
+#endif
   if (sym)
     {
       write_exp_elt_opcode (OP_VAR_VALUE);
--- gdb/config/pa/tm-hppa.h
+++ gdb/config/pa/tm-hppa.h
@@ -28,3 +28,5 @@
 
 extern int hppa_pc_requires_run_before_use (CORE_ADDR pc);
 #define DEPRECATED_PC_REQUIRES_RUN_BEFORE_USE(pc) hppa_pc_requires_run_before_use (pc)
+
+#define LOOKUP_DOLLAR_SYMBOLS

Re: only force symbol lookups with local variables on hppa

[Mark Kettenis]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Sun, 20 Aug 2006 09:03:06 -0400
> 
> as a follow up to the slow script thread:
> http://sources.redhat.com/ml/gdb/2006-08/msg00099.html
> 
> attached patch will change the behavior so symbols with dollar signs
> in them0 ($) are only looked up when a specific arch says to do so
> (with hppa being the only one at the moment)
> -mike

> 2006-08-20  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* parse.c (write_dollar_variable): Check LOOKUP_DOLLAR_SYMBOLS.
> 	* config/pa/tm-hppa.h [LOOKUP_DOLLAR_SYMBOLS]: Define.

Sorry but, this implementation is not acceptable; we want to get rid
of all the defines in tm.h files (and the files themselves
eventually), not add more :(.

Mark

Re: only force symbol lookups with local variables on hppa

[Daniel Jacobowitz]

On Sun, Aug 20, 2006 at 03:10:42PM +0200, Mark Kettenis wrote:
> > From: Mike Frysinger <vapier@gentoo.org>
> > Date: Sun, 20 Aug 2006 09:03:06 -0400
> > 
> > as a follow up to the slow script thread:
> > http://sources.redhat.com/ml/gdb/2006-08/msg00099.html
> > 
> > attached patch will change the behavior so symbols with dollar signs
> > in them0 ($) are only looked up when a specific arch says to do so
> > (with hppa being the only one at the moment)
> > -mike
> 
> > 2006-08-20  Mike Frysinger  <vapier@gentoo.org>
> > 
> > 	* parse.c (write_dollar_variable): Check LOOKUP_DOLLAR_SYMBOLS.
> > 	* config/pa/tm-hppa.h [LOOKUP_DOLLAR_SYMBOLS]: Define.
> 
> Sorry but, this implementation is not acceptable; we want to get rid
> of all the defines in tm.h files (and the files themselves
> eventually), not add more :(.

As a followup, there are several better options:

  - Set a gdbarch hook.  See gdbarch.sh.
  - Keep track of the number of symbols found which start with
    a '$', probably in each objfile.  Only do the lookup if
    symbols starting with dollar have been found.
  - Just make partial symbol lookups faster.

-- 
Daniel Jacobowitz
CodeSourcery

Re: only force symbol lookups with local variables on hppa

[Mark Kettenis]

> Date: Sun, 20 Aug 2006 09:57:54 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Sun, Aug 20, 2006 at 03:10:42PM +0200, Mark Kettenis wrote:
> > > From: Mike Frysinger <vapier@gentoo.org>
> > > Date: Sun, 20 Aug 2006 09:03:06 -0400
> > > 
> > > as a follow up to the slow script thread:
> > > http://sources.redhat.com/ml/gdb/2006-08/msg00099.html
> > > 
> > > attached patch will change the behavior so symbols with dollar signs
> > > in them0 ($) are only looked up when a specific arch says to do so
> > > (with hppa being the only one at the moment)
> > > -mike
> > 
> > > 2006-08-20  Mike Frysinger  <vapier@gentoo.org>
> > > 
> > > 	* parse.c (write_dollar_variable): Check LOOKUP_DOLLAR_SYMBOLS.
> > > 	* config/pa/tm-hppa.h [LOOKUP_DOLLAR_SYMBOLS]: Define.
> > 
> > Sorry but, this implementation is not acceptable; we want to get rid
> > of all the defines in tm.h files (and the files themselves
> > eventually), not add more :(.
> 
> As a followup, there are several better options:
> 
>   - Set a gdbarch hook.  See gdbarch.sh.
>   - Keep track of the number of symbols found which start with
>     a '$', probably in each objfile.  Only do the lookup if
>     symbols starting with dollar have been found.
>   - Just make partial symbol lookups faster.

And of these the last two are certainly preferable to the first
solution.

Actually, I'm afraid the assumption that symbols do not start with a
dollar sign isn't true on most platforms: I just created some
assembler code that contains such symbols and it assembles, links and
runs fine on OpenBSD/i386.  I have no doubt that the same will be true
on other ELF systems using the GNU toolchain.

Mark

Re: only force symbol lookups with local variables on hppa

[Daniel Jacobowitz]

On Mon, Aug 21, 2006 at 08:29:41PM +0200, Mark Kettenis wrote:
> Actually, I'm afraid the assumption that symbols do not start with a
> dollar sign isn't true on most platforms: I just created some
> assembler code that contains such symbols and it assembles, links and
> runs fine on OpenBSD/i386.  I have no doubt that the same will be true
> on other ELF systems using the GNU toolchain.

That's right; it's just not typical except on HPPA, where it involves
some symbols we actually care about (millicode routines, I think?).
Many C compilers allow dollars in the middle of symbols; they don't
allow them at the start, but the underlying assembler generally does.
ELF doesn't care at all, of course.

-- 
Daniel Jacobowitz
CodeSourcery

Re: only force symbol lookups with local variables on hppa

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

On Sunday 20 August 2006 09:57, Daniel Jacobowitz wrote:
> As a followup, there are several better options:

hmm, is there a spec somewhere that says what order symbols need to be 
resolved ?  how about an alternative option:
 - in write_dollar_variable(), if the symbol starts with a '$', we first try 
to look it up via lookup_internalvar() ... if that fails, we fall back to 
lookup_symbol() and lookup_minimal_symbol() and then jump back to the 
lookup_internalvar() ...
-mike

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: only force symbol lookups with local variables on hppa

[Daniel Jacobowitz]

On Mon, Aug 21, 2006 at 09:32:29PM -0400, Mike Frysinger wrote:
> On Sunday 20 August 2006 09:57, Daniel Jacobowitz wrote:
> > As a followup, there are several better options:
> 
> hmm, is there a spec somewhere that says what order symbols need to be 
> resolved ?  how about an alternative option:
>  - in write_dollar_variable(), if the symbol starts with a '$', we first try 
> to look it up via lookup_internalvar() ... if that fails, we fall back to 
> lookup_symbol() and lookup_minimal_symbol() and then jump back to the 
> lookup_internalvar() ...

Hmm... that's also an interesting option.  I have no opinion.

-- 
Daniel Jacobowitz
CodeSourcery

Re: only force symbol lookups with local variables on hppa

[Mike Frysinger]

[-- Attachment #1.1: Type: text/plain, Size: 768 bytes --]

On Monday 21 August 2006 21:35, Daniel Jacobowitz wrote:
> On Mon, Aug 21, 2006 at 09:32:29PM -0400, Mike Frysinger wrote:
> > On Sunday 20 August 2006 09:57, Daniel Jacobowitz wrote:
> > > As a followup, there are several better options:
> >
> > hmm, is there a spec somewhere that says what order symbols need to be
> > resolved ?  how about an alternative option:
> >  - in write_dollar_variable(), if the symbol starts with a '$', we first
> > try to look it up via lookup_internalvar() ... if that fails, we fall
> > back to lookup_symbol() and lookup_minimal_symbol() and then jump back to
> > the lookup_internalvar() ...
>
> Hmm... that's also an interesting option.  I have no opinion.

here's a stab at it then ... i'm sure others have better ideas :)
-mike

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: gdb-lookup-internal-first.patch --]
[-- Type: text/x-diff, Size: 2899 bytes --]

2006-08-21  Mike Frysinger  <vapier@gentoo.org>

	* value.h (lookup_only_internalvar): New prototype.
	* value.c (lookup_only_internalvar): New function.
	(lookup_internalvar): Use lookup_only_internalvar.
	* parse.c (write_dollar_variable): Look up $ symbols in internal
	table first rather than last.

--- gdb/value.h
+++ gdb/value.h
@@ -419,6 +419,8 @@ extern void set_internalvar_component (s
 				       int bitpos, int bitsize,
 				       struct value *newvalue);
 
+extern struct internalvar *lookup_only_internalvar (char *name);
+
 extern struct internalvar *lookup_internalvar (char *name);
 
 extern int value_equal (struct value *arg1, struct value *arg2);
--- gdb/value.c
+++ gdb/value.c
@@ -741,10 +741,10 @@ init_if_undefined_command (char* args, i
    normally include a dollar sign.
 
    If the specified internal variable does not exist,
-   one is created, with a void value.  */
+   the return value is NULL.  */
 
 struct internalvar *
-lookup_internalvar (char *name)
+lookup_only_internalvar (char *name)
 {
   struct internalvar *var;
 
@@ -752,6 +752,25 @@ lookup_internalvar (char *name)
     if (strcmp (var->name, name) == 0)
       return var;
 
+  return NULL;
+}
+
+
+/* Look up an internal variable with name NAME.  NAME should not
+   normally include a dollar sign.
+
+   If the specified internal variable does not exist,
+   one is created, with a void value.  */
+
+struct internalvar *
+lookup_internalvar (char *name)
+{
+  struct internalvar *var;
+
+  var = lookup_only_internalvar (name);
+  if (var)
+    return var;
+
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
--- gdb/parse.c
+++ gdb/parse.c
@@ -486,6 +486,17 @@ write_dollar_variable (struct stoken str
   if (i >= 0)
     goto handle_register;
 
+  /* Any names starting with $ are probably debugger internal variables.  */
+
+  if (str.ptr[0] == '$' && lookup_only_internalvar (copy_name (str) + 1))
+    {
+internal_lookup:
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      return;
+    }
+
   /* On some systems, such as HP-UX and hppa-linux, certain system routines 
      have names beginning with $ or $$.  Check for those, first. */
 
@@ -508,12 +519,9 @@ write_dollar_variable (struct stoken str
       return;
     }
 
-  /* Any other names starting in $ are debugger internal variables.  */
-
-  write_exp_elt_opcode (OP_INTERNALVAR);
-  write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
-  write_exp_elt_opcode (OP_INTERNALVAR);
-  return;
+  /* Any other names are assumed to be debugger internal variables.  */
+
+  goto internal_lookup;
 handle_last:
   write_exp_elt_opcode (OP_LAST);
   write_exp_elt_longcst ((LONGEST) i);

[patch ping] change lookup order of $localvars to happen before symbol tables

[Mike Frysinger]

[-- Attachment #1.1: Type: text/plain, Size: 648 bytes --]

On Monday 21 August 2006 21:35, Daniel Jacobowitz wrote:
> On Mon, Aug 21, 2006 at 09:32:29PM -0400, Mike Frysinger wrote:
> > hmm, is there a spec somewhere that says what order symbols need to be
> > resolved ?  how about an alternative option:
> >  - in write_dollar_variable(), if the symbol starts with a '$', we first
> > try to look it up via lookup_internalvar() ... if that fails, we fall
> > back to lookup_symbol() and lookup_minimal_symbol() and then jump back to
> > the lookup_internalvar() ...
>
> Hmm... that's also an interesting option.  I have no opinion.

here's a stab at it then ... i'm sure others have better ideas :)
-mike

[-- Attachment #1.2: gdb-lookup-internal-first.patch --]
[-- Type: text/x-diff, Size: 2995 bytes --]

2006-08-21  Mike Frysinger  <vapier@gentoo.org>

	* value.h (lookup_only_internalvar): New prototype.
	* value.c (lookup_only_internalvar): New function.
	(lookup_internalvar): Use lookup_only_internalvar.
	* parse.c (write_dollar_variable): Look up $ symbols in internal
	table first rather than last.

--- gdb/value.h
+++ gdb/value.h
@@ -419,6 +419,8 @@ extern void set_internalvar_component (s
 				       int bitpos, int bitsize,
 				       struct value *newvalue);
 
+extern struct internalvar *lookup_only_internalvar (char *name);
+
 extern struct internalvar *lookup_internalvar (char *name);
 
 extern int value_equal (struct value *arg1, struct value *arg2);
--- gdb/value.c
+++ gdb/value.c
@@ -741,10 +741,10 @@ init_if_undefined_command (char* args, i
    normally include a dollar sign.
 
    If the specified internal variable does not exist,
-   one is created, with a void value.  */
+   the return value is NULL.  */
 
 struct internalvar *
-lookup_internalvar (char *name)
+lookup_only_internalvar (char *name)
 {
   struct internalvar *var;
 
@@ -752,6 +752,25 @@ lookup_internalvar (char *name)
     if (strcmp (var->name, name) == 0)
       return var;
 
+  return NULL;
+}
+
+
+/* Look up an internal variable with name NAME.  NAME should not
+   normally include a dollar sign.
+
+   If the specified internal variable does not exist,
+   one is created, with a void value.  */
+
+struct internalvar *
+lookup_internalvar (char *name)
+{
+  struct internalvar *var;
+
+  var = lookup_only_internalvar (name);
+  if (var)
+    return var;
+
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
--- gdb/parse.c
+++ gdb/parse.c
@@ -486,6 +486,17 @@ write_dollar_variable (struct stoken str
   if (i >= 0)
     goto handle_register;
 
+  /* Any names starting with $ are probably debugger internal variables.  */
+
+  if (str.ptr[0] == '$' && lookup_only_internalvar (copy_name (str) + 1))
+    {
+internal_lookup:
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      return;
+    }
+
   /* On some systems, such as HP-UX and hppa-linux, certain system routines 
      have names beginning with $ or $$.  Check for those, first. */
 
@@ -508,12 +519,9 @@ write_dollar_variable (struct stoken str
       return;
     }
 
-  /* Any other names starting in $ are debugger internal variables.  */
-
-  write_exp_elt_opcode (OP_INTERNALVAR);
-  write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
-  write_exp_elt_opcode (OP_INTERNALVAR);
-  return;
+  /* Any other names are assumed to be debugger internal variables.  */
+
+  goto internal_lookup;
 handle_last:
   write_exp_elt_opcode (OP_LAST);
   write_exp_elt_longcst ((LONGEST) i);

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Daniel Jacobowitz]

Sorry, I'm way behind on patches, and it seems others don't have
much time for review either.

On Thu, Sep 14, 2006 at 01:00:54AM -0400, Mike Frysinger wrote:
> 2006-08-21  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* value.h (lookup_only_internalvar): New prototype.
> 	* value.c (lookup_only_internalvar): New function.
> 	(lookup_internalvar): Use lookup_only_internalvar.
> 	* parse.c (write_dollar_variable): Look up $ symbols in internal
> 	table first rather than last.

I think this approach is basically OK.  Minor comments on the
patch:

> --- gdb/parse.c
> +++ gdb/parse.c
> @@ -486,6 +486,17 @@ write_dollar_variable (struct stoken str
>    if (i >= 0)
>      goto handle_register;
>  
> +  /* Any names starting with $ are probably debugger internal variables.  */
> +
> +  if (str.ptr[0] == '$' && lookup_only_internalvar (copy_name (str) + 1))
> +    {
> +internal_lookup:
> +      write_exp_elt_opcode (OP_INTERNALVAR);
> +      write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
> +      write_exp_elt_opcode (OP_INTERNALVAR);
> +      return;
> +    }

You don't need to check ptr[0]; this is write_dollar_variable, if it
didn't start with a dollar sign we wouldn't be here :-)

Scanning the list of internal variables is linear.  Let's not be
wasteful with it; please save the result the first time you do it.

> @@ -508,12 +519,9 @@ write_dollar_variable (struct stoken str
>        return;
>      }
>  
> -  /* Any other names starting in $ are debugger internal variables.  */
> -
> -  write_exp_elt_opcode (OP_INTERNALVAR);
> -  write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
> -  write_exp_elt_opcode (OP_INTERNALVAR);
> -  return;
> +  /* Any other names are assumed to be debugger internal variables.  */
> +
> +  goto internal_lookup;

Let's avoid goto, please, since it's only three lines.  We're going
to rescan the internal variable list here, but we don't much care.

Would you mind resubmitting with those changes?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Mike Frysinger]

[-- Attachment #1.1: Type: text/plain, Size: 1189 bytes --]

On Saturday 16 September 2006 00:02, Daniel Jacobowitz wrote:
> Sorry, I'm way behind on patches, and it seems others don't have
> much time for review either.

no problem ... i just wasnt sure if it was a "people are overloaded and havent 
gotten a chance to review" or "people have reviewed just havent posted an 
opinion one way or the other"

> Scanning the list of internal variables is linear.  Let's not be
> wasteful with it; please save the result the first time you do it.

indeed ... that would go from 3 internal linear searches to 2 as the final 
case would be calling internal_lookup() ...

if we break up the the lookup_internal() up completely, splitting the creation 
of the variable into say "create_internal()", we could kill off the last 
linear search

> Would you mind resubmitting with those changes?

sure, ive attached two versions ... the first is just the previous patch with 
your requested changes while the second is that plus breaking up the 
internal_lookup() function into lookup_only_internal() and 
create_internal() ... doesnt matter to me which of these patches are accepted 
as they both accomplish my original goal :)
-mike

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: gdb-lookup-internal-first-2.patch --]
[-- Type: text/x-diff, Size: 2992 bytes --]

2006-08-21  Mike Frysinger  <vapier@gentoo.org>

	* value.h (lookup_only_internalvar): New prototype.
	* value.c (lookup_only_internalvar): New function.
	(lookup_internalvar): Use lookup_only_internalvar.
	* parse.c (write_dollar_variable): Look up $ symbols in internal
	table first rather than last.

--- gdb/value.h
+++ gdb/value.h
@@ -419,6 +419,8 @@ extern void set_internalvar_component (s
 				       int bitpos, int bitsize,
 				       struct value *newvalue);
 
+extern struct internalvar *lookup_only_internalvar (char *name);
+
 extern struct internalvar *lookup_internalvar (char *name);
 
 extern int value_equal (struct value *arg1, struct value *arg2);
--- gdb/value.c
+++ gdb/value.c
@@ -741,10 +741,10 @@ init_if_undefined_command (char* args, i
    normally include a dollar sign.
 
    If the specified internal variable does not exist,
-   one is created, with a void value.  */
+   the return value is NULL.  */
 
 struct internalvar *
-lookup_internalvar (char *name)
+lookup_only_internalvar (char *name)
 {
   struct internalvar *var;
 
@@ -752,6 +752,25 @@ lookup_internalvar (char *name)
     if (strcmp (var->name, name) == 0)
       return var;
 
+  return NULL;
+}
+
+
+/* Look up an internal variable with name NAME.  NAME should not
+   normally include a dollar sign.
+
+   If the specified internal variable does not exist,
+   one is created, with a void value.  */
+
+struct internalvar *
+lookup_internalvar (char *name)
+{
+  struct internalvar *var;
+
+  var = lookup_only_internalvar (name);
+  if (var)
+    return var;
+
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
--- gdb/parse.c
+++ gdb/parse.c
@@ -448,6 +448,7 @@ write_dollar_variable (struct stoken str
 {
   struct symbol *sym = NULL;
   struct minimal_symbol *msym = NULL;
+  struct internalvar *isym = NULL;
 
   /* Handle the tokens $digits; also $ (short for $0) and $$ (short for $$1)
      and $$digits (equivalent to $<-digits> if you could type that). */
@@ -486,6 +487,17 @@ write_dollar_variable (struct stoken str
   if (i >= 0)
     goto handle_register;
 
+  /* Any names starting with $ are probably debugger internal variables.  */
+
+  isym = lookup_only_internalvar (copy_name (str) + 1);
+  if (isym)
+    {
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      write_exp_elt_intern (isym);
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      return;
+    }
+
   /* On some systems, such as HP-UX and hppa-linux, certain system routines 
      have names beginning with $ or $$.  Check for those, first. */
 
@@ -508,7 +520,7 @@ write_dollar_variable (struct stoken str
       return;
     }
 
-  /* Any other names starting in $ are debugger internal variables.  */
+  /* Any other names are assumed to be debugger internal variables.  */
 
   write_exp_elt_opcode (OP_INTERNALVAR);
   write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));

[-- Attachment #3: gdb-lookup-internal-first-3.patch --]
[-- Type: text/x-diff, Size: 3695 bytes --]

2006-08-21  Mike Frysinger  <vapier@gentoo.org>

	* value.h (lookup_only_internalvar): New prototype.
	(create_internalvar): Likewise.
	* value.c (lookup_only_internalvar): New function.
	(create_internalvar): Likewise.
	(lookup_internalvar): Use new lookup_only_internalvar and
	create_internalvar functions.
	* parse.c (write_dollar_variable): Look up $ symbols in internal
	table first rather than last.

--- gdb/value.h
+++ gdb/value.h
@@ -419,6 +419,10 @@ extern void set_internalvar_component (s
 				       int bitpos, int bitsize,
 				       struct value *newvalue);
 
+extern struct internalvar *lookup_only_internalvar (char *name);
+
+extern struct internalvar *create_internalvar (char *name);
+
 extern struct internalvar *lookup_internalvar (char *name);
 
 extern int value_equal (struct value *arg1, struct value *arg2);
--- gdb/value.c
+++ gdb/value.c
@@ -741,10 +741,10 @@ init_if_undefined_command (char* args, i
    normally include a dollar sign.
 
    If the specified internal variable does not exist,
-   one is created, with a void value.  */
+   the return value is NULL.  */
 
 struct internalvar *
-lookup_internalvar (char *name)
+lookup_only_internalvar (char *name)
 {
   struct internalvar *var;
 
@@ -752,6 +752,17 @@ lookup_internalvar (char *name)
     if (strcmp (var->name, name) == 0)
       return var;
 
+  return NULL;
+}
+
+
+/* Create an internal variable with name NAME and with a void value.
+   NAME should not normally include a dollar sign.  */
+
+struct internalvar *
+create_internalvar (char *name)
+{
+  struct internalvar *var;
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
@@ -762,6 +773,25 @@ lookup_internalvar (char *name)
   return var;
 }
 
+
+/* Look up an internal variable with name NAME.  NAME should not
+   normally include a dollar sign.
+
+   If the specified internal variable does not exist,
+   one is created, with a void value.  */
+
+struct internalvar *
+lookup_internalvar (char *name)
+{
+  struct internalvar *var;
+
+  var = lookup_only_internalvar (name);
+  if (var)
+    return var;
+
+  return create_internalvar (name);
+}
+
 struct value *
 value_of_internalvar (struct internalvar *var)
 {
--- gdb/parse.c
+++ gdb/parse.c
@@ -448,6 +448,7 @@ write_dollar_variable (struct stoken str
 {
   struct symbol *sym = NULL;
   struct minimal_symbol *msym = NULL;
+  struct internalvar *isym = NULL;
 
   /* Handle the tokens $digits; also $ (short for $0) and $$ (short for $$1)
      and $$digits (equivalent to $<-digits> if you could type that). */
@@ -486,6 +487,17 @@ write_dollar_variable (struct stoken str
   if (i >= 0)
     goto handle_register;
 
+  /* Any names starting with $ are probably debugger internal variables.  */
+
+  isym = lookup_only_internalvar (copy_name (str) + 1);
+  if (isym)
+    {
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      write_exp_elt_intern (isym);
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      return;
+    }
+
   /* On some systems, such as HP-UX and hppa-linux, certain system routines 
      have names beginning with $ or $$.  Check for those, first. */
 
@@ -508,10 +520,10 @@ write_dollar_variable (struct stoken str
       return;
     }
 
-  /* Any other names starting in $ are debugger internal variables.  */
+  /* Any other names are assumed to be debugger internal variables.  */
 
   write_exp_elt_opcode (OP_INTERNALVAR);
-  write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
+  write_exp_elt_intern (create_internalvar (copy_name (str) + 1));
   write_exp_elt_opcode (OP_INTERNALVAR);
   return;
 handle_last:

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Daniel Jacobowitz]

On Sun, Sep 17, 2006 at 12:22:08AM -0400, Mike Frysinger wrote:
> > Would you mind resubmitting with those changes?
> 
> sure, ive attached two versions ... the first is just the previous patch with 
> your requested changes while the second is that plus breaking up the 
> internal_lookup() function into lookup_only_internal() and 
> create_internal() ... doesnt matter to me which of these patches are accepted 
> as they both accomplish my original goal :)

Nice.  The second one is OK.  There's one small problem; I don't think
we get to call a patch of this size trivial, and I don't see a
copyright assignment for you on file.  Am I missing one?  If not, I
think I have to ask you to put one in place.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

On Sunday 17 September 2006 11:10, Daniel Jacobowitz wrote:
> On Sun, Sep 17, 2006 at 12:22:08AM -0400, Mike Frysinger wrote:
> > > Would you mind resubmitting with those changes?
> >
> > sure, ive attached two versions ... the first is just the previous patch
> > with your requested changes while the second is that plus breaking up the
> > internal_lookup() function into lookup_only_internal() and
> > create_internal() ... doesnt matter to me which of these patches are
> > accepted as they both accomplish my original goal :)
>
> Nice.  The second one is OK.  There's one small problem; I don't think
> we get to call a patch of this size trivial, and I don't see a
> copyright assignment for you on file.  Am I missing one?  If not, I
> think I have to ask you to put one in place.

Analog Devices has a blank copyright assignment in place for more of the FSF 
projects; not sure if we've submitted the GDB one yet ... so see if you have 
one on file for ADI because that covers me :)
-mike

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Daniel Jacobowitz]

On Sun, Sep 17, 2006 at 03:19:54PM -0400, Mike Frysinger wrote:
> Analog Devices has a blank copyright assignment in place for more of the FSF 
> projects; not sure if we've submitted the GDB one yet ... so see if you have 
> one on file for ADI because that covers me :)

No, there is no GDB assignment on file, only binutils/gcc/libstdc++.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

On Sunday 17 September 2006 11:10, Daniel Jacobowitz wrote:
> On Sun, Sep 17, 2006 at 12:22:08AM -0400, Mike Frysinger wrote:
> > > Would you mind resubmitting with those changes?
> >
> > sure, ive attached two versions ... the first is just the previous patch
> > with your requested changes while the second is that plus breaking up the
> > internal_lookup() function into lookup_only_internal() and
> > create_internal() ... doesnt matter to me which of these patches are
> > accepted as they both accomplish my original goal :)
>
> Nice.  The second one is OK.  There's one small problem; I don't think
> we get to call a patch of this size trivial, and I don't see a
> copyright assignment for you on file.  Am I missing one?  If not, I
> think I have to ask you to put one in place.

looks like one of my mail systems ate your reply :/

seems the ADI lawyers are taking their sweet time with the GDB assignment so 
i'll do another patch ping when things are in place

what a pita ;)
-mike

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Mike Frysinger]

[-- Attachment #1.1: Type: text/plain, Size: 979 bytes --]

On Sunday 17 September 2006, Daniel Jacobowitz wrote:
> On Sun, Sep 17, 2006 at 12:22:08AM -0400, Mike Frysinger wrote:
> > > Would you mind resubmitting with those changes?
> >
> > sure, ive attached two versions ... the first is just the previous patch
> > with your requested changes while the second is that plus breaking up the
> > internal_lookup() function into lookup_only_internal() and
> > create_internal() ... doesnt matter to me which of these patches are
> > accepted as they both accomplish my original goal :)
>
> Nice.  The second one is OK.  There's one small problem; I don't think
> we get to call a patch of this size trivial, and I don't see a
> copyright assignment for you on file.  Am I missing one?  If not, I
> think I have to ask you to put one in place.

looks like we finally have an agreement in place with the FSF ... but since 
it's been almost a year (!), i'll just re-attach the patch re-diffed against 
current cvs :)
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: gdb-lookup-internal-first-4.patch --]
[-- Type: text/x-diff, Size: 3695 bytes --]

2007-09-26  Mike Frysinger  <vapier@gentoo.org>

	* value.h (lookup_only_internalvar): New prototype.
	(create_internalvar): Likewise.
	* value.c (lookup_only_internalvar): New function.
	(create_internalvar): Likewise.
	(lookup_internalvar): Use new lookup_only_internalvar and
	create_internalvar functions.
	* parse.c (write_dollar_variable): Look up $ symbols in internal
	table first rather than last.

--- gdb/value.h
+++ gdb/value.h
@@ -434,6 +434,10 @@ extern void set_internalvar_component (s
 				       int bitpos, int bitsize,
 				       struct value *newvalue);
 
+extern struct internalvar *lookup_only_internalvar (char *name);
+
+extern struct internalvar *create_internalvar (char *name);
+
 extern struct internalvar *lookup_internalvar (char *name);
 
 extern int value_equal (struct value *arg1, struct value *arg2);
--- gdb/value.c
+++ gdb/value.c
@@ -742,10 +742,10 @@ init_if_undefined_command (char* args, i
    normally include a dollar sign.
 
    If the specified internal variable does not exist,
-   one is created, with a void value.  */
+   the return value is NULL.  */
 
 struct internalvar *
-lookup_internalvar (char *name)
+lookup_only_internalvar (char *name)
 {
   struct internalvar *var;
 
@@ -753,6 +753,17 @@ lookup_internalvar (char *name)
     if (strcmp (var->name, name) == 0)
       return var;
 
+  return NULL;
+}
+
+
+/* Create an internal variable with name NAME and with a void value.
+   NAME should not normally include a dollar sign.  */
+
+struct internalvar *
+create_internalvar (char *name)
+{
+  struct internalvar *var;
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
@@ -763,6 +774,25 @@ lookup_internalvar (char *name)
   return var;
 }
 
+
+/* Look up an internal variable with name NAME.  NAME should not
+   normally include a dollar sign.
+
+   If the specified internal variable does not exist,
+   one is created, with a void value.  */
+
+struct internalvar *
+lookup_internalvar (char *name)
+{
+  struct internalvar *var;
+
+  var = lookup_only_internalvar (name);
+  if (var)
+    return var;
+
+  return create_internalvar (name);
+}
+
 struct value *
 value_of_internalvar (struct internalvar *var)
 {
--- gdb/parse.c
+++ gdb/parse.c
@@ -469,6 +469,7 @@ write_dollar_variable (struct stoken str
 {
   struct symbol *sym = NULL;
   struct minimal_symbol *msym = NULL;
+  struct internalvar *isym = NULL;
 
   /* Handle the tokens $digits; also $ (short for $0) and $$ (short for $$1)
      and $$digits (equivalent to $<-digits> if you could type that). */
@@ -507,6 +508,17 @@ write_dollar_variable (struct stoken str
   if (i >= 0)
     goto handle_register;
 
+  /* Any names starting with $ are probably debugger internal variables.  */
+
+  isym = lookup_only_internalvar (copy_name (str) + 1);
+  if (isym)
+    {
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      write_exp_elt_intern (isym);
+      write_exp_elt_opcode (OP_INTERNALVAR);
+      return;
+    }
+
   /* On some systems, such as HP-UX and hppa-linux, certain system routines 
      have names beginning with $ or $$.  Check for those, first. */
 
@@ -529,10 +541,10 @@ write_dollar_variable (struct stoken str
       return;
     }
 
-  /* Any other names starting in $ are debugger internal variables.  */
+  /* Any other names are assumed to be debugger internal variables.  */
 
   write_exp_elt_opcode (OP_INTERNALVAR);
-  write_exp_elt_intern (lookup_internalvar (copy_name (str) + 1));
+  write_exp_elt_intern (create_internalvar (copy_name (str) + 1));
   write_exp_elt_opcode (OP_INTERNALVAR);
   return;
 handle_last:

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Daniel Jacobowitz]

On Wed, Sep 26, 2007 at 03:50:14PM -0400, Mike Frysinger wrote:
> looks like we finally have an agreement in place with the FSF ... but since 
> it's been almost a year (!), i'll just re-attach the patch re-diffed against 
> current cvs :)

Thanks - it's still OK to commit :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch ping] change lookup order of $localvars to happen before symbol tables

[Mike Frysinger]

On Sunday 30 September 2007, Daniel Jacobowitz wrote:
> On Wed, Sep 26, 2007 at 03:50:14PM -0400, Mike Frysinger wrote:
> > looks like we finally have an agreement in place with the FSF ... but
> > since it's been almost a year (!), i'll just re-attach the patch
> > re-diffed against current cvs :)
>
> Thanks - it's still OK to commit :-)

ah i didnt think i had commit access ... thanks :)
-mike