[RFA] Introduce notion of "search name"

[RFA] Introduce notion of "search name"

[Paul Hilfinger]

The following patch does nothing except to prepare the way for some later
Ada modifications by providing a few "hooks". I've discussed this 
modification earlier on this newsgroup.  At the moment, I've given
the definitions placeholder definitions that simply result in the
current semantics.  I propose to change them when Ada support is turned on.

The idea is to define "search name" as "the name that a language
module uses when searching for this symbol".  As discussed earlier,
the search name would be normally be the natural name, but in the case
of Ada, would just be the linkage name.

The modification to the signature of symbol_natural_name is to allow the
option of delayed (lazy) evaluation of the demangled name, which is
actually the point of introducing search names.

Paul Hilfinger

2004-03-03  Paul N. Hilfinger  <hilfinger@gnat.com>

	* symtab.h (symbol_natural_name): Remove const qualification 
	from argument.
	(SYMBOL_SEARCH_NAME): New definition.
	(SYMBOL_MATCHES_SEARCH_NAME): New definition.
	(SYMBOL_DEMANGLED_SEARCH_NAME): New definition.	

	* dictionary.c (iter_name_first_hashed): Match on SYMBOL_SEARCH_NAME.
	(iter_name_next_hashed): Ditto.
	(iter_name_next_linear): Ditto.
	(insert_symbol_hashed): Hash on SYMBOL_SEARCH_NAME.

	* symtab.c (symbol_natural_name): Remove const qualification from
	argument to allow lazy evaluation of demangled name.
	(lookup_partial_symbol): Assume symbols ordered by search name, using
	SYMBOL_SEARCH_NAME and SYMBOL_MATCHES_SEARCH_NAME.
	
	* symfile.c (compare_psymbols): Order by SYMBOL_SEARCH_NAME.
	
	* minsyms.c (build_minimal_symbol_hash_tables): Use 
	SYMBOL_DEMANGLED_SEARCH_NAME to test for adding to demangled
	hash table.
	

Index: current-public.49/gdb/minsyms.c
--- current-public.49/gdb/minsyms.c Mon, 09 Feb 2004 23:40:20 -0800 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2 644)
+++ submit.31(w)/gdb/minsyms.c Wed, 03 Mar 2004 00:34:32 -0800 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.1.1 644)
@@ -794,7 +794,7 @@ build_minimal_symbol_hash_tables (struct
       add_minsym_to_hash_table (msym, objfile->msymbol_hash);
 
       msym->demangled_hash_next = 0;
-      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
+      if (SYMBOL_DEMANGLED_SEARCH_NAME (msym) != NULL)
 	add_minsym_to_demangled_hash_table (msym,
                                             objfile->msymbol_demangled_hash);
     }
Index: current-public.49/gdb/symfile.c
--- current-public.49/gdb/symfile.c Tue, 02 Mar 2004 23:57:04 -0800 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1 644)
+++ submit.31(w)/gdb/symfile.c Wed, 03 Mar 2004 00:34:32 -0800 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1.1.1 644)
@@ -208,8 +208,8 @@ compare_psymbols (const void *s1p, const
   struct partial_symbol *const *s1 = s1p;
   struct partial_symbol *const *s2 = s2p;
 
-  return strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*s1),
-			    SYMBOL_NATURAL_NAME (*s2));
+  return strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*s1),
+			    SYMBOL_SEARCH_NAME (*s2));
 }
 
 void
