From patchwork Mon Nov 10 03:27:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 3632 Received: (qmail 32348 invoked by alias); 10 Nov 2014 03:28:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 32332 invoked by uid 89); 10 Nov 2014 03:28:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, KAM_STOCKGEN, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pa0-f48.google.com Received: from mail-pa0-f48.google.com (HELO mail-pa0-f48.google.com) (209.85.220.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 10 Nov 2014 03:28:39 +0000 Received: by mail-pa0-f48.google.com with SMTP id ey11so7280955pad.7 for ; Sun, 09 Nov 2014 19:28:37 -0800 (PST) X-Received: by 10.68.138.196 with SMTP id qs4mr29489014pbb.39.1415590117416; Sun, 09 Nov 2014 19:28:37 -0800 (PST) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id df1sm15054318pbb.2.2014.11.09.19.28.36 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 09 Nov 2014 19:28:36 -0800 (PST) From: Doug Evans To: gdb-patches@sourceware.org, keiths@redhat.com Subject: [PATCH] The result of symtab expansion is always a primary symtab Date: Sun, 09 Nov 2014 19:27:45 -0800 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes Hi. This patch is another step in adding clarity to the code. [The clearer the code the easier it is to make more significant changes.] The result of symtab expansion is always the primary symbol table. This observation was surprising to me at first, given the current data structures we use, but I can see it now (assuming I haven't missed anything of course), and chalk this up to another consequence of storing in one list both "symbol tables" (stored in the blockvector) and "line tables" (the file+line information in struct symtab). I tried to find a case where this isn't true, including going through every debug info reader, and can't, so I'm going ahead with this. This patch could use another pair of eyes. Here's an example of the simplification. This test in psymtab.c:lookup_symbol_aux_psymtabs is unnecessary: struct symtab *stab = psymtab_to_symtab (objfile, ps); ... if (stab->primary) I've gone through the history of this test. It was added to psymtab.c here https://sourceware.org/ml/gdb-patches/2010-06/msg00467.html and then I added a similar check to dwarf2read.c here https://sourceware.org/ml/gdb-patches/2012-12/msg00769.html I think just out of a sense of consistency. I believe the test was added out of a sense of conservatism, and lack of clarity in what the result of psymtab_to_symtab is. keiths: No need to spend time digging through your notes unless you want to. Regression tested on amd64-linux. It could use testing on non-ELF targets too. 2014-11-09 Doug Evans The result of symtab expansion is always a primary symtab. * dwarf2read.c (dw2_instantiate_symtab): Add assert. (dw2_lookup_symbol): Remove unnecessary test for primary symbol table. * psymtab.c (lookup_symbol_aux_psymtabs): Ditto. (psymtab_to_symtab): Add comment and assert. (map_matching_symbols_psymtab): Remove unnecessary test for non-primary symtab. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index ce37adf..35b8f13 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -2660,6 +2660,10 @@ dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu) process_cu_includes (); do_cleanups (back_to); } + + /* The result of symtab expansion is always the primary symtab. */ + gdb_assert (per_cu->v.quick->symtab->primary); + return per_cu->v.quick->symtab; } @@ -3607,17 +3611,13 @@ dw2_lookup_symbol (struct objfile *objfile, int block_index, { struct symbol *sym = NULL; struct symtab *stab = dw2_instantiate_symtab (per_cu); + const struct blockvector *bv = BLOCKVECTOR (stab); + struct block *block = BLOCKVECTOR_BLOCK (bv, block_index); /* Some caution must be observed with overloaded functions and methods, since the index will not contain any overload information (but NAME might contain it). */ - if (stab->primary) - { - const struct blockvector *bv = BLOCKVECTOR (stab); - struct block *block = BLOCKVECTOR_BLOCK (bv, block_index); - - sym = block_lookup_symbol (block, name, domain); - } + sym = block_lookup_symbol (block, name, domain); if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0) { diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 2514b55..9cfc2c1 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -510,17 +510,16 @@ lookup_symbol_aux_psymtabs (struct objfile *objfile, { struct symbol *sym = NULL; struct symtab *stab = psymtab_to_symtab (objfile, ps); + /* Note: While psymtab_to_symtab can return NULL if the partial symtab + is empty, we can assume it won't here because lookup_partial_symbol + succeeded. */ + const struct blockvector *bv = BLOCKVECTOR (stab); + struct block *block = BLOCKVECTOR_BLOCK (bv, block_index); /* Some caution must be observed with overloaded functions and methods, since the psymtab will not contain any overload information (but NAME might contain it). */ - if (stab->primary) - { - const struct blockvector *bv = BLOCKVECTOR (stab); - struct block *block = BLOCKVECTOR_BLOCK (bv, block_index); - - sym = block_lookup_symbol (block, name, domain); - } + sym = block_lookup_symbol (block, name, domain); if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0) { @@ -756,7 +755,10 @@ lookup_partial_symbol (struct objfile *objfile, } /* Get the symbol table that corresponds to a partial_symtab. - This is fast after the first time you do it. */ + This is fast after the first time you do it. + The result will be NULL if the primary symtab has no symbols, + which can happen. Otherwise the result is the primary symtab + that contains PST. */ static struct symtab * psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst) @@ -779,6 +781,9 @@ psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst) do_cleanups (back_to); } + if (pst->symtab != NULL) + gdb_assert (pst->symtab->primary); + return pst->symtab; } @@ -1259,7 +1264,7 @@ map_matching_symbols_psymtab (struct objfile *objfile, struct symtab *s = psymtab_to_symtab (objfile, ps); struct block *block; - if (s == NULL || !s->primary) + if (s == NULL) continue; block = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), block_kind); if (map_block (name, namespace, objfile, block,