diff mbox

The result of symtab expansion is always a primary symtab

Message ID m3mw7z20r2.fsf@sspiff.org
State New
Headers show

Commit Message

Doug Evans Nov. 10, 2014, 3:27 a.m. UTC 
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  <xdje42@gmail.com>

	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.

Comments

Doug Evans Nov. 18, 2014, 5:09 p.m. UTC | #1 
On Sun, Nov 9, 2014 at 7:27 PM, Doug Evans <xdje42@gmail.com> wrote:
> [...]
>
> 2014-11-09  Doug Evans  <xdje42@gmail.com>
>
>         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.

Committed.
diff mbox

Patch

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,