Index: current-public.49/gdb/symtab.c
--- current-public.49/gdb/symtab.c Mon, 23 Feb 2004 00:44:25 -0800 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1 644)
+++ submit.31(w)/gdb/symtab.c Wed, 03 Mar 2004 01:55:44 -0800 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1.1.1 644)
@@ -630,7 +630,7 @@ symbol_init_demangled_name (struct gener
    demangling is necessary, this is the demangled name.  */
 
 char *
-symbol_natural_name (const struct general_symbol_info *gsymbol)
+symbol_natural_name (struct general_symbol_info *gsymbol)
 {
   if ((gsymbol->language == language_cplus
        || gsymbol->language == language_java
@@ -1467,7 +1467,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -1482,7 +1482,7 @@ lookup_partial_symbol (struct partial_sy
       while (top <= real_top
 	     && (linkage_name != NULL
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
-		 : SYMBOL_MATCHES_NATURAL_NAME (*top,name)))
+		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
 	  if (SYMBOL_DOMAIN (*top) == domain)
 	    {
@@ -1503,7 +1503,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
-		  : SYMBOL_MATCHES_NATURAL_NAME (*psym, name))
+		  : SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
 		{
 		  return (*psym);
 		}
Index: current-public.49/gdb/symtab.h
--- current-public.49/gdb/symtab.h Wed, 18 Feb 2004 01:34:45 -0800 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1 644)
+++ submit.31(w)/gdb/symtab.h Wed, 03 Mar 2004 01:55:59 -0800 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1.1.1 644)
@@ -213,7 +213,7 @@ extern void symbol_set_names (struct gen
 
 #define SYMBOL_NATURAL_NAME(symbol) \
   (symbol_natural_name (&(symbol)->ginfo))
-extern char *symbol_natural_name (const struct general_symbol_info *symbol);
+extern char *symbol_natural_name (struct general_symbol_info *symbol);
 
 /* Return SYMBOL's name from the point of view of the linker.  In
    languages like C++ where symbols may be mangled for ease of
@@ -257,6 +257,22 @@ extern char *symbol_demangled_name (stru
 
 #define SYMBOL_MATCHES_NATURAL_NAME(symbol, name)			\
   (strcmp_iw (SYMBOL_NATURAL_NAME (symbol), (name)) == 0)
+
+/* Macro that returns the name to be used when sorting and searching symbols. 
+   In most languages, we search for the demangled form of a name,
+   and so sort symbols accordingly. */
+#define SYMBOL_SEARCH_NAME(symbol)					 \
+  SYMBOL_NATURAL_NAME (symbol)
+
+/* Analogous to SYMBOL_MATCHES_NATURAL_NAME, but uses the search
+   name. */
+#define SYMBOL_MATCHES_SEARCH_NAME(symbol, name)			\
+  (strcmp_iw (SYMBOL_SEARCH_NAME (symbol), (name)) == 0)
+
+/* For languages that do not demangle or that do not do searches by 
+   demangled name, NULL.  Otherwise, the demangled name. */
+#define SYMBOL_DEMANGLED_SEARCH_NAME(symbol)				 \
+  SYMBOL_DEMANGLED_NAME (symbol)
 
 /* Classification types for a minimal symbol.  These should be taken as
    "advisory only", since if gdb can't easily figure out a
Index: current-public.49/gdb/dictionary.c
--- current-public.49/gdb/dictionary.c Tue, 17 Jun 2003 02:41:56 -0700 hilfingr (GdbPub/L/b/35_dictionary 1.1 644)
+++ submit.31(w)/gdb/dictionary.c Wed, 03 Mar 2004 00:34:33 -0800 hilfingr (GdbPub/L/b/35_dictionary 1.1.4.1 644)
@@ -636,7 +636,7 @@ iter_name_first_hashed (const struct dic
        sym = sym->hash_next)
     {
       /* Warning: the order of arguments to strcmp_iw matters!  */
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  break;
 	}
@@ -656,7 +656,7 @@ iter_name_next_hashed (const char *name,
        next != NULL;
        next = next->hash_next)
     {
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (next), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (next), name) == 0)
 	break;
     }
 
@@ -674,7 +674,7 @@ insert_symbol_hashed (struct dictionary 
   unsigned int hash_index;
   struct symbol **buckets = DICT_HASHED_BUCKETS (dict);
 
-  hash_index = (msymbol_hash_iw (SYMBOL_NATURAL_NAME (sym))
+  hash_index = (msymbol_hash_iw (SYMBOL_SEARCH_NAME (sym))
 		% DICT_HASHED_NBUCKETS (dict));
   sym->hash_next = buckets[hash_index];
   buckets[hash_index] = sym;
@@ -789,7 +789,7 @@ iter_name_next_linear (const char *name,
   for (i = DICT_ITERATOR_INDEX (iterator) + 1; i < nsyms; ++i)
     {
       sym = DICT_LINEAR_SYM (dict, i);
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  retval = sym;
 	  break;

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 02:15:50PM -0500, Paul Hilfinger wrote:
> 
> The following patch does nothing except to prepare the way for some later
> Ada modifications by providing a few "hooks". I've discussed this 
> modification earlier on this newsgroup.  At the moment, I've given
> the definitions placeholder definitions that simply result in the
> current semantics.  I propose to change them when Ada support is turned on.
> 
> The idea is to define "search name" as "the name that a language
> module uses when searching for this symbol".  As discussed earlier,
> the search name would be normally be the natural name, but in the case
> of Ada, would just be the linkage name.
> 
> The modification to the signature of symbol_natural_name is to allow the
> option of delayed (lazy) evaluation of the demangled name, which is
> actually the point of introducing search names.

I'm no symtab maintainer, but I've had my hands in this code lately, so
I have a couple of comments anyway.  There are some of things I don't
like about this patch, unfortunately.


It doesn't address on of the thornier problems I hit when doing the
same thing, namely that of allocation.  OK, someone uses
SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
objfile is not available.  I think there may be no option but to
pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?


You define SYMBOL_DEMANGLED_SEARCH_NAME.  What's it really good for,
and how does it do any good?  You only use it for the minimal symbol
hash tables; the fundamental problem with minimal symbols is that we
don't know their language, so I don't know how you can reliably make a
language-specific decision like this one.

SYMBOL_DEMANGLED_SEARCH_NAME also codifies more than necessary of the
difference between the other SYMBOL_*_NAME macros and
SYMBOL_SEARCH_NAME.  Something that I think may be useful is to use
just the basename for the search name and then have language-specific
code to cherry-pick the resulting matches afterwards; one big advantage
of this is that it lets me sidestep the Java vs. C++ demangling issues.
I suspect it is possible (for all supported languages) to unambiguously
and efficiently identify the basename. I need to look at some of the
other in-use manglings to follow up on that idea though, particularly
g++ v2 and ObjC.

This idea may only be workable for minimal symbols, because of the
namespace inference hacks we use for C++.


Oh, and two spaces after a full stop in comments.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 02:15:50PM -0500, Paul Hilfinger wrote:
> 
> The following patch does nothing except to prepare the way for some later
> Ada modifications by providing a few "hooks". I've discussed this 
> modification earlier on this newsgroup.  At the moment, I've given
> the definitions placeholder definitions that simply result in the
> current semantics.  I propose to change them when Ada support is turned on.
> 
> The idea is to define "search name" as "the name that a language
> module uses when searching for this symbol".  As discussed earlier,
> the search name would be normally be the natural name, but in the case
> of Ada, would just be the linkage name.
> 
> The modification to the signature of symbol_natural_name is to allow the
> option of delayed (lazy) evaluation of the demangled name, which is
> actually the point of introducing search names.

I'm no symtab maintainer, but I've had my hands in this code lately, so
I have a couple of comments anyway.  There are some of things I don't
like about this patch, unfortunately.


It doesn't address on of the thornier problems I hit when doing the
same thing, namely that of allocation.  OK, someone uses
SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
objfile is not available.  I think there may be no option but to
pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?


You define SYMBOL_DEMANGLED_SEARCH_NAME.  What's it really good for,
and how does it do any good?  You only use it for the minimal symbol
hash tables; the fundamental problem with minimal symbols is that we
don't know their language, so I don't know how you can reliably make a
language-specific decision like this one.

SYMBOL_DEMANGLED_SEARCH_NAME also codifies more than necessary of the
difference between the other SYMBOL_*_NAME macros and
SYMBOL_SEARCH_NAME.  Something that I think may be useful is to use
just the basename for the search name and then have language-specific
code to cherry-pick the resulting matches afterwards; one big advantage
of this is that it lets me sidestep the Java vs. C++ demangling issues.
I suspect it is possible (for all supported languages) to unambiguously
and efficiently identify the basename. I need to look at some of the
other in-use manglings to follow up on that idea though, particularly
g++ v2 and ObjC.

This idea may only be workable for minimal symbols, because of the
namespace inference hacks we use for C++.


Oh, and two spaces after a full stop in comments.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

Daniel,

> It doesn't address on of the thornier problems I hit when doing the
> same thing, namely that of allocation.  OK, someone uses
> SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
> objfile is not available.  I think there may be no option but to
> pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?

You're right, I did not address this in the patch proper.  I had
prepared a patch in which I used that extra byte in struct symtab to
tag the union and allow an objfile member.  However, I was aware from
correspondence with you that you were working in this area, and that
some of what you proposed to do might eventually allow us to re-do Ada
symbol lookup.  So I decided not to modify the symtab struct for the
moment, and instead submit a patch that would change as little as
possible.  I figured it would be better not to do anything just now
that might interfere with on-going work on the symbol table.

So as an interim measure, I use your suggestion of 21 Jan and first
try to find an objfile via the BFD section.  When that doesn't work, I
simply use a global hashtable to hold the demangled strings.  Yes,
that is a memory leak, but on consideration, I realized that it's only
REALLY a memory leak if (a) I routinely change the entire set of
demangled names numerous times during a single GDB session, or (b)
demangle entirely different, large sets of names each time I reload
the symbol tables.  Yeah, I know, it's not pretty, but again I am hoping
it will ensure that demangled names behave until the next interation of
symtab modifications allow an entirely different strategy.

> You define SYMBOL_DEMANGLED_SEARCH_NAME.  What's it really good for,
> and how does it do any good?  You only use it for the minimal symbol
> hash tables; the fundamental problem with minimal symbols is that we
> don't know their language, so I don't know how you can reliably make a
> language-specific decision like this one.

The relevant code now reads

      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
	add_minsym_to_demangled_hash_table (msym,
                                            objfile->msymbol_demangled_hash);

Ada does demangle; SYMBOL_DEMANGLED_NAME does have to return a
demangled name, if there is one.  Therefore the test here will
precipitate computing and caching the demangled name prematurely (once
symbol_demangled_name is extended to include the Ada case).  This code
also adds the demangled name to the hash table.  But we never look for
demangled names, so that is a waste.

As to your question about how this can work: Ada doesn't really change
your question.  I could just as well ask "How can
SYMBOL_DEMANGLED_NAME work on minimal symbols, given that it doesn't
know what language to use for demangling?" The answer is that if it
quacks like a duck ... excuse me, I mean if ObjC demangling works,
assume you have an ObjC symbol, if C++ demangling works, then assume
it is a C++ symbol, etc., and hope that the demangling schemes don't
collide.  That's what the code says now.  You'll have to argue the 
sensibility of this strategy with others.

> SYMBOL_DEMANGLED_SEARCH_NAME also codifies more than necessary of the
> difference between the other SYMBOL_*_NAME macros and
> SYMBOL_SEARCH_NAME.  Something that I think may be useful is to use
> just the basename for the search name and then have language-specific
> code to cherry-pick the resulting matches afterwards; one big advantage
> of this is that it lets me sidestep the Java vs. C++ demangling issues.
> I suspect it is possible (for all supported languages) to unambiguously
> and efficiently identify the basename. I need to look at some of the
> other in-use manglings to follow up on that idea though, particularly
> g++ v2 and ObjC.

Yes, I know: you discussed that before and I eagerly await these changes.
For the nonce, I still think I've found a reasonably small hook that 
accomplishes our purposes.

> Oh, and two spaces after a full stop in comments.

Oh, woe: of all your comments, this will be the most difficult to
accommodate.  You see, ACT is full of these confounded Europeans who
insist on single spaces after periods and yell whenever I do things
properly instead.  No doubt they will relent when they hear how this
practice will slow up these patches.  :->).

Paul Hilfinger
Ada Core Technologies, Inc.

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

Daniel,

> It doesn't address on of the thornier problems I hit when doing the
> same thing, namely that of allocation.  OK, someone uses
> SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
> objfile is not available.  I think there may be no option but to
> pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?

You're right, I did not address this in the patch proper.  I had
prepared a patch in which I used that extra byte in struct symtab to
tag the union and allow an objfile member.  However, I was aware from
correspondence with you that you were working in this area, and that
some of what you proposed to do might eventually allow us to re-do Ada
symbol lookup.  So I decided not to modify the symtab struct for the
moment, and instead submit a patch that would change as little as
possible.  I figured it would be better not to do anything just now
that might interfere with on-going work on the symbol table.

So as an interim measure, I use your suggestion of 21 Jan and first
try to find an objfile via the BFD section.  When that doesn't work, I
simply use a global hashtable to hold the demangled strings.  Yes,
that is a memory leak, but on consideration, I realized that it's only
REALLY a memory leak if (a) I routinely change the entire set of
demangled names numerous times during a single GDB session, or (b)
demangle entirely different, large sets of names each time I reload
the symbol tables.  Yeah, I know, it's not pretty, but again I am hoping
it will ensure that demangled names behave until the next interation of
symtab modifications allow an entirely different strategy.

> You define SYMBOL_DEMANGLED_SEARCH_NAME.  What's it really good for,
> and how does it do any good?  You only use it for the minimal symbol
> hash tables; the fundamental problem with minimal symbols is that we
> don't know their language, so I don't know how you can reliably make a
> language-specific decision like this one.

The relevant code now reads

      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
	add_minsym_to_demangled_hash_table (msym,
                                            objfile->msymbol_demangled_hash);

Ada does demangle; SYMBOL_DEMANGLED_NAME does have to return a
demangled name, if there is one.  Therefore the test here will
precipitate computing and caching the demangled name prematurely (once
symbol_demangled_name is extended to include the Ada case).  This code
also adds the demangled name to the hash table.  But we never look for
demangled names, so that is a waste.

As to your question about how this can work: Ada doesn't really change
your question.  I could just as well ask "How can
SYMBOL_DEMANGLED_NAME work on minimal symbols, given that it doesn't
know what language to use for demangling?" The answer is that if it
quacks like a duck ... excuse me, I mean if ObjC demangling works,
assume you have an ObjC symbol, if C++ demangling works, then assume
it is a C++ symbol, etc., and hope that the demangling schemes don't
collide.  That's what the code says now.  You'll have to argue the 
sensibility of this strategy with others.

> SYMBOL_DEMANGLED_SEARCH_NAME also codifies more than necessary of the
> difference between the other SYMBOL_*_NAME macros and
> SYMBOL_SEARCH_NAME.  Something that I think may be useful is to use
> just the basename for the search name and then have language-specific
> code to cherry-pick the resulting matches afterwards; one big advantage
> of this is that it lets me sidestep the Java vs. C++ demangling issues.
> I suspect it is possible (for all supported languages) to unambiguously
> and efficiently identify the basename. I need to look at some of the
> other in-use manglings to follow up on that idea though, particularly
> g++ v2 and ObjC.

Yes, I know: you discussed that before and I eagerly await these changes.
For the nonce, I still think I've found a reasonably small hook that 
accomplishes our purposes.

> Oh, and two spaces after a full stop in comments.

Oh, woe: of all your comments, this will be the most difficult to
accommodate.  You see, ACT is full of these confounded Europeans who
insist on single spaces after periods and yell whenever I do things
properly instead.  No doubt they will relent when they hear how this
practice will slow up these patches.  :->).

Paul Hilfinger
Ada Core Technologies, Inc.

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Fri, Mar 05, 2004 at 05:39:25AM -0500, Paul Hilfinger wrote:
> 
> Daniel,
> 
> > It doesn't address on of the thornier problems I hit when doing the
> > same thing, namely that of allocation.  OK, someone uses
> > SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
> > objfile is not available.  I think there may be no option but to
> > pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?
> 
> You're right, I did not address this in the patch proper.  I had
> prepared a patch in which I used that extra byte in struct symtab to
> tag the union and allow an objfile member.  However, I was aware from
> correspondence with you that you were working in this area, and that
> some of what you proposed to do might eventually allow us to re-do Ada
> symbol lookup.  So I decided not to modify the symtab struct for the
> moment, and instead submit a patch that would change as little as
> possible.  I figured it would be better not to do anything just now
> that might interfere with on-going work on the symbol table.
> 
> So as an interim measure, I use your suggestion of 21 Jan and first
> try to find an objfile via the BFD section.  When that doesn't work, I
> simply use a global hashtable to hold the demangled strings.  Yes,
> that is a memory leak, but on consideration, I realized that it's only
> REALLY a memory leak if (a) I routinely change the entire set of
> demangled names numerous times during a single GDB session, or (b)
> demangle entirely different, large sets of names each time I reload
> the symbol tables.  Yeah, I know, it's not pretty, but again I am hoping
> it will ensure that demangled names behave until the next interation of
> symtab modifications allow an entirely different strategy.

I'm not sure what others will think of this interim measure.  I don't
like it much, though.

> > You define SYMBOL_DEMANGLED_SEARCH_NAME.  What's it really good for,
> > and how does it do any good?  You only use it for the minimal symbol
> > hash tables; the fundamental problem with minimal symbols is that we
> > don't know their language, so I don't know how you can reliably make a
> > language-specific decision like this one.

What you did not explain is how this is supposed to be different from
SYMBOL_SEARCH_NAME.

> The relevant code now reads
> 
>       if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
> 	add_minsym_to_demangled_hash_table (msym,
>                                             objfile->msymbol_demangled_hash);
> 
> Ada does demangle; SYMBOL_DEMANGLED_NAME does have to return a
> demangled name, if there is one.  Therefore the test here will
> precipitate computing and caching the demangled name prematurely (once
> symbol_demangled_name is extended to include the Ada case).  This code
> also adds the demangled name to the hash table.  But we never look for
> demangled names, so that is a waste.
> 
> As to your question about how this can work: Ada doesn't really change
> your question.  I could just as well ask "How can
> SYMBOL_DEMANGLED_NAME work on minimal symbols, given that it doesn't
> know what language to use for demangling?" The answer is that if it
> quacks like a duck ... excuse me, I mean if ObjC demangling works,
> assume you have an ObjC symbol, if C++ demangling works, then assume
> it is a C++ symbol, etc., and hope that the demangling schemes don't
> collide.  That's what the code says now.  You'll have to argue the 
> sensibility of this strategy with others.

The existing ada_demangle never fails.  How does that interact with
what you said above?  Hopefully not by tagging all minsyms as Ada!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Introduce notion of "search name"

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> On Fri, Mar 05, 2004 at 05:39:25AM -0500, Paul Hilfinger wrote:
> > Daniel,
> > 
> > > It doesn't address on of the thornier problems I hit when doing the
> > > same thing, namely that of allocation.  OK, someone uses
> > > SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
> > > objfile is not available.  I think there may be no option but to
> > > pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?
> > 
> > You're right, I did not address this in the patch proper.  I had
> > prepared a patch in which I used that extra byte in struct symtab to
> > tag the union and allow an objfile member.  However, I was aware from
> > correspondence with you that you were working in this area, and that
> > some of what you proposed to do might eventually allow us to re-do Ada
> > symbol lookup.  So I decided not to modify the symtab struct for the
> > moment, and instead submit a patch that would change as little as
> > possible.  I figured it would be better not to do anything just now
> > that might interfere with on-going work on the symbol table.
> > 
> > So as an interim measure, I use your suggestion of 21 Jan and first
> > try to find an objfile via the BFD section.  When that doesn't work, I
> > simply use a global hashtable to hold the demangled strings.  Yes,
> > that is a memory leak, but on consideration, I realized that it's only
> > REALLY a memory leak if (a) I routinely change the entire set of
> > demangled names numerous times during a single GDB session, or (b)
> > demangle entirely different, large sets of names each time I reload
> > the symbol tables.  Yeah, I know, it's not pretty, but again I am hoping
> > it will ensure that demangled names behave until the next interation of
> > symtab modifications allow an entirely different strategy.
> 
> I'm not sure what others will think of this interim measure.  I don't
> like it much, though.

Under what circumstances does finding an objfile by the minsym's BFD
section not work?  That minsym must have come from somewhere.  Do we
produce minsyms whose sections are unset, for some reason?

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Thu, Apr 01, 2004 at 09:52:46AM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@false.org> writes:
> > On Fri, Mar 05, 2004 at 05:39:25AM -0500, Paul Hilfinger wrote:
> > > Daniel,
> > > 
> > > > It doesn't address on of the thornier problems I hit when doing the
> > > > same thing, namely that of allocation.  OK, someone uses
> > > > SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
> > > > objfile is not available.  I think there may be no option but to
> > > > pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?
> > > 
> > > You're right, I did not address this in the patch proper.  I had
> > > prepared a patch in which I used that extra byte in struct symtab to
> > > tag the union and allow an objfile member.  However, I was aware from
> > > correspondence with you that you were working in this area, and that
> > > some of what you proposed to do might eventually allow us to re-do Ada
> > > symbol lookup.  So I decided not to modify the symtab struct for the
> > > moment, and instead submit a patch that would change as little as
> > > possible.  I figured it would be better not to do anything just now
> > > that might interfere with on-going work on the symbol table.
> > > 
> > > So as an interim measure, I use your suggestion of 21 Jan and first
> > > try to find an objfile via the BFD section.  When that doesn't work, I
> > > simply use a global hashtable to hold the demangled strings.  Yes,
> > > that is a memory leak, but on consideration, I realized that it's only
> > > REALLY a memory leak if (a) I routinely change the entire set of
> > > demangled names numerous times during a single GDB session, or (b)
> > > demangle entirely different, large sets of names each time I reload
> > > the symbol tables.  Yeah, I know, it's not pretty, but again I am hoping
> > > it will ensure that demangled names behave until the next interation of
> > > symtab modifications allow an entirely different strategy.
> > 
> > I'm not sure what others will think of this interim measure.  I don't
> > like it much, though.
> 
> Under what circumstances does finding an objfile by the minsym's BFD
> section not work?  That minsym must have come from somewhere.  Do we
> produce minsyms whose sections are unset, for some reason?

Well, (A) it's inefficient, since there's no pointer from the BFD
section to the GDB section; (B) I don't know whether we produce minsyms
whose sections are unset; (C) I really want to remove the section
pointer from general_symbol_info someday, and this will make that
harder.  I've abandoned that project for the moment while I catch up on
other projects, but I'll be back to it :)

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Introduce notion of "search name"

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:

> On Thu, Apr 01, 2004 at 09:52:46AM -0500, Jim Blandy wrote:
> > 
> > Daniel Jacobowitz <drow@false.org> writes:
> > > On Fri, Mar 05, 2004 at 05:39:25AM -0500, Paul Hilfinger wrote:
> > > > Daniel,
> > > > 
> > > > > It doesn't address on of the thornier problems I hit when doing the
> > > > > same thing, namely that of allocation.  OK, someone uses
> > > > > SYMBOL_DEMANGLED_NAME, we lazily allocate a demangled name - where? The
> > > > > objfile is not available.  I think there may be no option but to
> > > > > pass the objfile to SYMBOL_DEMANGLED_NAME.  What did you do for Ada?
> > > > 
> > > > You're right, I did not address this in the patch proper.  I had
> > > > prepared a patch in which I used that extra byte in struct symtab to
> > > > tag the union and allow an objfile member.  However, I was aware from
> > > > correspondence with you that you were working in this area, and that
> > > > some of what you proposed to do might eventually allow us to re-do Ada
> > > > symbol lookup.  So I decided not to modify the symtab struct for the
> > > > moment, and instead submit a patch that would change as little as
> > > > possible.  I figured it would be better not to do anything just now
> > > > that might interfere with on-going work on the symbol table.
> > > > 
> > > > So as an interim measure, I use your suggestion of 21 Jan and first
> > > > try to find an objfile via the BFD section.  When that doesn't work, I
> > > > simply use a global hashtable to hold the demangled strings.  Yes,
> > > > that is a memory leak, but on consideration, I realized that it's only
> > > > REALLY a memory leak if (a) I routinely change the entire set of
> > > > demangled names numerous times during a single GDB session, or (b)
> > > > demangle entirely different, large sets of names each time I reload
> > > > the symbol tables.  Yeah, I know, it's not pretty, but again I am hoping
> > > > it will ensure that demangled names behave until the next interation of
> > > > symtab modifications allow an entirely different strategy.
> > > 
> > > I'm not sure what others will think of this interim measure.  I don't
> > > like it much, though.
> > 
> > Under what circumstances does finding an objfile by the minsym's BFD
> > section not work?  That minsym must have come from somewhere.  Do we
> > produce minsyms whose sections are unset, for some reason?
> 
> Well, (A) it's inefficient, since there's no pointer from the BFD
> section to the GDB section; (B) I don't know whether we produce minsyms
> whose sections are unset; (C) I really want to remove the section
> pointer from general_symbol_info someday, and this will make that
> harder.  I've abandoned that project for the moment while I catch up on
> other projects, but I'll be back to it :)

Yes, (A) was apparent; by asking correctness questions I didn't mean
to imply I thought efficiency was a done deal.  :) (B) is the real
question; if someone knows that we do produce such symbols, I'd love
to hear about it.  (C) is good to know, too.

I have to say, I think our allocation rules are hairy enough already.
If there is some better solution on the horizon for lazy demangling's
allocation, I am relucantant to put in complex and slow mechanisms,
even temporarily.  It's a step backwards, justified by work that isn't
even scheduled.

Paul, how critical is this to the intent of your change?  I haven't
reviewed it.

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> I have to say, I think our allocation rules are hairy enough already.
> If there is some better solution on the horizon for lazy demangling's
> allocation, I am relucantant to put in complex and slow mechanisms,
> even temporarily.  It's a step backwards, justified by work that isn't
> even scheduled.
>
> Paul, how critical is this to the intent of your change?  I haven't
> reviewed it.

Sorry, I'm not sure what that pesky pronoun "this" refers to here.
The complex allocation is the one that goes symbol->bfd
section->bfd->objfile.  It is not critical at all; I use it only on
the general principle that we try to allocate names things on
obstacks.  I have a simple fallback that just uses a hash table on
malloced symbols on the theory that the set of demangled names does
not generally change over a GDB session (unless, of course, you are in
the habit of running 'file' on many different executables in one
session or completely rewriting your program before reloading it into
GDB), so that one would gain little storage saving from using an
obstack anyway.  Using just that fallback (at least until we come up
with a better way of getting to an appropriate obstack) would simplify
my code.

Paul

Re: [RFA] Introduce notion of "search name"

[Jim Blandy]

Paul Hilfinger <hilfingr@gnat.com> writes:
> > I have to say, I think our allocation rules are hairy enough already.
> > If there is some better solution on the horizon for lazy demangling's
> > allocation, I am relucantant to put in complex and slow mechanisms,
> > even temporarily.  It's a step backwards, justified by work that isn't
> > even scheduled.
> >
> > Paul, how critical is this to the intent of your change?  I haven't
> > reviewed it.
> 
> Sorry, I'm not sure what that pesky pronoun "this" refers to here.

I meant lazily demangling the names.  I was wondering if the
demangling could be done at minsym reading time, or psymtab->symtab
conversion time, as the other languages do, when there is an
appropriate objfile available.

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> I meant lazily demangling the names.  I was wondering if the
> demangling could be done at minsym reading time, or psymtab->symtab
> conversion time, as the other languages do, when there is an

I'd really be very reluctant to do this; it vitiates the entire point
of this "search name" strategy, which was to avoid the storage
requirements of demangled names.  We adopted that strategy quite a few years
ago, due in part to the fact that ACT had customers with LOTS of symbols
and this was a way to save a good deal of symbol space.  

(Side comment: Indeed, until I started to make changes for the public tree,
the ACT version of GDB never even cached demangled names at all, but
instead demangled on the fly into a static area.  One of you (Daniel
or David) objected that this was dangerous, and that we should make
permanent copies of demangled names, which is what gave rise to this
whole caching problem.  Oddly enough, we had never enountered
difficulties as a result of the short lifetimes of our demangled symbols:
it was just a contingent property of GDB that such persistance wasn't
needed.  Nevertheless, I had to agree that it is better not to
rely on a property that is not guaranteed.) 

Do you really object to our simply making permanent copies of these
names as needed?  (I'm perfectly happy to ditch the kludgy
objfile-finding hack.) You can't expect to save any space by
reclaiming these demangled names space when you reload the symbol
file, because (as I argued before) the set of demangled names
generally doesn't change much in such cases, and we are "interning"
the demangled symbols, so that multiple computations of the same
demangled symbol don't increase space.  This particular choice of name
storage is confined to the Ada module (the symbol table module doesn't
apply free to names stored in symbols, partial symbols, or minimal
symbols, so it doesn't care if one them has a longer than necessary
lifetime).

Paul Hilfinger

Re: [RFA] Introduce notion of "search name"

[Jim Blandy]

Paul Hilfinger <hilfingr@gnat.com> writes:
> Do you really object to our simply making permanent copies of these
> names as needed?  (I'm perfectly happy to ditch the kludgy
> objfile-finding hack.) You can't expect to save any space by
> reclaiming these demangled names space when you reload the symbol
> file, because (as I argued before) the set of demangled names
> generally doesn't change much in such cases, and we are "interning"
> the demangled symbols, so that multiple computations of the same
> demangled symbol don't increase space.  This particular choice of name
> storage is confined to the Ada module (the symbol table module doesn't
> apply free to names stored in symbols, partial symbols, or minimal
> symbols, so it doesn't care if one them has a longer than necessary
> lifetime).

I tend to think it's not ideal, but okay.  Daniel, you were unhappy
with this; did you have suggestions?

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> Under what circumstances does finding an objfile by the minsym's BFD
> section not work?  That minsym must have come from somewhere.  Do we
> produce minsyms whose sections are unset, for some reason?

Don't know that it happens with minsyms.  However, I have to find 
a place for demangled names for symbols other than minsyms (in Ada, we
can have mangled type names, enumeration literals, and field names), and for 
some of these I can't find an objfile.

Paul

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

>> So as an interim measure, I use your suggestion of 21 Jan and first
>> try to find an objfile via the BFD section.  When that doesn't work, I
>> simply use a global hashtable to hold the demangled strings.  Yes,
>> that is a memory leak, but on consideration, I realized that it's only
>> REALLY a memory leak if (a) I routinely change the entire set of
>> demangled names numerous times during a single GDB session, or (b)
>> demangle entirely different, large sets of names each time I reload
>> the symbol tables.  Yeah, I know, it's not pretty, but again I am hoping
>> it will ensure that demangled names behave until the next interation of
>> symtab modifications allow an entirely different strategy.

> I'm not sure what others will think of this interim measure.  I don't
> like it much, though.

I'm not overly fond of any of these solutions myself; they simply have
effectiveness to recommend them.  All I really need is to be able to
find an objfile, *any* objfile really, whose lifetime is at least as
great as the symbols are supposed to be.  Or I could keep an obstack
for this purpose, IF there were some way to know a safe time to 
reinitialize it (again, after the symbols die).

> What you did not explain is how [SYMBOL_DEMANGLED_SEARCH_NAME] 
> is supposed to be different from SYMBOL_SEARCH_NAME.

Well, the direct answer is that for Ada, 
      SYMBOL_DEMANGLED_SEARCH_NAME (sym) == NULL
whereas 
      SYMBOL_SEARCH_NAME (sym) == the "linkage name" of the symbol
Perhaps, now that you bring it up, it might be clearer simply to make
this a predicate: 
      SYMBOL_SEARCHED_BY_DEMANGLED_NAME (sym) 
or something like that?

> The existing ada_demangle never fails.  How does that interact with
> what you said above?  Hopefully not by tagging all minsyms as Ada!

Ah, I have been putting off syncing the ada-* files with ours (they
aren't compiled at the moment, and I feel no need to clutter the 
public file system with unused versions.  However, perhaps it's time to
check in the current versions, which are considerably different from the
snapshot that's currently there.  In short: you are quite right, and the
current Ada demangler returns NULL for non-Ada-mangled symbols.

Paul

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Tue, Apr 06, 2004 at 09:35:16AM -0500, Jim Blandy wrote:
> 
> Paul Hilfinger <hilfingr@gnat.com> writes:
> > Do you really object to our simply making permanent copies of these
> > names as needed?  (I'm perfectly happy to ditch the kludgy
> > objfile-finding hack.) You can't expect to save any space by
> > reclaiming these demangled names space when you reload the symbol
> > file, because (as I argued before) the set of demangled names
> > generally doesn't change much in such cases, and we are "interning"
> > the demangled symbols, so that multiple computations of the same
> > demangled symbol don't increase space.  This particular choice of name
> > storage is confined to the Ada module (the symbol table module doesn't
> > apply free to names stored in symbols, partial symbols, or minimal
> > symbols, so it doesn't care if one them has a longer than necessary
> > lifetime).
> 
> I tend to think it's not ideal, but okay.  Daniel, you were unhappy
> with this; did you have suggestions?

I'm unhappy with it, partly because not having a lifetime for these
things make it harder to identify memory leaks (of which we already
have quite enough, thank you).  But there are two options:

  - decide that lazily demangling is useful, and arrange to pass
    an objfile to every call to SYMBOL_DEMANGLED_NAME.  This also
    affects, at least, SYMBOL_NATURAL_NAME and SYMBOL_PRINT_NAME.
    I think it wouldn't be hard, just messy.
  - decide that being picky about the storage lifetime doesn't matter,
    and Paul's approximation is good enough.

I'm happier with the global hash table than I am with kludging around
searching for an objfile, certainly.  What do you think of the options?

On Fri, Apr 02, 2004 at 03:29:55AM -0500, Paul Hilfinger wrote:
> > What you did not explain is how [SYMBOL_DEMANGLED_SEARCH_NAME] 
> > is supposed to be different from SYMBOL_SEARCH_NAME.
> 
> Well, the direct answer is that for Ada, 
>       SYMBOL_DEMANGLED_SEARCH_NAME (sym) == NULL
> whereas 
>       SYMBOL_SEARCH_NAME (sym) == the "linkage name" of the symbol
> Perhaps, now that you bring it up, it might be clearer simply to make
> this a predicate: 
>       SYMBOL_SEARCHED_BY_DEMANGLED_NAME (sym) 
> or something like that?

I don't think that either of those divisions is general enough to be
useful.  Why should the search name have to be the linkage name or the
demangled name?  For C++ there are two potential 'search names' - the
name without DMGL_PARAMS, or just the basename.  Neither of these is
the linkage or natural name.

> > The existing ada_demangle never fails.  How does that interact with
> > what you said above?  Hopefully not by tagging all minsyms as Ada!
> 
> Ah, I have been putting off syncing the ada-* files with ours (they
> aren't compiled at the moment, and I feel no need to clutter the 
> public file system with unused versions.  However, perhaps it's time to
> check in the current versions, which are considerably different from the
> snapshot that's currently there.  In short: you are quite right, and the
> current Ada demangler returns NULL for non-Ada-mangled symbols.

Perhaps it's time to do this again.  I'm not sure how to handle it.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> I'm unhappy with it, partly because not having a lifetime for these
> things make it harder to identify memory leaks (of which we already
> have quite enough, thank you).  But there are two options:
> 
>   - decide that lazily demangling is useful, and arrange to pass
>     an objfile to every call to SYMBOL_DEMANGLED_NAME.  This also
>     affects, at least, SYMBOL_NATURAL_NAME and SYMBOL_PRINT_NAME.
>     I think it wouldn't be hard, just messy.
>   - decide that being picky about the storage lifetime doesn't matter,
>     and Paul's approximation is good enough.
> 
> I'm happier with the global hash table than I am with kludging around
> searching for an objfile, certainly.  What do you think of the options?

If leaking is your concern, how about this: I'll arrange our global
hash table for demangled names to store the strings themselves in a
a corresponding global obstack.  That way, the total amount of memory devoted 
to this particular set of demangled names is easy to monitor.  

There are a few opportunities, I suppose, to clear the whole thing,
such as when someone executes 'file' or 'symbol-file' (with no
arguments).  Not sure if they're worth exploiting.

> > Ah, I have been putting off syncing the ada-* files with ours (they
> > aren't compiled at the moment, and I feel no need to clutter the 
> > public file system with unused versions.  However, perhaps it's time to
> > check in the current versions, which are considerably different from the
> > snapshot that's currently there.  In short: you are quite right, and the
> > current Ada demangler returns NULL for non-Ada-mangled symbols.

> Perhaps it's time to do this again.  I'm not sure how to handle it.

I don't think that's a problem.  I'll take another stylistic pass over
the Ada sources and then just check in our current versions. Since
they aren't compiled (yet), they can't break anything, after all.

Paul Hilfinger
ACT, Inc.

Re: [RFA] Introduce notion of "search name"

[Jim Blandy]

Paul Hilfinger <hilfingr@gnat.com> writes:
> > I'm unhappy with it, partly because not having a lifetime for these
> > things make it harder to identify memory leaks (of which we already
> > have quite enough, thank you).  But there are two options:
> > 
> >   - decide that lazily demangling is useful, and arrange to pass
> >     an objfile to every call to SYMBOL_DEMANGLED_NAME.  This also
> >     affects, at least, SYMBOL_NATURAL_NAME and SYMBOL_PRINT_NAME.
> >     I think it wouldn't be hard, just messy.
> >   - decide that being picky about the storage lifetime doesn't matter,
> >     and Paul's approximation is good enough.
> > 
> > I'm happier with the global hash table than I am with kludging around
> > searching for an objfile, certainly.  What do you think of the options?
> 
> If leaking is your concern, how about this: I'll arrange our global
> hash table for demangled names to store the strings themselves in a
> a corresponding global obstack.  That way, the total amount of memory devoted 
> to this particular set of demangled names is easy to monitor.  
> 
> There are a few opportunities, I suppose, to clear the whole thing,
> such as when someone executes 'file' or 'symbol-file' (with no
> arguments).  Not sure if they're worth exploiting.
> 
> > > Ah, I have been putting off syncing the ada-* files with ours (they
> > > aren't compiled at the moment, and I feel no need to clutter the 
> > > public file system with unused versions.  However, perhaps it's time to
> > > check in the current versions, which are considerably different from the
> > > snapshot that's currently there.  In short: you are quite right, and the
> > > current Ada demangler returns NULL for non-Ada-mangled symbols.
> 
> > Perhaps it's time to do this again.  I'm not sure how to handle it.
> 
> I don't think that's a problem.  I'll take another stylistic pass over
> the Ada sources and then just check in our current versions. Since
> they aren't compiled (yet), they can't break anything, after all.

Daniel, you've got higher standards on this issue than I do, but I'd
like to see your standards prevail.  If you and Paul can work out
something, then I'll approve of it.

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

Daniel,

So where do we stand on all this?  I've been off on other things for a bit,
but I'd like to get the issues surrounding this patch settled.  

I understand your concern about memory leaks, but I've suggested why
first, these can be at least a contained problem, and second,
why in any case there's good reason to think my strategy doesn't really 
result in "leaks"---in the sense of garbage that never gets re-used.

I do admit that the changes you have been working on (at least as I
understand them from your remarks) might make it necessary (or at
least advisable) to do some considerable revision, but at the moment
we'd really like to start getting the ACT stuff activated in the
public version, even if we have to revisit certain parts later.

Paul Hilfinger

> Paul Hilfinger <hilfingr@gnat.com> writes:
> > > I'm unhappy with it, partly because not having a lifetime for these
> > > things make it harder to identify memory leaks (of which we already
> > > have quite enough, thank you).  But there are two options:
> > > 
> > >   - decide that lazily demangling is useful, and arrange to pass
> > >     an objfile to every call to SYMBOL_DEMANGLED_NAME.  This also
> > >     affects, at least, SYMBOL_NATURAL_NAME and SYMBOL_PRINT_NAME.
> > >     I think it wouldn't be hard, just messy.
> > >   - decide that being picky about the storage lifetime doesn't matter,
> > >     and Paul's approximation is good enough.
> > > 
> > > I'm happier with the global hash table than I am with kludging around
> > > searching for an objfile, certainly.  What do you think of the options?
> > 
> > If leaking is your concern, how about this: I'll arrange our global
> > hash table for demangled names to store the strings themselves in a
> > a corresponding global obstack.  That way, the total amount of memory devoted 
> > to this particular set of demangled names is easy to monitor.  
> > 
> > There are a few opportunities, I suppose, to clear the whole thing,
> > such as when someone executes 'file' or 'symbol-file' (with no
> > arguments).  Not sure if they're worth exploiting.
> > 
> > > > Ah, I have been putting off syncing the ada-* files with ours (they
> > > > aren't compiled at the moment, and I feel no need to clutter the 
> > > > public file system with unused versions.  However, perhaps it's time to
> > > > check in the current versions, which are considerably different from the
> > > > snapshot that's currently there.  In short: you are quite right, and the
> > > > current Ada demangler returns NULL for non-Ada-mangled symbols.
> > 
> > > Perhaps it's time to do this again.  I'm not sure how to handle it.
> > 
> > I don't think that's a problem.  I'll take another stylistic pass over
> > the Ada sources and then just check in our current versions. Since
> > they aren't compiled (yet), they can't break anything, after all.
> 
> Daniel, you've got higher standards on this issue than I do, but I'd
> like to see your standards prevail.  If you and Paul can work out
> something, then I'll approve of it.
> 

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

Dan & Jim,

> This is not what I was talking about - I was suggesting
> SYMBOL_DEMANGLED_NAME (symbol, objfile).  That's more mechanical work
> but less confusing.

Ah.  Well, a quick scan of the sources indicates that there are a few
places that ask for SYMBOL_PRINT_NAME in cases where it is not
apparent where to find an objfile.  On the other hand, there are many
other cases where there IS an objfile immediately to hand, so I
suspect your design can be made to work.  It won't, of course, be a
purely mechanical replacement.

> I don't care.  Go ahead without doing this if Jim's OK with the patch
> otherwise.  I'll come back and fix it in a year or two if I ever run
> out of memory leaks.

At the moment, I'd really like to concentrate on getting Ada support 
turned on, and any leakage problems arising from these changes will affect
only Ada users (who haven't complained about them in the last 7 years).  If 
you can accept the patch largely as it is for now, I will undertake 
to return to the objfile-finding problem after clearing my more immediate
hurdles.

As an update, here is the most recent version of my "search-name" patch.

Thanks for your attention.

Paul Hilfinger
ACT, Inc.

2004-05-03  Paul N. Hilfinger  <hilfinger@gnat.com>

	* symtab.h (SYMBOL_SEARCH_NAME): New definition.
	(SYMBOL_MATCHES_SEARCH_NAME): New definition.
	(SYMBOL_DEMANGLED_SEARCH_NAME): New definition.	

	* dictionary.c (iter_name_first_hashed): Match on SYMBOL_SEARCH_NAME.
	(iter_name_next_hashed): Ditto.
	(iter_name_next_linear): Ditto.
	(insert_symbol_hashed): Hash on SYMBOL_SEARCH_NAME.

	* symtab.c (lookup_partial_symbol): Assume symbols ordered by 
	search name, using SYMBOL_SEARCH_NAME and SYMBOL_MATCHES_SEARCH_NAME.
	(symbol_search_name): New function.
	(symbol_demangled_search_name): New function.
	
	* symfile.c (compare_psymbols): Order by SYMBOL_SEARCH_NAME.
	
	* minsyms.c (build_minimal_symbol_hash_tables): Use 
	SYMBOL_DEMANGLED_SEARCH_NAME to test for adding to demangled
	hash table.
	
Index: current-public.61/gdb/minsyms.c
--- current-public.61/gdb/minsyms.c Wed, 31 Mar 2004 23:53:02 -0800 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.2.1 644)
+++ current-public.61(w)/gdb/minsyms.c Sat, 01 May 2004 15:52:19 -0700 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.2.1 644)
@@ -791,7 +791,7 @@ build_minimal_symbol_hash_tables (struct
       add_minsym_to_hash_table (msym, objfile->msymbol_hash);
 
       msym->demangled_hash_next = 0;
-      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
+      if (SYMBOL_DEMANGLED_SEARCH_NAME (msym) != NULL)
 	add_minsym_to_demangled_hash_table (msym,
                                             objfile->msymbol_demangled_hash);
     }
Index: current-public.61/gdb/symfile.c
--- current-public.61/gdb/symfile.c Sat, 01 May 2004 02:27:55 -0700 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1.2.1.2.1.2.1 644)
+++ current-public.61(w)/gdb/symfile.c Mon, 03 May 2004 01:05:30 -0700 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1.2.1.2.1.2.1 644)
@@ -207,8 +207,8 @@ compare_psymbols (const void *s1p, const
   struct partial_symbol *const *s1 = s1p;
   struct partial_symbol *const *s2 = s2p;
 
-  return strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*s1),
-			    SYMBOL_NATURAL_NAME (*s2));
+  return strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*s1),
+			    SYMBOL_SEARCH_NAME (*s2));
 }
 
 void
