From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1431 invoked by alias); 1 Sep 2010 21:44:08 -0000 Received: (qmail 1423 invoked by uid 22791); 1 Sep 2010 21:44:06 -0000 X-SWARE-Spam-Status: No, hits=-5.3 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 01 Sep 2010 21:44:00 +0000 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o81LhwSf024307 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 1 Sep 2010 17:43:58 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o81LhwAT030969; Wed, 1 Sep 2010 17:43:58 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id o81Lhviw001833; Wed, 1 Sep 2010 17:43:57 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 0B64E379677; Wed, 1 Sep 2010 15:43:56 -0600 (MDT) From: Tom Tromey To: gdb-patches@sourceware.org Subject: FYI: change type searching Date: Wed, 01 Sep 2010 22:22:00 -0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-09/txt/msg00067.txt.bz2 I'm checking this in. I noticed a bad bug when using a DWARF index. If you search for the type "char" (in my case it was being done via gdb.lookup_type), gdb would happily expand all symtabs before returning the answer. This happens because lookup_typename uses lookup_symbol, which first searches for public symbols. The index doesn't record this bit of info, and since "char" is defined pretty much everywhere, it eagerly expands each symtab, only to find that the public "char" isn't defined there. All of this searching is pointless, though, because types are made static, not public. This patch fixes the situation first by changing the quick method to return on the first match, and second by avoiding the useless lookups when searching for a type. IMO it would be nice if we had a domain specifically for types. I looked at this a little but it seems pretty invasive, so I didn't attempt it. Built and regtested on x86-64 (compile farm). I also ran gdb locally to verify that the eager expansion no longer happens. Tom 2010-09-01 Tom Tromey * symtab.h (lookup_type_symbol): Declare. * symtab.c (lookup_symbol_in_language_full): Rename from lookup_symbol_in_language. Add 'for_type' argument. (lookup_symbol_in_language): New function. (lookup_type_symbol): Likewise. (lookup_symbol_aux): Add 'for_type' argument. (match_symbol_aux): New function. (lookup_symbol_aux_symtabs): Use expand_one_symtab_matching. (match_transparent_type): New function. (basic_lookup_transparent_type): Use expand_one_symtab_matching. * symfile.h (struct quick_symbol_functions) : Remove. : New field. * psymtab.c (expand_one_symtab_matching_psymtabs): New function. (pre_expand_symtabs_matching_psymtabs): Remove. (psym_functions): Update. * gdbtypes.c (lookup_typename): Use lookup_type_symbol. * dwarf2read.c (dw2_lookup_symbol): Update comment. (dw2_pre_expand_symtabs_matching): Remove. (dw2_expand_one_symtab_matching): New function. (dwarf2_gdb_index_functions): Update. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 78491c8..c5dc1a5 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -2140,7 +2140,7 @@ static struct symtab * dw2_lookup_symbol (struct objfile *objfile, int block_index, const char *name, domain_enum domain) { - /* We do all the work in the pre_expand_symtabs_matching hook + /* We do all the work in the expand_one_symtab_matching hook instead. */ return NULL; } @@ -2171,12 +2171,46 @@ dw2_do_expand_symtabs_matching (struct objfile *objfile, const char *name) } } -static void -dw2_pre_expand_symtabs_matching (struct objfile *objfile, - int kind, const char *name, - domain_enum domain) +static struct symbol * +dw2_expand_one_symtab_matching (struct objfile *objfile, + int kind, const char *name, + domain_enum domain, + struct symbol *(*matcher) (struct symtab *, + int, + const char *, + domain_enum, + void *), + void *data) { - dw2_do_expand_symtabs_matching (objfile, name); + dw2_setup (objfile); + + if (dwarf2_per_objfile->index_table) + { + offset_type *vec; + + if (find_slot_in_mapped_hash (dwarf2_per_objfile->index_table, + name, &vec)) + { + offset_type i, len = MAYBE_SWAP (*vec); + for (i = 0; i < len; ++i) + { + offset_type cu_index = MAYBE_SWAP (vec[i + 1]); + struct dwarf2_per_cu_data *cu = dw2_get_cu (cu_index); + struct symtab *symtab; + struct symbol *sym; + + if (cu->v.quick->symtab) + continue; + + symtab = dw2_instantiate_symtab (objfile, cu); + sym = matcher (symtab, kind, name, domain, data); + if (sym) + return sym; + } + } + } + + return NULL; } static void @@ -2479,7 +2513,7 @@ const struct quick_symbol_functions dwarf2_gdb_index_functions = dw2_forget_cached_source_info, dw2_lookup_symtab, dw2_lookup_symbol, - dw2_pre_expand_symtabs_matching, + dw2_expand_one_symtab_matching, dw2_print_stats, dw2_dump, dw2_relocate, @@ -5940,9 +5974,9 @@ dwarf2_attach_fields_to_type (struct field_info *fip, struct type *type, (B_TYPE *) TYPE_ALLOC (type, B_BYTES (nfields)); B_CLRALL (TYPE_FIELD_PROTECTED_BITS (type), nfields); - TYPE_FIELD_IGNORE_BITS (type) = - (B_TYPE *) TYPE_ALLOC (type, B_BYTES (nfields)); - B_CLRALL (TYPE_FIELD_IGNORE_BITS (type), nfields); + /* We don't set TYPE_FIELD_IGNORE_BITS here. The DWARF reader + never sets any bits in that array, so leaving it NULL lets us + save a little memory. */ } /* If the type has baseclasses, allocate and clear a bit vector for diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index da4d673..882d245 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -1051,7 +1051,7 @@ lookup_typename (const struct language_defn *language, struct symbol *sym; struct type *tmp; - sym = lookup_symbol (name, block, VAR_DOMAIN, 0); + sym = lookup_type_symbol (name, block, VAR_DOMAIN, language->la_language); if (sym == NULL || SYMBOL_CLASS (sym) != LOC_TYPEDEF) { tmp = language_lookup_primitive_type_by_name (language, gdbarch, name); diff --git a/gdb/psymtab.c b/gdb/psymtab.c index bc47681..44ccb0f 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -421,12 +421,18 @@ lookup_symbol_aux_psymtabs (struct objfile *objfile, return NULL; } -static void -pre_expand_symtabs_matching_psymtabs (struct objfile *objfile, - int kind, const char *name, - domain_enum domain) +static struct symbol * +expand_one_symtab_matching_psymtabs (struct objfile *objfile, + int kind, const char *name, + domain_enum domain, + struct symbol *(*matcher) (struct symtab *, + int, + const char *, + domain_enum, + void *), + void *data) { - /* Nothing. */ + return NULL; } /* Look, in partial_symtab PST, for symbol whose natural name is NAME. @@ -1207,7 +1213,7 @@ const struct quick_symbol_functions psym_functions = forget_cached_source_info_partial, lookup_symtab_via_partial_symtab, lookup_symbol_aux_psymtabs, - pre_expand_symtabs_matching_psymtabs, + expand_one_symtab_matching_psymtabs, print_psymtab_stats_for_objfile, dump_psymtabs_for_objfile, relocate_psymtabs, diff --git a/gdb/symfile.h b/gdb/symfile.h index 5815354..84b770d 100644 --- a/gdb/symfile.h +++ b/gdb/symfile.h @@ -171,14 +171,25 @@ struct quick_symbol_functions int kind, const char *name, domain_enum domain); - /* This is called to expand symbol tables before looking up a - symbol. A backend can choose to implement this and then have its - `lookup_symbol' hook always return NULL, or the reverse. (It - doesn't make sense to implement both.) The arguments are as for - `lookup_symbol'. */ - void (*pre_expand_symtabs_matching) (struct objfile *objfile, - int kind, const char *name, - domain_enum domain); + /* Expand each symbol table in OBJFILE that may have items matching + KIND, NAME, and DOMAIN -- these arguments are as for + `lookup_symbol'. For each such symbol table, call MATCHER with + the symbol table and DATA arguments. If MATCHER returns NULL, + keep going. Otherwise, return the result of MATCHER. If MATCHER + never returns non-NULL, return NULL. A backend can choose to + implement this and then have its `lookup_symbol' hook always + return NULL, or the reverse. (It doesn't make sense to implement + both.) */ + struct symbol *(*expand_one_symtab_matching) + (struct objfile *objfile, + int kind, const char *name, + domain_enum domain, + struct symbol *(*matcher) (struct symtab *symtab, + int kind, + const char *name, + domain_enum domain, + void *data), + void *data); /* Print statistics about any indices loaded for OBJFILE. The statistics should be printed to gdb_stdout. This is used for diff --git a/gdb/symtab.c b/gdb/symtab.c index d43d573..5bcd315 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -91,7 +91,8 @@ static struct symbol *lookup_symbol_aux (const char *name, const struct block *block, const domain_enum domain, enum language language, - int *is_a_field_of_this); + int *is_a_field_of_this, + int for_type); static struct symbol *lookup_symbol_aux_local (const char *name, @@ -994,6 +995,8 @@ fixup_symbol_section (struct symbol *sym, struct objfile *objfile) C++: if IS_A_FIELD_OF_THIS is nonzero on entry, check to see if NAME is a field of the current implied argument `this'. If so set *IS_A_FIELD_OF_THIS to 1, otherwise set it to zero. + FOR_TYPE is non-zero if searching specifically for a type; zero + otherwise. BLOCK_FOUND is set to the block in which NAME is found (in the case of a field of `this', value_of_this sets BLOCK_FOUND to the proper value.) */ @@ -1007,10 +1010,10 @@ fixup_symbol_section (struct symbol *sym, struct objfile *objfile) variable and thus can probably assume it will never hit the C++ code). */ -struct symbol * -lookup_symbol_in_language (const char *name, const struct block *block, - const domain_enum domain, enum language lang, - int *is_a_field_of_this) +static struct symbol * +lookup_symbol_in_language_full (const char *name, const struct block *block, + const domain_enum domain, enum language lang, + int *is_a_field_of_this, int for_type) { char *demangled_name = NULL; const char *modified_name = NULL; @@ -1075,12 +1078,41 @@ lookup_symbol_in_language (const char *name, const struct block *block, } returnval = lookup_symbol_aux (modified_name, block, domain, lang, - is_a_field_of_this); + is_a_field_of_this, for_type); do_cleanups (cleanup); return returnval; } +/* Find the definition for a specified symbol name NAME + in domain DOMAIN, visible from lexical block BLOCK. + Returns the struct symbol pointer, or zero if no symbol is found. + C++: if IS_A_FIELD_OF_THIS is nonzero on entry, check to see if + NAME is a field of the current implied argument `this'. If so set + *IS_A_FIELD_OF_THIS to 1, otherwise set it to zero. + BLOCK_FOUND is set to the block in which NAME is found (in the case of + a field of `this', value_of_this sets BLOCK_FOUND to the proper value.) */ + +struct symbol * +lookup_symbol_in_language (const char *name, const struct block *block, + const domain_enum domain, enum language lang, + int *is_a_field_of_this) +{ + return lookup_symbol_in_language_full (name, block, domain, lang, + is_a_field_of_this, 0); +} + +/* Like lookup_symbol_in_language, but search specifically for a + type. */ + +struct symbol * +lookup_type_symbol (const char *name, const struct block *block, + const domain_enum domain, enum language lang) +{ + return lookup_symbol_in_language_full (name, block, domain, lang, + NULL, 1); +} + /* Behave like lookup_symbol_in_language, but performed with the current language. */ @@ -1101,7 +1133,8 @@ lookup_symbol (const char *name, const struct block *block, static struct symbol * lookup_symbol_aux (const char *name, const struct block *block, const domain_enum domain, enum language language, - int *is_a_field_of_this) + int *is_a_field_of_this, + int for_type) { struct symbol *sym; const struct language_defn *langdef; @@ -1165,14 +1198,20 @@ lookup_symbol_aux (const char *name, const struct block *block, } /* Now do whatever is appropriate for LANGUAGE to look - up static and global variables. */ + up static and global variables. If we are searching for a type, + we bypass this lookup, because types aren't global. */ - sym = langdef->la_lookup_symbol_nonlocal (name, block, domain); - if (sym != NULL) - return sym; + if (!for_type) + { + sym = langdef->la_lookup_symbol_nonlocal (name, block, domain); + if (sym != NULL) + return sym; + } - /* Now search all static file-level symbols. Not strictly correct, - but more useful than an error. */ + /* Now search all static file-level symbols. When searching for a + type, this is what we generally want, because types are put into + the file scope. For other objects, not strictly correct, but + more useful than an error. */ return lookup_static_symbol_aux (name, domain); } @@ -1327,6 +1366,35 @@ lookup_global_symbol_from_objfile (const struct objfile *main_objfile, return NULL; } +/* A helper for lookup_symbol_aux_symtabs that is passed as a callback + to the expand_one_symtab_matching quick function. */ + +static struct symbol * +match_symbol_aux (struct symtab *symtab, + int kind, const char *name, domain_enum domain, + void *arg) +{ + struct objfile *objfile = arg; + + if (symtab->primary) + { + struct symbol *sym; + struct blockvector *bv; + const struct block *block; + + bv = BLOCKVECTOR (symtab); + block = BLOCKVECTOR_BLOCK (bv, kind); + sym = lookup_block_symbol (block, name, domain); + if (sym) + { + block_found = block; + return fixup_symbol_section (sym, objfile); + } + } + + return NULL; +} + /* Check to see if the symbol is defined in one of the symtabs. BLOCK_INDEX should be either GLOBAL_BLOCK or STATIC_BLOCK, depending on whether or not we want to search global symbols or @@ -1344,11 +1412,6 @@ lookup_symbol_aux_symtabs (int block_index, const char *name, ALL_OBJFILES (objfile) { - if (objfile->sf) - objfile->sf->qf->pre_expand_symtabs_matching (objfile, - block_index, - name, domain); - ALL_OBJFILE_SYMTABS (objfile, s) if (s->primary) { @@ -1361,6 +1424,17 @@ lookup_symbol_aux_symtabs (int block_index, const char *name, return fixup_symbol_section (sym, objfile); } } + + if (objfile->sf) + { + sym = objfile->sf->qf->expand_one_symtab_matching (objfile, + block_index, + name, domain, + match_symbol_aux, + objfile); + if (sym) + return sym; + } } return NULL; @@ -1582,6 +1656,30 @@ basic_lookup_transparent_type_quick (struct objfile *objfile, int kind, return NULL; } +/* A helper function for basic_lookup_transparent_type that is passed + to the expand_one_symtab_matching quick function. */ + +static struct symbol * +match_transparent_type (struct symtab *symtab, + int kind, const char *name, domain_enum domain, + void *data) +{ + if (symtab->primary) + { + struct blockvector *bv; + struct block *block; + struct symbol *sym; + + bv = BLOCKVECTOR (symtab); + block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK); + sym = lookup_block_symbol (block, name, STRUCT_DOMAIN); + if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))) + return sym; + } + + return NULL; +} + /* The standard implementation of lookup_transparent_type. This code was modeled on lookup_symbol -- the parts not relevant to looking up types were just left out. In particular it's assumed here that @@ -1605,11 +1703,6 @@ basic_lookup_transparent_type (const char *name) ALL_OBJFILES (objfile) { - if (objfile->sf) - objfile->sf->qf->pre_expand_symtabs_matching (objfile, - GLOBAL_BLOCK, - name, STRUCT_DOMAIN); - ALL_OBJFILE_SYMTABS (objfile, s) if (s->primary) { @@ -1621,6 +1714,18 @@ basic_lookup_transparent_type (const char *name) return SYMBOL_TYPE (sym); } } + + if (objfile->sf) + { + sym + = objfile->sf->qf->expand_one_symtab_matching (objfile, + GLOBAL_BLOCK, name, + STRUCT_DOMAIN, + match_transparent_type, + NULL); + if (sym) + return SYMBOL_TYPE (sym); + } } ALL_OBJFILES (objfile) @@ -1640,10 +1745,6 @@ basic_lookup_transparent_type (const char *name) ALL_OBJFILES (objfile) { - if (objfile->sf) - objfile->sf->qf->pre_expand_symtabs_matching (objfile, STATIC_BLOCK, - name, STRUCT_DOMAIN); - ALL_OBJFILE_SYMTABS (objfile, s) { bv = BLOCKVECTOR (s); @@ -1654,6 +1755,18 @@ basic_lookup_transparent_type (const char *name) return SYMBOL_TYPE (sym); } } + + if (objfile->sf) + { + sym + = objfile->sf->qf->expand_one_symtab_matching (objfile, + STATIC_BLOCK, name, + STRUCT_DOMAIN, + match_transparent_type, + NULL); + if (sym) + return SYMBOL_TYPE (sym); + } } ALL_OBJFILES (objfile) diff --git a/gdb/symtab.h b/gdb/symtab.h index 04cb443..f96c3f4 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -977,6 +977,10 @@ extern int find_pc_line_pc_range (CORE_ADDR, CORE_ADDR *, CORE_ADDR *); extern void reread_symbols (void); +extern struct symbol *lookup_type_symbol (const char* name, + const struct block *block, + const domain_enum domain, + enum language lang); extern struct type *lookup_transparent_type (const char *); extern struct type *basic_lookup_transparent_type (const char *);