This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Do not add partial_symbol again and again to the list
- From: Aleksandar Ristovski <aristovski at qnx dot com>
- To: gdb-patches at sourceware dot org, Daniel Jacobowitz <drow at false dot org>
- Cc: Aleksandar Ristovski <aristovski at qnx dot com>
- Date: Mon, 05 May 2008 14:58:53 -0400
- Subject: Re: [patch] Do not add partial_symbol again and again to the list
- Newsgroups: gmane.comp.gdb.patches
- References: <20080211203809.GA29560@caradoc.them.org> <47B0B56F.4010607@qnx.com> <20080211210935.GA31767@caradoc.them.org> <47B0C0F4.4090302@qnx.com> <20080211214750.GA1953@caradoc.them.org> <47B0C7C6.9090605@qnx.com> <20080211223056.GA3833@caradoc.them.org> <47B0CF8A.6080306@qnx.com> <20080211225314.GA5832@caradoc.them.org> <47B27EB1.6030606@qnx.com> <20080503205413.GA22704@caradoc.them.org>
Daniel Jacobowitz wrote:
* bcache.c (bcache_data): Call bcache_added function.
(bcache_added): New function name. Body of function bcache_data
is used here with the addition of 'added' argument. The argument
is optional and if specified, bcache_added returns 1 if the data
was added and 0 if it was found in the hash.
Whenever you find yourself writing this sort of explanation in the
changelog, it's a sign that you should have a shorter changelog entry
and a long comment in the source code. Please move the explanation to
bcache.h.
Comments added.
+const void *
+bcache (const void *addr, int length, struct bcache *bcache)
+{
+ return bcache_data (addr, length, bcache);
+}
+
+void *
+bcache_added (const void *addr, int length, struct bcache *bcache,
+ int *added)
It should return a const pointer, like bcache. Also the indentation
on the second line is too deep.
bcache_added is more like bcache_data which returns void*. It would make sense to return void const*
but then I would have to change const-ness in many places (too many: I would leave that for some
other patch).
case DW_TAG_namespace:
- add_psymbol_to_list (actual_name, strlen (actual_name),
+ add_psymbol_to_global_list (actual_name, strlen (actual_name),
VAR_DOMAIN, LOC_TYPEDEF,
- &objfile->global_psymbols,
0, (CORE_ADDR) 0, cu->language, objfile);
Please adjust indentation on the subsequent lines, since the open
paren moved over.
Done.
case DW_TAG_enumerator:
-j add_psymbol_to_list (actual_name, strlen (actual_name),
+ if (cu->language == language_cplus
+ || cu->language == language_java)
+ add_psymbol_to_global_list (actual_name, strlen (actual_name),
+ VAR_DOMAIN, LOC_CONST,
+ 0, (CORE_ADDR) 0, cu->language, objfile);
This part I don't understand. Why does the behavior of enumerators
depend on the language? The logic for finding them during lookup is
the same.
I must admit that I am confused a bit too. I followed the existing logic
but now that you mentioned it, I realized I do not have a good explanation.
I am inclined a bit to store them to the static list for C++, but for now
I would just leave it as-is.
In fact, if lookup_partial_symbol will never find a global symbol
with an identical name why do we need them for any duplicated symbol,
even legitimately duplicated things like static functions?
I don't think static functions are a good example since they would not
go into the global list anyway (see dwarf2read.c, case DW_TAG_subprogram).
The attached is revised patch with the changes above; additionally, it
calls new add_partial_symbol_to_global_list for all partial symbols that
are being added to the global list.
Thanks,
Aleksandar Ristovski
QNX Software Systems
ChangeLog:
* bcache.c (bcache_data): Call bcache_added function.
(bcache_added): New function name. Body of function bcache_data
is used here with the addition of 'added' argument.
* bcache.h (bcache_added): New function.
* dwarf2read.c (add_partial_symbol): Make call to
new function add_psymbol_to_global_list for partial symbols that
are being added to the global list.
(load_partial_dies): Likewise.
* symfile.c (add_psymbol_to_bcache): New helper function, takes part of
work from add_psymbol_to_list - initialises partial symbol and stashes
it in objfile's cache.
(append_psymbol_to_list): New helper function, takes other part of
work from add_psymbol_to_list - adds partial symbol to the given list.
(add_psymbol_to_list): Call helper functions instead of doing work
here. Functionally, the function hasn't changed.
(add_psymbol_to_global_list): New function, adds partial symbol to
global list and does it only once per objfile.
* symfile.h (add_psymbol_to_global_list): New function.
Index: gdb/bcache.c
===================================================================
RCS file: /cvs/src/src/gdb/bcache.c,v
retrieving revision 1.20
diff -u -p -r1.20 bcache.c
--- gdb/bcache.c 1 Jan 2008 22:53:09 -0000 1.20
+++ gdb/bcache.c 5 May 2008 18:49:56 -0000
@@ -197,11 +197,40 @@ expand_hash_table (struct bcache *bcache
static void *
bcache_data (const void *addr, int length, struct bcache *bcache)
{
+ return bcache_added (addr, length, bcache, NULL);
+}
+
+
+void *
+deprecated_bcache (const void *addr, int length, struct bcache *bcache)
+{
+ return bcache_data (addr, length, bcache);
+}
+
+const void *
+bcache (const void *addr, int length, struct bcache *bcache)
+{
+ return bcache_data (addr, length, bcache);
+}
+
+/* Find a copy of the LENGTH bytes at ADDR in BCACHE. If BCACHE has
+ never seen those bytes before, add a copy of them to BCACHE. In
+ either case, return a pointer to BCACHE's copy of that string. If
+ optional ADDED is not NULL, return 1 in case of new entry or 0 if
+ returning an old entry. */
+
+void *
+bcache_added (const void *addr, int length, struct bcache *bcache,
+ int *added)
+{
unsigned long full_hash;
unsigned short half_hash;
int hash_index;
struct bstring *s;
+ if (added)
+ *added = 0;
+
/* If our average chain length is too high, expand the hash table. */
if (bcache->unique_count >= bcache->num_buckets * CHAIN_LENGTH_THRESHOLD)
expand_hash_table (bcache);
@@ -242,21 +271,12 @@ bcache_data (const void *addr, int lengt
bcache->unique_size += length;
bcache->structure_size += BSTRING_SIZE (length);
+ if (added)
+ *added = 1;
+
return &new->d.data;
}
}
-
-void *
-deprecated_bcache (const void *addr, int length, struct bcache *bcache)
-{
- return bcache_data (addr, length, bcache);
-}
-
-const void *
-bcache (const void *addr, int length, struct bcache *bcache)
-{
- return bcache_data (addr, length, bcache);
-}
/* Allocating and freeing bcaches. */
Index: gdb/bcache.h
===================================================================
RCS file: /cvs/src/src/gdb/bcache.h,v
retrieving revision 1.12
diff -u -p -r1.12 bcache.h
--- gdb/bcache.h 1 Jan 2008 22:53:09 -0000 1.12
+++ gdb/bcache.h 5 May 2008 18:49:56 -0000
@@ -150,6 +150,8 @@ extern void *deprecated_bcache (const vo
extern const void *bcache (const void *addr, int length,
struct bcache *bcache);
+extern void *bcache_added (const void *addr, int length,
+ struct bcache *bcache, int *added);
/* Free all the storage used by BCACHE. */
extern void bcache_xfree (struct bcache *bcache);
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.263
diff -u -p -r1.263 dwarf2read.c
--- gdb/dwarf2read.c 5 May 2008 14:37:32 -0000 1.263
+++ gdb/dwarf2read.c 5 May 2008 18:49:57 -0000
@@ -1964,11 +1964,10 @@ add_partial_symbol (struct partial_die_i
in the global scope. */
/*prim_record_minimal_symbol (actual_name, pdi->lowpc + baseaddr,
mst_text, objfile); */
- psym = add_psymbol_to_list (actual_name, strlen (actual_name),
- VAR_DOMAIN, LOC_BLOCK,
- &objfile->global_psymbols,
- 0, pdi->lowpc + baseaddr,
- cu->language, objfile);
+ psym = add_psymbol_to_global_list (actual_name, strlen (actual_name),
+ VAR_DOMAIN, LOC_BLOCK,
+ 0, pdi->lowpc + baseaddr,
+ cu->language, objfile);
}
else
{
@@ -2000,11 +1999,11 @@ add_partial_symbol (struct partial_die_i
if (pdi->locdesc)
addr = decode_locdesc (pdi->locdesc, cu);
if (pdi->locdesc || pdi->has_type)
- psym = add_psymbol_to_list (actual_name, strlen (actual_name),
- VAR_DOMAIN, LOC_STATIC,
- &objfile->global_psymbols,
- 0, addr + baseaddr,
- cu->language, objfile);
+ psym = add_psymbol_to_global_list (actual_name,
+ strlen (actual_name),
+ VAR_DOMAIN, LOC_STATIC,
+ 0, addr + baseaddr,
+ cu->language, objfile);
}
else
{
@@ -2034,10 +2033,9 @@ add_partial_symbol (struct partial_die_i
0, (CORE_ADDR) 0, cu->language, objfile);
break;
case DW_TAG_namespace:
- add_psymbol_to_list (actual_name, strlen (actual_name),
- VAR_DOMAIN, LOC_TYPEDEF,
- &objfile->global_psymbols,
- 0, (CORE_ADDR) 0, cu->language, objfile);
+ add_psymbol_to_global_list (actual_name, strlen (actual_name),
+ VAR_DOMAIN, LOC_TYPEDEF,
+ 0, (CORE_ADDR) 0, cu->language, objfile);
break;
case DW_TAG_class_type:
case DW_TAG_interface_type:
@@ -2058,23 +2056,28 @@ add_partial_symbol (struct partial_die_i
/* NOTE: carlton/2003-10-07: See comment in new_symbol about
static vs. global. */
- add_psymbol_to_list (actual_name, strlen (actual_name),
- STRUCT_DOMAIN, LOC_TYPEDEF,
- (cu->language == language_cplus
- || cu->language == language_java)
- ? &objfile->global_psymbols
- : &objfile->static_psymbols,
- 0, (CORE_ADDR) 0, cu->language, objfile);
-
+ if (cu->language == language_cplus
+ || cu->language == language_java)
+ add_psymbol_to_global_list (actual_name, strlen (actual_name),
+ STRUCT_DOMAIN, LOC_TYPEDEF,
+ 0, (CORE_ADDR) 0, cu->language, objfile);
+ else
+ add_psymbol_to_list (actual_name, strlen (actual_name),
+ STRUCT_DOMAIN, LOC_TYPEDEF,
+ &objfile->static_psymbols,
+ 0, (CORE_ADDR) 0, cu->language, objfile);
break;
case DW_TAG_enumerator:
- add_psymbol_to_list (actual_name, strlen (actual_name),
- VAR_DOMAIN, LOC_CONST,
- (cu->language == language_cplus
- || cu->language == language_java)
- ? &objfile->global_psymbols
- : &objfile->static_psymbols,
- 0, (CORE_ADDR) 0, cu->language, objfile);
+ if (cu->language == language_cplus
+ || cu->language == language_java)
+ add_psymbol_to_global_list (actual_name, strlen (actual_name),
+ VAR_DOMAIN, LOC_CONST,
+ 0, (CORE_ADDR) 0, cu->language, objfile);
+ else
+ add_psymbol_to_list (actual_name, strlen (actual_name),
+ VAR_DOMAIN, LOC_CONST,
+ &objfile->static_psymbols,
+ 0, (CORE_ADDR) 0, cu->language, objfile);
break;
default:
break;
@@ -5638,13 +5641,21 @@ load_partial_dies (bfd *abfd, gdb_byte *
if (part_die->name == NULL)
complaint (&symfile_complaints, _("malformed enumerator DIE ignored"));
else if (building_psymtab)
- add_psymbol_to_list (part_die->name, strlen (part_die->name),
- VAR_DOMAIN, LOC_CONST,
- (cu->language == language_cplus
- || cu->language == language_java)
- ? &cu->objfile->global_psymbols
- : &cu->objfile->static_psymbols,
- 0, (CORE_ADDR) 0, cu->language, cu->objfile);
+ {
+ if (cu->language == language_cplus
+ || cu->language == language_java)
+ add_psymbol_to_global_list (part_die->name,
+ strlen (part_die->name),
+ VAR_DOMAIN, LOC_CONST,
+ 0, (CORE_ADDR) 0,
+ cu->language, cu->objfile);
+ else
+ add_psymbol_to_list (part_die->name, strlen (part_die->name),
+ VAR_DOMAIN, LOC_CONST,
+ &cu->objfile->static_psymbols,
+ 0, (CORE_ADDR) 0,
+ cu->language, cu->objfile);
+ }
info_ptr = locate_pdi_sibling (part_die, info_ptr, abfd, cu);
continue;
Index: gdb/symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.203
diff -u -p -r1.203 symfile.c
--- gdb/symfile.c 5 May 2008 16:13:49 -0000 1.203
+++ gdb/symfile.c 5 May 2008 18:49:57 -0000
@@ -3082,6 +3082,68 @@ start_psymtab_common (struct objfile *ob
return (psymtab);
}
+/* Helper function, initialises partial symbol structure and stashes
+ it into objfile's bcache. Note that our caching mechanism will
+ use all fields of struct partial_symbol to determine hash value of the
+ structure. In other words, having two symbols with the same name but
+ different domain (or address) is possible and correct. */
+
+static struct partial_symbol *
+add_psymbol_to_bcache (char *name, int namelength, domain_enum domain,
+ enum address_class class,
+ long val, /* Value as a long */
+ CORE_ADDR coreaddr, /* Value as a CORE_ADDR */
+ enum language language, struct objfile *objfile,
+ int *added)
+{
+ char *buf = name;
+ /* psymbol is static so that there will be no uninitialized gaps in the
+ structure which might contain random data, causing cache misses in
+ bcache. */
+ static struct partial_symbol psymbol;
+
+ if (name[namelength] != '\0')
+ {
+ buf = alloca (namelength + 1);
+ /* Create local copy of the partial symbol */
+ memcpy (buf, name, namelength);
+ buf[namelength] = '\0';
+ }
+ /* val and coreaddr are mutually exclusive, one of them *will* be zero */
+ if (val != 0)
+ {
+ SYMBOL_VALUE (&psymbol) = val;
+ }
+ else
+ {
+ SYMBOL_VALUE_ADDRESS (&psymbol) = coreaddr;
+ }
+ SYMBOL_SECTION (&psymbol) = 0;
+ SYMBOL_LANGUAGE (&psymbol) = language;
+ PSYMBOL_DOMAIN (&psymbol) = domain;
+ PSYMBOL_CLASS (&psymbol) = class;
+
+ SYMBOL_SET_NAMES (&psymbol, buf, namelength, objfile);
+
+ /* Stash the partial symbol away in the cache */
+ return bcache_added (&psymbol, sizeof (struct partial_symbol),
+ objfile->psymbol_cache, added);
+}
+
+/* Helper function, adds partial symbol to the given partial symbol
+ list. */
+
+static void
+append_psymbol_to_list (struct psymbol_allocation_list *list,
+ struct partial_symbol *psym,
+ struct objfile *objfile)
+{
+ if (list->next >= list->list + list->size)
+ extend_psymbol_list (list, objfile);
+ *list->next++ = psym;
+ OBJSTAT (objfile, n_psyms++);
+}
+
/* Add a symbol with a long value to a psymtab.
Since one arg is a struct, we pass in a ptr and deref it (sigh).
Return the partial symbol that has been added. */
@@ -3105,43 +3167,37 @@ add_psymbol_to_list (char *name, int nam
enum language language, struct objfile *objfile)
{
struct partial_symbol *psym;
- char *buf = alloca (namelength + 1);
- /* psymbol is static so that there will be no uninitialized gaps in the
- structure which might contain random data, causing cache misses in
- bcache. */
- static struct partial_symbol psymbol;
+ /* Stash the partial symbol away in the cache */
+ psym = add_psymbol_to_bcache (name, namelength, domain, class,
+ val, coreaddr, language, objfile, NULL);
- /* Create local copy of the partial symbol */
- memcpy (buf, name, namelength);
- buf[namelength] = '\0';
- /* val and coreaddr are mutually exclusive, one of them *will* be zero */
- if (val != 0)
- {
- SYMBOL_VALUE (&psymbol) = val;
- }
- else
- {
- SYMBOL_VALUE_ADDRESS (&psymbol) = coreaddr;
- }
- SYMBOL_SECTION (&psymbol) = 0;
- SYMBOL_LANGUAGE (&psymbol) = language;
- PSYMBOL_DOMAIN (&psymbol) = domain;
- PSYMBOL_CLASS (&psymbol) = class;
+ /* Save pointer to partial symbol in psymtab, growing symtab if needed. */
+ append_psymbol_to_list (list, psym, objfile);
+ return psym;
+}
- SYMBOL_SET_NAMES (&psymbol, buf, namelength, objfile);
+/* Add partial symbol to OBJFILE's global list of partial symbols.
+ Make sure we do not duplicate this information. See add_psymbol_to_bcache
+ for more details. */
- /* Stash the partial symbol away in the cache */
- psym = deprecated_bcache (&psymbol, sizeof (struct partial_symbol),
- objfile->psymbol_cache);
+const struct partial_symbol *
+add_psymbol_to_global_list (char *name, int namelength, domain_enum domain,
+ enum address_class class,
+ long val, /* Value as a long */
+ CORE_ADDR coreaddr, /* Value as a CORE_ADDR */
+ enum language language, struct objfile *objfile)
+{
+ struct partial_symbol *psym;
+ int added;
+
+ psym = add_psymbol_to_bcache (name, namelength, domain, class, val,
+ coreaddr, language, objfile, &added);
- /* Save pointer to partial symbol in psymtab, growing symtab if needed. */
- if (list->next >= list->list + list->size)
- {
- extend_psymbol_list (list, objfile);
- }
- *list->next++ = psym;
- OBJSTAT (objfile, n_psyms++);
+ if (!added)
+ return psym;
+ /* Save pointer to partial symbol in psymtab, growing symtab if needed. */
+ append_psymbol_to_list (&objfile->global_psymbols, psym, objfile);
return psym;
}
Index: gdb/symfile.h
===================================================================
RCS file: /cvs/src/src/gdb/symfile.h,v
retrieving revision 1.46
diff -u -p -r1.46 symfile.h
--- gdb/symfile.h 3 Feb 2008 22:13:29 -0000 1.46
+++ gdb/symfile.h 5 May 2008 18:49:57 -0000
@@ -199,6 +199,13 @@ struct partial_symbol *add_psymbol_to_li
long, CORE_ADDR,
enum language, struct objfile *);
+extern const
+struct partial_symbol *add_psymbol_to_global_list (char *, int, domain_enum,
+ enum address_class,
+ long, CORE_ADDR,
+ enum language,
+ struct objfile *);
+
extern void init_psymbol_list (struct objfile *, int);
extern void sort_pst_symbols (struct partial_symtab *);