Index: current-public.61/gdb/symtab.c
--- current-public.61/gdb/symtab.c Sat, 10 Apr 2004 00:36:50 -0700 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1.2.1 644)
+++ current-public.61(w)/gdb/symtab.c Mon, 03 May 2004 01:41:17 -0700 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1.2.1 644)
@@ -659,6 +659,18 @@ symbol_demangled_name (struct general_sy
     return NULL;
 }
 
+/* Return the search name of a symbol---i.e., the natural or linkage name 
+   of the symbol, depending on how it will be searched for.  */
+char *symbol_search_name (const struct general_symbol_info *gsymbol) {
+  return symbol_natural_name (gsymbol);
+}
+
+/* For languages that do not demangle or that do not do searches by 
+   demangled name, return NULL.  Otherwise, return the demangled name.  */
+char *symbol_demangled_search_name (struct general_symbol_info *gsymbol) {
+  return symbol_demangled_name (gsymbol);
+}
+
 /* Initialize the structure fields to zero values.  */
 void
 init_sal (struct symtab_and_line *sal)
@@ -1467,7 +1479,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -1482,7 +1494,7 @@ lookup_partial_symbol (struct partial_sy
       while (top <= real_top
 	     && (linkage_name != NULL
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
-		 : SYMBOL_MATCHES_NATURAL_NAME (*top,name)))
+		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
 	  if (SYMBOL_DOMAIN (*top) == domain)
 	    {
@@ -1503,7 +1515,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
-		  : SYMBOL_MATCHES_NATURAL_NAME (*psym, name))
+		  : SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
 		{
 		  return (*psym);
 		}
Index: current-public.61/gdb/symtab.h
--- current-public.61/gdb/symtab.h Sat, 10 Apr 2004 00:36:50 -0700 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1.2.1.2.1 644)
+++ current-public.61(w)/gdb/symtab.h Mon, 03 May 2004 01:42:37 -0700 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1.2.1.2.1 644)
@@ -258,6 +258,25 @@ extern char *symbol_demangled_name (stru
 #define SYMBOL_MATCHES_NATURAL_NAME(symbol, name)			\
   (strcmp_iw (SYMBOL_NATURAL_NAME (symbol), (name)) == 0)
 
+/* Macro that returns the name to be used when sorting and searching symbols. 
+   In  C++, Chill, and Java, we search for the demangled form of a name,
+   and so sort symbols accordingly.  In Ada, however, we search by mangled
+   name.  */
+#define SYMBOL_SEARCH_NAME(symbol)					 \
+   (symbol_search_name (&(symbol)->ginfo))
+extern char *symbol_search_name (const struct general_symbol_info *);
+
+/* Analogous to SYMBOL_MATCHES_NATURAL_NAME, but uses the search
+   name.  */
+#define SYMBOL_MATCHES_SEARCH_NAME(symbol, name)			\
+  (strcmp_iw (SYMBOL_SEARCH_NAME (symbol), (name)) == 0)
+
+/* For languages that do not demangle or that do not do searches by 
+   demangled name, NULL.  Otherwise, the demangled name.  */
+#define SYMBOL_DEMANGLED_SEARCH_NAME(symbol)				 \
+   (symbol_demangled_search_name (&(symbol)->ginfo))
+extern char *symbol_demangled_search_name (struct general_symbol_info *);
+
 /* Classification types for a minimal symbol.  These should be taken as
    "advisory only", since if gdb can't easily figure out a
    classification it simply selects mst_unknown.  It may also have to
Index: current-public.61/gdb/dictionary.c
--- current-public.61/gdb/dictionary.c Tue, 17 Jun 2003 02:41:56 -0700 hilfingr (GdbPub/L/b/35_dictionary 1.1 644)
+++ current-public.61(w)/gdb/dictionary.c Sat, 01 May 2004 15:52:19 -0700 hilfingr (GdbPub/L/b/35_dictionary 1.1 644)
@@ -636,7 +636,7 @@ iter_name_first_hashed (const struct dic
        sym = sym->hash_next)
     {
       /* Warning: the order of arguments to strcmp_iw matters!  */
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  break;
 	}
@@ -656,7 +656,7 @@ iter_name_next_hashed (const char *name,
        next != NULL;
        next = next->hash_next)
     {
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (next), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (next), name) == 0)
 	break;
     }
 
@@ -674,7 +674,7 @@ insert_symbol_hashed (struct dictionary 
   unsigned int hash_index;
   struct symbol **buckets = DICT_HASHED_BUCKETS (dict);
 
-  hash_index = (msymbol_hash_iw (SYMBOL_NATURAL_NAME (sym))
+  hash_index = (msymbol_hash_iw (SYMBOL_SEARCH_NAME (sym))
 		% DICT_HASHED_NBUCKETS (dict));
   sym->hash_next = buckets[hash_index];
   buckets[hash_index] = sym;
@@ -789,7 +789,7 @@ iter_name_next_linear (const char *name,
   for (i = DICT_ITERATOR_INDEX (iterator) + 1; i < nsyms; ++i)
     {
       sym = DICT_LINEAR_SYM (dict, i);
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  retval = sym;
 	  break;

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Mon, May 03, 2004 at 04:49:37AM -0400, Paul Hilfinger wrote:
> 
> Dan & Jim,
> 
> > This is not what I was talking about - I was suggesting
> > SYMBOL_DEMANGLED_NAME (symbol, objfile).  That's more mechanical work
> > but less confusing.
> 
> Ah.  Well, a quick scan of the sources indicates that there are a few
> places that ask for SYMBOL_PRINT_NAME in cases where it is not
> apparent where to find an objfile.  On the other hand, there are many
> other cases where there IS an objfile immediately to hand, so I
> suspect your design can be made to work.  It won't, of course, be a
> purely mechanical replacement.
> 
> > I don't care.  Go ahead without doing this if Jim's OK with the patch
> > otherwise.  I'll come back and fix it in a year or two if I ever run
> > out of memory leaks.
> 
> At the moment, I'd really like to concentrate on getting Ada support 
> turned on, and any leakage problems arising from these changes will affect
> only Ada users (who haven't complained about them in the last 7 years).  If 
> you can accept the patch largely as it is for now, I will undertake 
> to return to the objfile-finding problem after clearing my more immediate
> hurdles.
> 
> As an update, here is the most recent version of my "search-name" patch.
> 
> Thanks for your attention.

We've more or less consensused on ignoring the memory lifetime issue. 
Unfortunately, discussion of that took over the thread, and you never
answered my last question about this patch:

 On Fri, Apr 02, 2004 at 03:29:55AM -0500, Paul Hilfinger wrote:
 > > What you did not explain is how [SYMBOL_DEMANGLED_SEARCH_NAME]                                      
 > > is supposed to be different from SYMBOL_SEARCH_NAME.                                                
 >                                                                                                       
 > Well, the direct answer is that for Ada,                                                              
 >       SYMBOL_DEMANGLED_SEARCH_NAME (sym) == NULL                                                      
 > whereas                                                                                               
 >       SYMBOL_SEARCH_NAME (sym) == the "linkage name" of the symbol                                    
 > Perhaps, now that you bring it up, it might be clearer simply to make                                 
 > this a predicate:                                                                                     
 >       SYMBOL_SEARCHED_BY_DEMANGLED_NAME (sym)                                                         
 > or something like that?                                                                               

 I don't think that either of those divisions is general enough to be
 useful.  Why should the search name have to be the linkage name or the
 demangled name?  For C++ there are two potential 'search names' - the
 name without DMGL_PARAMS, or just the basename.  Neither of these is
 the linkage or natural name.

I don't want us to proliferate name-related macros without a very clear
understanding of when each one is appropriate.

-- 
Daniel Jacobowitz

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> Unfortunately, discussion of that took over the thread, and you never
> answered my last question about this patch:
> 
>  On Fri, Apr 02, 2004 at 03:29:55AM -0500, Paul Hilfinger wrote:
>  > > What you did not explain is how [SYMBOL_DEMANGLED_SEARCH_NAME]                                      
>  > > is supposed to be different from SYMBOL_SEARCH_NAME.                                                
>  >                                                                                                       
>  > Well, the direct answer is that for Ada,                                                              
>  >       SYMBOL_DEMANGLED_SEARCH_NAME (sym) == NULL                                                      
>  > whereas                                                                                               
>  >       SYMBOL_SEARCH_NAME (sym) == the "linkage name" of the symbol                                    
>  > Perhaps, now that you bring it up, it might be clearer simply to make                                 
>  > this a predicate:                                                                                     
>  >       SYMBOL_SEARCHED_BY_DEMANGLED_NAME (sym)                                                         
>  > or something like that?                                                                               
> 
>  I don't think that either of those divisions is general enough to be
>  useful.  Why should the search name have to be the linkage name or the
>  demangled name?  For C++ there are two potential 'search names' - the
>  name without DMGL_PARAMS, or just the basename.  Neither of these is
>  the linkage or natural name.
> 
> I don't want us to proliferate name-related macros without a very clear
> understanding of when each one is appropriate.

Daniel,

OK.  The only use for SYMBOL_SEARCHED_BY_DEMANGLED_NAME is to answer
the question, "Do we need to index this minimal symbol under its
demangled name?"  It would work to re-write the test in
build_minimal_symbol_hash_tables as

      if (SYMBOL_SEARCH_NAME (msym) != SYMBOL_LINKAGE_NAME (msym))
	add_minsym_to_demangled_hash_table (msym,
                                            objfile->msymbol_demangled_hash);

from the current

     if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
	...

(although to use !=, you'd also want to document the fact that when
SYMBOL_SEARCH_NAME is strcmp-equal to SYMBOL_LINKAGE_NAME, it is also
pointer equal).  This re-write avoids introducing a new name, answering 
one of your objections.  Furthermore, minimal symbols are searched for only
by the linkage name or the search name (by definition), so it seems that the
proposed test is correct.  

What do you think?

Paul

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Wed, May 12, 2004 at 06:59:59AM -0400, Paul Hilfinger wrote:
> 
> > Unfortunately, discussion of that took over the thread, and you never
> > answered my last question about this patch:
> > 
> >  On Fri, Apr 02, 2004 at 03:29:55AM -0500, Paul Hilfinger wrote:
> >  > > What you did not explain is how [SYMBOL_DEMANGLED_SEARCH_NAME]                                      
> >  > > is supposed to be different from SYMBOL_SEARCH_NAME.                                                
> >  >                                                                                                       
> >  > Well, the direct answer is that for Ada,                                                              
> >  >       SYMBOL_DEMANGLED_SEARCH_NAME (sym) == NULL                                                      
> >  > whereas                                                                                               
> >  >       SYMBOL_SEARCH_NAME (sym) == the "linkage name" of the symbol                                    
> >  > Perhaps, now that you bring it up, it might be clearer simply to make                                 
> >  > this a predicate:                                                                                     
> >  >       SYMBOL_SEARCHED_BY_DEMANGLED_NAME (sym)                                                         
> >  > or something like that?                                                                               
> > 
> >  I don't think that either of those divisions is general enough to be
> >  useful.  Why should the search name have to be the linkage name or the
> >  demangled name?  For C++ there are two potential 'search names' - the
> >  name without DMGL_PARAMS, or just the basename.  Neither of these is
> >  the linkage or natural name.
> > 
> > I don't want us to proliferate name-related macros without a very clear
> > understanding of when each one is appropriate.
> 
> Daniel,
> 
> OK.  The only use for SYMBOL_SEARCHED_BY_DEMANGLED_NAME is to answer
> the question, "Do we need to index this minimal symbol under its
> demangled name?"  It would work to re-write the test in
> build_minimal_symbol_hash_tables as
> 
>       if (SYMBOL_SEARCH_NAME (msym) != SYMBOL_LINKAGE_NAME (msym))
> 	add_minsym_to_demangled_hash_table (msym,
>                                             objfile->msymbol_demangled_hash);
> 
> from the current
> 
>      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
> 	...
> 
> (although to use !=, you'd also want to document the fact that when
> SYMBOL_SEARCH_NAME is strcmp-equal to SYMBOL_LINKAGE_NAME, it is also
> pointer equal).  This re-write avoids introducing a new name, answering 
> one of your objections.  Furthermore, minimal symbols are searched for only
> by the linkage name or the search name (by definition), so it seems that the
> proposed test is correct.  
> 
> What do you think?

I like it!

-- 
Daniel Jacobowitz

Re: [RFA] Introduce notion of "search name"

[Andrew Cagney]

OK.  The only use for SYMBOL_SEARCHED_BY_DEMANGLED_NAME is to answer
the question, "Do we need to index this minimal symbol under its
demangled name?"  It would work to re-write the test in
build_minimal_symbol_hash_tables as
      if (SYMBOL_SEARCH_NAME (msym) != SYMBOL_LINKAGE_NAME (msym))
	add_minsym_to_demangled_hash_table (msym,
                                            objfile->msymbol_demangled_hash);
from the current

     if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
	...
(although to use !=, you'd also want to document the fact that when
SYMBOL_SEARCH_NAME is strcmp-equal to SYMBOL_LINKAGE_NAME, it is also
pointer equal).  This re-write avoids introducing a new name, answering 
one of your objections.  Furthermore, minimal symbols are searched for only
by the linkage name or the search name (by definition), so it seems that the
proposed test is correct.  

What do you think?


I like it!
Brain explodes.

Why, when doing these symtab comparisons, are we insisting that the 
symtab convert everything to a string, and then that the client 
manipulate that?

Surely the only time a symbol name needs to be converted to a published 
string is when we print it (even then *sym_puts (sym, ui_file) would 
do).  For other cases, the last thing we should be doing is locking the 
symtab into string representations.  Instead it should publish 
interfaces such as:
	sym_demangled_cmp (sym1, sym2)
and I suspect that there's an equivalent here.

Andrew

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Wed, May 12, 2004 at 10:14:22AM -0400, Andrew Cagney wrote:
> Brain explodes.

Mine too, in response.

> Why, when doing these symtab comparisons, are we insisting that the 
> symtab convert everything to a string, and then that the client 
> manipulate that?
> 
> Surely the only time a symbol name needs to be converted to a published 
> string is when we print it (even then *sym_puts (sym, ui_file) would 
> do).  For other cases, the last thing we should be doing is locking the 
> symtab into string representations.  Instead it should publish 
> interfaces such as:
> 	sym_demangled_cmp (sym1, sym2)
> and I suspect that there's an equivalent here.

This doesn't make the slightest bit of sense to me.  Maybe I'm just not
getting the abstraction you're suggesting.  In the code Paul referred
to, we're inside the symtab implementation trying to add the symbol to
a hash table, which we will later search _by string, from the user_. 
What else would we do?  Given that we are representing symbols by their
name, how else than a string representation would you suggest?

-- 
Daniel Jacobowitz

Re: [RFA] Introduce notion of "search name"

[Andrew Cagney]

A quick grep of GDB shows that we're already doing:

$ grep -e SYMBOL_PRINT_NAME -e SYMBOL_CPLUS_DEMANGLED_NAME -e 
DEPRECATED_SYMBOL_NAME -e SYMBOL_NATURAL_NAME -e SYMBOL_SOURCE_NAME -e 
SYMBOL_LINKAGE_NAME *.c  | grep -v -e ^sym -e syms.c: -e read.c: | cut 
-d: -f1 | sort -u
ada-lang.c
ada-typeprint.c
ax-gdb.c
blockframe.c
breakpoint.c
buildsym.c
c-valprint.c
cp-namespace.c
cp-support.c
cp-valprint.c
dictionary.c
dwarf2loc.c
expprint.c
f-valprint.c
findvar.c
glibc-tdep.c
gnu-v2-abi.c
hppa-hpux-tdep.c
i386-tdep.c
infcall.c
infcmd.c
jv-lang.c
linespec.c
maint.c
objc-lang.c
objfiles.c
p-valprint.c
ppc-sysv-tdep.c
printcmd.c
rs6000-tdep.c
solib-frv.c
somsolib.c
stack.c
tracepoint.c
typeprint.c
valops.c

I.e., through out GDB there is the assumption that symtab internally 
uses simple strings.

We shouldn't.  Recalling that one of the underlying problems here was 
the need to construct search names on the fly using more complext debug 
info, we should instead have interfaces such as:

symtab_name_put (symtab, ui_file):
Write the printable name to the specified output.
symtab_demanged_cmp (block, symtab, symtab):
Compare to symbols returning their relative position.
that completly abastract symbol name lifetimes (you never know if it is 
defined).

By doing that we better specify the symtab interface; clarify the 
clients requirements; and free the symtab code of specific 
implementation assumptions.

We can also finally answer questions such as:
- how often is the printed name used
- how often is a symbol comparison performed
and use that to adjust the internal representation so that it better 
meets real needs

Andrew

Re: [RFA] Introduce notion of "search name"

[Joel Brobecker]

> I.e., through out GDB there is the assumption that symtab internally 
> uses simple strings.
> 
> We shouldn't.  Recalling that one of the underlying problems here was 
> the need to construct search names on the fly using more complext debug 
> info, we should instead have interfaces such as:

This is a worthy goal, but I see it as a goal separate from what the
patch is trying to achieve. From what Paul is saying, the change you
are objecting to introduces a check that is correct within the current
implementation, and makes the kind of assumption that's already used
everywhere. Why can't we use the current framework as is until the
cleanup is done (BTW: who's going to do it, and when?). Admitedly, we
have to document a temporary assumption, but I don't think this will
affect the symbol interface cleanup you're suggesting all that much.

-- 
Joel

Re: [RFA] Introduce notion of "search name"

[Andrew Cagney]

I.e., through out GDB there is the assumption that symtab internally 
uses simple strings.

We shouldn't.  Recalling that one of the underlying problems here was 
the need to construct search names on the fly using more complext debug 
info, we should instead have interfaces such as:


This is a worthy goal, but I see it as a goal separate from what the
patch is trying to achieve.
This is in part true.  The mistake is to seperate cleanups (or 
refactorings) from enhancements.  Enhancement and fixes needs to go 
hand-in-hand with a corresponding cleanups.  If we don't do this, we 
quickly find that those long over-due changes never occure.

> From what Paul is saying, the change you
> are objecting to introduces a check that is correct within the current
> implementation, and makes the kind of assumption that's already used
> everywhere. Why can't we use the current framework as is until the
> cleanup is done (BTW: who's going to do it, and when?). Admitedly, we
> have to document a temporary assumption, but I don't think this will
> affect the symbol interface cleanup you're suggesting all that much.
If this, as an implementation detail, were to be strictly hidden behind 
the symtab interface, it wouldn't be a problem.  Unfortunatly, as I 
illustrated, the SYMBOL_*_NAME macros permieate GDB, so hand in hand 
with this change, should be changes to screw down that aspect of the 
symtab interface, and then have Ada use that.

enjoy,
Andew

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

Oh great.  I walk into my office to find skull fragments all over the
floor, but still no final decision.  Ah well.  Risking exposure to
stray prions, I submit the latest version of this patch below.

I am actually sympathetic to Andrew's ideas here, since with some
version of the extra abstraction he suggests, I could avoid all
permanent storage for demangled names.  However, it WOULD be a
substantial change and speaking for ACT, we'd really, really first
like to get some substantial portion of the Ada changes integrated
into (and active) in the public GDB version.

Paul Hilfinger

2004-05-13  Paul N. Hilfinger  <hilfinger@gnat.com>

	* symtab.h (SYMBOL_SEARCH_NAME): New definition.
	(SYMBOL_MATCHES_SEARCH_NAME): New definition.
	(symbol_search_name): Declare.
	
	* dictionary.c (iter_name_first_hashed): Match on SYMBOL_SEARCH_NAME.
	(iter_name_next_hashed): Ditto.
	(iter_name_next_linear): Ditto.
	(insert_symbol_hashed): Hash on SYMBOL_SEARCH_NAME.

	* symtab.c (lookup_partial_symbol): Assume symbols ordered by 
	search name, using SYMBOL_SEARCH_NAME and SYMBOL_MATCHES_SEARCH_NAME.
	(symbol_search_name): New function.
	
	* symfile.c (compare_psymbols): Order by SYMBOL_SEARCH_NAME.
	
	* minsyms.c (build_minimal_symbol_hash_tables): Change
	test for adding to demangled hash table to check for difference
	between SYMBOL_SEARCH_NAME and SYMBOL_LINKAGE_NAME.
	

Index: current-public.63/gdb/minsyms.c
--- current-public.63/gdb/minsyms.c Wed, 31 Mar 2004 23:53:02 -0800 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.2.1 644)
+++ current-public.63(w)/gdb/minsyms.c Thu, 13 May 2004 01:59:29 -0700 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.2.1 644)
@@ -791,7 +791,7 @@ build_minimal_symbol_hash_tables (struct
       add_minsym_to_hash_table (msym, objfile->msymbol_hash);
 
       msym->demangled_hash_next = 0;
-      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
+      if (SYMBOL_SEARCH_NAME (msym) != SYMBOL_LINKAGE_NAME (msym))
 	add_minsym_to_demangled_hash_table (msym,
                                             objfile->msymbol_demangled_hash);
     }
Index: current-public.63/gdb/symfile.c
--- current-public.63/gdb/symfile.c Thu, 06 May 2004 01:06:20 -0700 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1.2.1.2.1.2.1.2.1 644)
+++ current-public.63(w)/gdb/symfile.c Thu, 13 May 2004 02:00:21 -0700 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1.2.1.2.1.2.1.2.1 644)
@@ -207,8 +207,8 @@ compare_psymbols (const void *s1p, const
   struct partial_symbol *const *s1 = s1p;
   struct partial_symbol *const *s2 = s2p;
 
-  return strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*s1),
-			    SYMBOL_NATURAL_NAME (*s2));
+  return strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*s1),
+			    SYMBOL_SEARCH_NAME (*s2));
 }
 
 void
Index: current-public.63/gdb/symtab.c
--- current-public.63/gdb/symtab.c Sat, 10 Apr 2004 00:36:50 -0700 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1.2.1 644)
+++ current-public.63(w)/gdb/symtab.c Thu, 13 May 2004 01:58:56 -0700 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1.2.1 644)
@@ -659,6 +659,12 @@ symbol_demangled_name (struct general_sy
     return NULL;
 }
 
+/* Return the search name of a symbol---i.e., the natural or linkage name 
+   of the symbol, depending on how it will be searched for.  */
+char *symbol_search_name (const struct general_symbol_info *gsymbol) {
+  return symbol_natural_name (gsymbol);
+}
+
 /* Initialize the structure fields to zero values.  */
 void
 init_sal (struct symtab_and_line *sal)
@@ -1467,7 +1473,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -1482,7 +1488,7 @@ lookup_partial_symbol (struct partial_sy
       while (top <= real_top
 	     && (linkage_name != NULL
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
-		 : SYMBOL_MATCHES_NATURAL_NAME (*top,name)))
+		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
 	  if (SYMBOL_DOMAIN (*top) == domain)
 	    {
@@ -1503,7 +1509,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
-		  : SYMBOL_MATCHES_NATURAL_NAME (*psym, name))
+		  : SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
 		{
 		  return (*psym);
 		}
Index: current-public.63/gdb/symtab.h
--- current-public.63/gdb/symtab.h Sat, 10 Apr 2004 00:36:50 -0700 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1.2.1.2.1 644)
+++ current-public.63(w)/gdb/symtab.h Thu, 13 May 2004 01:58:24 -0700 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1.2.1.2.1 644)
@@ -258,6 +258,19 @@ extern char *symbol_demangled_name (stru
 #define SYMBOL_MATCHES_NATURAL_NAME(symbol, name)			\
   (strcmp_iw (SYMBOL_NATURAL_NAME (symbol), (name)) == 0)
 
+/* Macro that returns the name to be used when sorting and searching symbols. 
+   In  C++, Chill, and Java, we search for the demangled form of a name,
+   and so sort symbols accordingly.  In Ada, however, we search by mangled
+   name.  */
+#define SYMBOL_SEARCH_NAME(symbol)					 \
+   (symbol_search_name (&(symbol)->ginfo))
+extern char *symbol_search_name (const struct general_symbol_info *);
+
+/* Analogous to SYMBOL_MATCHES_NATURAL_NAME, but uses the search
+   name.  */
+#define SYMBOL_MATCHES_SEARCH_NAME(symbol, name)			\
+  (strcmp_iw (SYMBOL_SEARCH_NAME (symbol), (name)) == 0)
+
 /* Classification types for a minimal symbol.  These should be taken as
    "advisory only", since if gdb can't easily figure out a
    classification it simply selects mst_unknown.  It may also have to
Index: current-public.63/gdb/dictionary.c
--- current-public.63/gdb/dictionary.c Tue, 17 Jun 2003 02:41:56 -0700 hilfingr (GdbPub/L/b/35_dictionary 1.1 644)
+++ current-public.63(w)/gdb/dictionary.c Sat, 01 May 2004 15:52:19 -0700 hilfingr (GdbPub/L/b/35_dictionary 1.1 644)
@@ -636,7 +636,7 @@ iter_name_first_hashed (const struct dic
        sym = sym->hash_next)
     {
       /* Warning: the order of arguments to strcmp_iw matters!  */
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  break;
 	}
@@ -656,7 +656,7 @@ iter_name_next_hashed (const char *name,
        next != NULL;
        next = next->hash_next)
     {
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (next), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (next), name) == 0)
 	break;
     }
 
@@ -674,7 +674,7 @@ insert_symbol_hashed (struct dictionary 
   unsigned int hash_index;
   struct symbol **buckets = DICT_HASHED_BUCKETS (dict);
 
-  hash_index = (msymbol_hash_iw (SYMBOL_NATURAL_NAME (sym))
+  hash_index = (msymbol_hash_iw (SYMBOL_SEARCH_NAME (sym))
 		% DICT_HASHED_NBUCKETS (dict));
   sym->hash_next = buckets[hash_index];
   buckets[hash_index] = sym;
@@ -789,7 +789,7 @@ iter_name_next_linear (const char *name,
   for (i = DICT_ITERATOR_INDEX (iterator) + 1; i < nsyms; ++i)
     {
       sym = DICT_LINEAR_SYM (dict, i);
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  retval = sym;
 	  break;

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Thu, May 13, 2004 at 05:30:23AM -0400, Paul Hilfinger wrote:
> 
> Oh great.  I walk into my office to find skull fragments all over the
> floor, but still no final decision.  Ah well.  Risking exposure to
> stray prions, I submit the latest version of this patch below.
> 
> I am actually sympathetic to Andrew's ideas here, since with some
> version of the extra abstraction he suggests, I could avoid all
> permanent storage for demangled names.  However, it WOULD be a
> substantial change and speaking for ACT, we'd really, really first
> like to get some substantial portion of the Ada changes integrated
> into (and active) in the public GDB version.

This patch is OK with me; let's see if Jim likes it.  Thanks for
following through; you've been a real sport about this.

> 
> Paul Hilfinger
> 
> 2004-05-13  Paul N. Hilfinger  <hilfinger@gnat.com>
> 
> 	* symtab.h (SYMBOL_SEARCH_NAME): New definition.
> 	(SYMBOL_MATCHES_SEARCH_NAME): New definition.
> 	(symbol_search_name): Declare.
> 	
> 	* dictionary.c (iter_name_first_hashed): Match on SYMBOL_SEARCH_NAME.
> 	(iter_name_next_hashed): Ditto.
> 	(iter_name_next_linear): Ditto.
> 	(insert_symbol_hashed): Hash on SYMBOL_SEARCH_NAME.
> 
> 	* symtab.c (lookup_partial_symbol): Assume symbols ordered by 
> 	search name, using SYMBOL_SEARCH_NAME and SYMBOL_MATCHES_SEARCH_NAME.
> 	(symbol_search_name): New function.
> 	
> 	* symfile.c (compare_psymbols): Order by SYMBOL_SEARCH_NAME.
> 	
> 	* minsyms.c (build_minimal_symbol_hash_tables): Change
> 	test for adding to demangled hash table to check for difference
> 	between SYMBOL_SEARCH_NAME and SYMBOL_LINKAGE_NAME.

-- 
Daniel Jacobowitz

Re: [RFA] Introduce notion of "search name"

[Jim Blandy]

Paul Hilfinger <hilfingr@gnat.com> writes:
> I am actually sympathetic to Andrew's ideas here, since with some
> version of the extra abstraction he suggests, I could avoid all
> permanent storage for demangled names.

I think generalizing names is probably the way to go.  Certainly C++
needs names with some structure; using strings there is silly.  I'm
not convinced it can be made quite as simple as Andrew says, but I
could be wrong about that.

But, either way, I don't think this patch should be blocked on making
that overhaul.

> Index: current-public.63/gdb/minsyms.c
> --- current-public.63/gdb/minsyms.c Wed, 31 Mar 2004 23:53:02 -0800 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.2.1 644)
> +++ current-public.63(w)/gdb/minsyms.c Thu, 13 May 2004 01:59:29 -0700 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.2.1 644)
> @@ -791,7 +791,7 @@ build_minimal_symbol_hash_tables (struct
>        add_minsym_to_hash_table (msym, objfile->msymbol_hash);
>  
>        msym->demangled_hash_next = 0;
> -      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
> +      if (SYMBOL_SEARCH_NAME (msym) != SYMBOL_LINKAGE_NAME (msym))
>  	add_minsym_to_demangled_hash_table (msym,
>                                              objfile->msymbol_demangled_hash);
>      }

We can use != to compare the names here because symbol_set_names only
stores pointers to objfile->demangled_names_hash keys --- right?

If Daniel J. has signed off on this, it looks fine to me.

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> But, either way, I don't think this patch should be blocked on making
> that overhaul.
...
> If Daniel J. has signed off on this, it looks fine to me.

I am interpreting his messages as saying he was (he will no doubt
swiftly correct me otherwise).  Great!  In that case, I will commit
this patch later today, after running the test suite.

> Paul Hilfinger <hilfingr@gnat.com> writes:
> > I am actually sympathetic to Andrew's ideas here, since with some
> > version of the extra abstraction he suggests, I could avoid all
> > permanent storage for demangled names.
> 
> I think generalizing names is probably the way to go.  Certainly C++
> needs names with some structure; using strings there is silly.  I'm
> not convinced it can be made quite as simple as Andrew says, but I
> could be wrong about that.

In view of the fact that both you and Andrew seem to think that more 
abstraction is needed here, and in view of the fact that I believe I could 
take advantage of a properly formulated abstraction to further reduce memory 
needs, I will undertake to revisit this issue after getting a reasonable
amount of Ada support stably ported to the public version.  

> >        add_minsym_to_hash_table (msym, objfile->msymbol_hash);
> >  
> >        msym->demangled_hash_next = 0;
> > -      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
> > +      if (SYMBOL_SEARCH_NAME (msym) != SYMBOL_LINKAGE_NAME (msym))
> >  	add_minsym_to_demangled_hash_table (msym,
> >                                              objfile->msymbol_demangled_hash);
> >      }

> We can use != to compare the names here because symbol_set_names only
> stores pointers to objfile->demangled_names_hash keys --- right?

Close.  The minimal symbol demangled hash table is needed only when
one searches for symbols using some key that differs from the "linkage
name".  The condition

     SYMBOL_SEARCH_NAME (msym) != SYMBOL_LINKAGE_NAME (msym)

must necessarily be true in that case, regardless of where we store demangled
names.  My intention was that when there is no demangled name distinct from
the "linkage name", then SYMBOL_SEARCH_NAME should return the same 
object as "linkage name" (which reminds me that I must update the comment
to that effect before submitting).   

Paul Hilfinger

Re: [RFA] Introduce notion of "search name"

[Daniel Jacobowitz]

On Wed, May 19, 2004 at 05:55:42AM -0400, Paul Hilfinger wrote:
> 
> > But, either way, I don't think this patch should be blocked on making
> > that overhaul.
> ...
> > If Daniel J. has signed off on this, it looks fine to me.
> 
> I am interpreting his messages as saying he was (he will no doubt
> swiftly correct me otherwise).  Great!  In that case, I will commit
> this patch later today, after running the test suite.

Nah, it's fine by me :)


-- 
Daniel Jacobowitz

Re: [RFA] Introduce notion of "search name"

[Andrew Cagney]

Paul Hilfinger <hilfingr@gnat.com> writes:

I am actually sympathetic to Andrew's ideas here, since with some
version of the extra abstraction he suggests, I could avoid all
permanent storage for demangled names.


I think generalizing names is probably the way to go.  Certainly C++
needs names with some structure; using strings there is silly.  I'm
not convinced it can be made quite as simple as Andrew says, but I
could be wrong about that.
Just like the frame, arch, and regcache code, this won't be easy or 
simple - it will take multiple iterations and true long term commitment 
to GDB.

What we do need to do is recognize the real value in perusing this 
strategy (screwing down and better defining interfaces), and that it is 
a strategy we can't afford to postpone.  Can we do that?

Andrew

Abstracting "name"

[Paul Hilfinger]

[Subject changed from Re: [RFA] Introduce notion of "search name"]

Andrew Cagney wrote:

>Just like the frame, arch, and regcache code, this won't be easy or 
>simple - it will take multiple iterations and true long term commitment 
>to GDB.

>What we do need to do is recognize the real value in perusing this 
>strategy (screwing down and better defining interfaces), and that it is 
>a strategy we can't afford to postpone.  Can we do that?

I'm all for good interfaces.  The first step here, it seems to me, is to
understand better just what we want out of such an interface---a 
requirements memo or API sketch.  Andrew has so far listed the following:

    symtab_name_put (symbol, ui_file):
    Write the printable name to the specified output.

    symtab_demangled_cmp (block, symbol, symbol):
    Compare two symbols returning their relative position.

[Actually, Andrew's name for this last was "symtab_demanged_cmp", which is
 appealing, but perhaps too likely to provoke remark. :->)]  Presumably,
you'd also need various functions such as 

    symtab_*_name_matches (char*, symbol)
    
where * has a number of replacements indicating the type of match desired
(literal, demangled, etc.).  Or there could be one of these with an
extra modifier parameter.  I presume the dictionary module is
to be folded into this; will this require a symtab_hash_value (symbol) 
operation?  Also, what will clients want to do with structured names?  

Paul Hilfinger

Re: Abstracting "name"

[Andrew Cagney]

[Subject changed from Re: [RFA] Introduce notion of "search name"]

Andrew Cagney wrote:


Just like the frame, arch, and regcache code, this won't be easy or 
simple - it will take multiple iterations and true long term commitment 
to GDB.


What we do need to do is recognize the real value in perusing this 
strategy (screwing down and better defining interfaces), and that it is 
a strategy we can't afford to postpone.  Can we do that?


I'm all for good interfaces.  The first step here, it seems to me, is to
understand better just what we want out of such an interface---a 
requirements memo or API sketch.  Andrew has so far listed the following:

    symtab_name_put (symbol, ui_file):
    Write the printable name to the specified output.
    symtab_demangled_cmp (block, symbol, symbol):
    Compare two symbols returning their relative position.
[Actually, Andrew's name for this last was "symtab_demanged_cmp", which is
 appealing, but perhaps too likely to provoke remark. :->)]  Presumably,
you'd also need various functions such as 

    symtab_*_name_matches (char*, symbol)
(const char * :-)

FYI, another long term objective is to make published symbol objects a 
flyweight.  Stop trying to differentiate between full and partial 
symbols (except within the symtab code) and stop dictating that there's 
this two stage load.

Andrew

where * has a number of replacements indicating the type of match desired
(literal, demangled, etc.).  Or there could be one of these with an
extra modifier parameter.  I presume the dictionary module is
to be folded into this; will this require a symtab_hash_value (symbol) 
operation?  Also, what will clients want to do with structured names?  

Paul Hilfinger

Re: Abstracting "name"

[Jim Blandy]

Andrew Cagney <cagney@gnu.org> writes:
> > [Subject changed from Re: [RFA] Introduce notion of "search name"]
> > Andrew Cagney wrote:
> >
> >>> Just like the frame, arch, and regcache code, this won't be easy
> >>> or simple - it will take multiple iterations and true long term
> >>> commitment to GDB.
> >
> >>> What we do need to do is recognize the real value in perusing this
> >>> strategy (screwing down and better defining interfaces), and that
> >>> it is a strategy we can't afford to postpone.  Can we do that?
> > I'm all for good interfaces.  The first step here, it seems to me,
> > is to
> > understand better just what we want out of such an interface---a
> > requirements memo or API sketch.  Andrew has so far listed the following:
> >     symtab_name_put (symbol, ui_file):
> >     Write the printable name to the specified output.
> >     symtab_demangled_cmp (block, symbol, symbol):
> >     Compare two symbols returning their relative position.
> > [Actually, Andrew's name for this last was "symtab_demanged_cmp",
> > which is
> >  appealing, but perhaps too likely to provoke remark. :->)]  Presumably,
> > you'd also need various functions such as     symtab_*_name_matches
> > (char*, symbol)
> (const char * :-)
> 
> FYI, another long term objective is to make published symbol objects a
> flyweight.  Stop trying to differentiate between full and partial
> symbols (except within the symtab code) and stop dictating that
> there's this two stage load.

Yes --- reading symbolic information incrementally is a good strategy,
but it needs to be done differently for STABS and Dwarf 2.  The
interface shouldn't expose how or when various stages take place.

Dividing information about symbols and types into intrinsic (names;
lists of members and their types) and extrinsic (the address this
symbol has in this run of this executable) and then taking advantage
of the new sharing opportunities that enables would be nice, too.

The two are distinct goals, though.

Re: [RFA] Introduce notion of "search name"

[David Carlton]

On Wed, 3 Mar 2004 14:15:50 -0500 (EST), Paul Hilfinger
<hilfingr@gnat.com> said:

> The modification to the signature of symbol_natural_name is to allow the
> option of delayed (lazy) evaluation of the demangled name, which is
> actually the point of introducing search names.

Personally, I would leave the signature as is and cast away the
constness when you eventually add this lazy demangling.  My
justification is that the operation is logically a const operation;
you're planning to generate some information on the fly, but it
wouldn't actually change the state of the object.

Obviously this would work better if GDB were written in C++ (since,
for example, we could declare the affect field in the symbol to be
mutable); I'm honestly not sure if my suggestion is idiomatic C or
not.  (Then again, const and C have never been the closest of cousins
to begin with.)

David Carlton
carlton@kealia.com

Re: [RFA] Introduce notion of "search name"

[David Carlton]

On Wed, 3 Mar 2004 14:15:50 -0500 (EST), Paul Hilfinger
<hilfingr@gnat.com> said:

> The modification to the signature of symbol_natural_name is to allow the
> option of delayed (lazy) evaluation of the demangled name, which is
> actually the point of introducing search names.

Personally, I would leave the signature as is and cast away the
constness when you eventually add this lazy demangling.  My
justification is that the operation is logically a const operation;
you're planning to generate some information on the fly, but it
wouldn't actually change the state of the object.

Obviously this would work better if GDB were written in C++ (since,
for example, we could declare the affect field in the symbol to be
mutable); I'm honestly not sure if my suggestion is idiomatic C or
not.  (Then again, const and C have never been the closest of cousins
to begin with.)

David Carlton
carlton@kealia.com

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> Personally, I would leave the signature as is and cast away the
> constness when you eventually add this lazy demangling.  My
> justification is that the operation is logically a const operation;
> you're planning to generate some information on the fly, but it
> wouldn't actually change the state of the object.

Sounds fine to me.  I suppose that would be as close to the "mutable" idiom
as one can come in C.

Paul Hilfinger
Hilfinger@gnat.com

Re: [RFA] Introduce notion of "search name"

[Paul Hilfinger]

> Personally, I would leave the signature as is and cast away the
> constness when you eventually add this lazy demangling.  My
> justification is that the operation is logically a const operation;
> you're planning to generate some information on the fly, but it
> wouldn't actually change the state of the object.

Sounds fine to me.  I suppose that would be as close to the "mutable" idiom
as one can come in C.

Paul Hilfinger
Hilfinger@gnat.com

[RFA] Introduce notion of "search name"

[Paul Hilfinger]

The following patch does nothing except to prepare the way for some later
Ada modifications by providing a few "hooks". I've discussed this 
modification earlier on this newsgroup.  At the moment, I've given
the definitions placeholder definitions that simply result in the
current semantics.  I propose to change them when Ada support is turned on.

The idea is to define "search name" as "the name that a language
module uses when searching for this symbol".  As discussed earlier,
the search name would be normally be the natural name, but in the case
of Ada, would just be the linkage name.

The modification to the signature of symbol_natural_name is to allow the
option of delayed (lazy) evaluation of the demangled name, which is
actually the point of introducing search names.

Paul Hilfinger

2004-03-03  Paul N. Hilfinger  <hilfinger@gnat.com>

	* symtab.h (symbol_natural_name): Remove const qualification 
	from argument.
	(SYMBOL_SEARCH_NAME): New definition.
	(SYMBOL_MATCHES_SEARCH_NAME): New definition.
	(SYMBOL_DEMANGLED_SEARCH_NAME): New definition.	

	* dictionary.c (iter_name_first_hashed): Match on SYMBOL_SEARCH_NAME.
	(iter_name_next_hashed): Ditto.
	(iter_name_next_linear): Ditto.
	(insert_symbol_hashed): Hash on SYMBOL_SEARCH_NAME.

	* symtab.c (symbol_natural_name): Remove const qualification from
	argument to allow lazy evaluation of demangled name.
	(lookup_partial_symbol): Assume symbols ordered by search name, using
	SYMBOL_SEARCH_NAME and SYMBOL_MATCHES_SEARCH_NAME.
	
	* symfile.c (compare_psymbols): Order by SYMBOL_SEARCH_NAME.
	
	* minsyms.c (build_minimal_symbol_hash_tables): Use 
	SYMBOL_DEMANGLED_SEARCH_NAME to test for adding to demangled
	hash table.
	

Index: current-public.49/gdb/minsyms.c
--- current-public.49/gdb/minsyms.c Mon, 09 Feb 2004 23:40:20 -0800 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2 644)
+++ submit.31(w)/gdb/minsyms.c Wed, 03 Mar 2004 00:34:32 -0800 hilfingr (GdbPub/j/4_minsyms.c 1.1.1.3.1.1.1.1.1.1.1.1.1.1.3.2.1.1 644)
@@ -794,7 +794,7 @@ build_minimal_symbol_hash_tables (struct
       add_minsym_to_hash_table (msym, objfile->msymbol_hash);
 
       msym->demangled_hash_next = 0;
-      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
+      if (SYMBOL_DEMANGLED_SEARCH_NAME (msym) != NULL)
 	add_minsym_to_demangled_hash_table (msym,
                                             objfile->msymbol_demangled_hash);
     }
Index: current-public.49/gdb/symfile.c
--- current-public.49/gdb/symfile.c Tue, 02 Mar 2004 23:57:04 -0800 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1 644)
+++ submit.31(w)/gdb/symfile.c Wed, 03 Mar 2004 00:34:32 -0800 hilfingr (GdbPub/k/48_symfile.c 1.1.1.7.1.1.1.2.1.1.1.1.1.1.1.1.1.1.3.1.1.2.1.1.1.1.1.1 644)
@@ -208,8 +208,8 @@ compare_psymbols (const void *s1p, const
   struct partial_symbol *const *s1 = s1p;
   struct partial_symbol *const *s2 = s2p;
 
-  return strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*s1),
-			    SYMBOL_NATURAL_NAME (*s2));
+  return strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*s1),
+			    SYMBOL_SEARCH_NAME (*s2));
 }
 
 void
Index: current-public.49/gdb/symtab.c
--- current-public.49/gdb/symtab.c Mon, 23 Feb 2004 00:44:25 -0800 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1 644)
+++ submit.31(w)/gdb/symtab.c Wed, 03 Mar 2004 01:55:44 -0800 hilfingr (GdbPub/k/50_symtab.c 1.1.1.4.1.1.1.1.1.1.1.1.1.1.2.1.2.1.1.1.1.1.1.1.1.1 644)
@@ -630,7 +630,7 @@ symbol_init_demangled_name (struct gener
    demangling is necessary, this is the demangled name.  */
 
 char *
-symbol_natural_name (const struct general_symbol_info *gsymbol)
+symbol_natural_name (struct general_symbol_info *gsymbol)
 {
   if ((gsymbol->language == language_cplus
        || gsymbol->language == language_java
@@ -1467,7 +1467,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      do_linear_search = 1;
 	    }
-	  if (strcmp_iw_ordered (SYMBOL_NATURAL_NAME (*center), name) >= 0)
+	  if (strcmp_iw_ordered (SYMBOL_SEARCH_NAME (*center), name) >= 0)
 	    {
 	      top = center;
 	    }
@@ -1482,7 +1482,7 @@ lookup_partial_symbol (struct partial_sy
       while (top <= real_top
 	     && (linkage_name != NULL
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
-		 : SYMBOL_MATCHES_NATURAL_NAME (*top,name)))
+		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
 	  if (SYMBOL_DOMAIN (*top) == domain)
 	    {
@@ -1503,7 +1503,7 @@ lookup_partial_symbol (struct partial_sy
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
-		  : SYMBOL_MATCHES_NATURAL_NAME (*psym, name))
+		  : SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
 		{
 		  return (*psym);
 		}
Index: current-public.49/gdb/symtab.h
--- current-public.49/gdb/symtab.h Wed, 18 Feb 2004 01:34:45 -0800 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1 644)
+++ submit.31(w)/gdb/symtab.h Wed, 03 Mar 2004 01:55:59 -0800 hilfingr (GdbPub/l/2_symtab.h 1.1.1.5.1.1.1.1.1.1.1.1.1.1.3.1.1.1.1.1.1.1.1.1 644)
@@ -213,7 +213,7 @@ extern void symbol_set_names (struct gen
 
 #define SYMBOL_NATURAL_NAME(symbol) \
   (symbol_natural_name (&(symbol)->ginfo))
-extern char *symbol_natural_name (const struct general_symbol_info *symbol);
+extern char *symbol_natural_name (struct general_symbol_info *symbol);
 
 /* Return SYMBOL's name from the point of view of the linker.  In
    languages like C++ where symbols may be mangled for ease of
@@ -257,6 +257,22 @@ extern char *symbol_demangled_name (stru
 
 #define SYMBOL_MATCHES_NATURAL_NAME(symbol, name)			\
   (strcmp_iw (SYMBOL_NATURAL_NAME (symbol), (name)) == 0)
+
+/* Macro that returns the name to be used when sorting and searching symbols. 
+   In most languages, we search for the demangled form of a name,
+   and so sort symbols accordingly. */
+#define SYMBOL_SEARCH_NAME(symbol)					 \
+  SYMBOL_NATURAL_NAME (symbol)
+
+/* Analogous to SYMBOL_MATCHES_NATURAL_NAME, but uses the search
+   name. */
+#define SYMBOL_MATCHES_SEARCH_NAME(symbol, name)			\
+  (strcmp_iw (SYMBOL_SEARCH_NAME (symbol), (name)) == 0)
+
+/* For languages that do not demangle or that do not do searches by 
+   demangled name, NULL.  Otherwise, the demangled name. */
+#define SYMBOL_DEMANGLED_SEARCH_NAME(symbol)				 \
+  SYMBOL_DEMANGLED_NAME (symbol)
 
 /* Classification types for a minimal symbol.  These should be taken as
    "advisory only", since if gdb can't easily figure out a
Index: current-public.49/gdb/dictionary.c
--- current-public.49/gdb/dictionary.c Tue, 17 Jun 2003 02:41:56 -0700 hilfingr (GdbPub/L/b/35_dictionary 1.1 644)
+++ submit.31(w)/gdb/dictionary.c Wed, 03 Mar 2004 00:34:33 -0800 hilfingr (GdbPub/L/b/35_dictionary 1.1.4.1 644)
@@ -636,7 +636,7 @@ iter_name_first_hashed (const struct dic
        sym = sym->hash_next)
     {
       /* Warning: the order of arguments to strcmp_iw matters!  */
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  break;
 	}
@@ -656,7 +656,7 @@ iter_name_next_hashed (const char *name,
        next != NULL;
        next = next->hash_next)
     {
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (next), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (next), name) == 0)
 	break;
     }
 
@@ -674,7 +674,7 @@ insert_symbol_hashed (struct dictionary 
   unsigned int hash_index;
   struct symbol **buckets = DICT_HASHED_BUCKETS (dict);
 
-  hash_index = (msymbol_hash_iw (SYMBOL_NATURAL_NAME (sym))
+  hash_index = (msymbol_hash_iw (SYMBOL_SEARCH_NAME (sym))
 		% DICT_HASHED_NBUCKETS (dict));
   sym->hash_next = buckets[hash_index];
   buckets[hash_index] = sym;
@@ -789,7 +789,7 @@ iter_name_next_linear (const char *name,
   for (i = DICT_ITERATOR_INDEX (iterator) + 1; i < nsyms; ++i)
     {
       sym = DICT_LINEAR_SYM (dict, i);
-      if (strcmp_iw (SYMBOL_NATURAL_NAME (sym), name) == 0)
+      if (strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0)
 	{
 	  retval = sym;
 	  break;