This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [PATCH RFA]: Fix hash table bugs in minsyms.c


Kevin Buettner <kevinb@cygnus.com> writes:

Darn it, I knew, after staring at that code (since it's one of the
places SYMBOL_INIT_DEMANGLED_NAME gets called, and where things get
into the demangled minsym hash table) , that there was something
off about it, but i couldn't pinpoint exactly what it was, and moved
on. I had a feeling it was missing the last symbol, but never ran into
something where it actuallly happened.

Nice catch.



> I request approval for committing the patch below.
> 
> I'm working on a project in which the OS wants to place certain groups
> of sections at different addresses than specified by the symbolic
> information in the executable.  Thus it is necessary to call
> objfile_relocate() (in objfiles.c).  objfile_relocate() calls
> msymbols_sort() which cause the minimal symbols to get sorted.
> 
> The problem with this is that after the sort operation, the
> minimal symbol hash table entries are no longer correct.  In
> fact, they are wildly incorrect.  The reason for this is that
> the sort routine does not account for the motion of the symbol
> table entries in the hash table.
> 
> In the process of diagnosing this problem, I took a close look
> at how the hash table was being constructed.  The minimal symbols
> reside in an obstack and the hash table links are constructed
> in compact_minimal_symbols().  This code, as written, contains
> a minor bug, a fence post error in which the last symbol in
> the obstack was *not* added to the hash table.
> 
> But it contains a much more serious bug as well.  Because the
> symbols reside in an obstack, the obstack machinery is free
> to relocate the space up until the time obstack_finish() is
> called.  Since the call to compact_minimal_symbols() (and hence
> the hash table creation) occurs before obstack_finish(), it is
> not safe to create references to the obstack entries since the
> space could get moved and any reference created could wind up
> being either incorrect or even dangling.
> 
> In order to correct these problems, I've added a new function called
> build_minimal_symbol_hash_tables() which is called after
> obstack_finish() as well as after the sorting has been done in
> msymbols_sort.
> 
> I've tested this patch on GNU/linux/x86 with the compiler set to
> generate DWARF2 info instead of the default (stabs).  I've seen no
> regressions.  (And better still, it also fixes the problems that I ran
> into on my project.)
> 
> 	* minsyms.c (build_minimal_symbol_hash_tables): New function.
> 	(compact_minimal_symbols): Don't construct hash tables here.
> 	(install_minimal_symbols): Instead, construct them here.
> 	(msymbols_sort): And rebuild them here too.
> 
> Index: minsyms.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/minsyms.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 minsyms.c
> --- minsyms.c	2000/07/30 01:48:26	1.10
> +++ minsyms.c	2000/08/03 23:27:59
> @@ -801,11 +801,7 @@ compact_minimal_symbols (struct minimal_
>  	      copyfrom++;
>  	    }
>  	  else
> -	    {
> -	      *copyto++ = *copyfrom++;
> -
> -	      add_minsym_to_hash_table (copyto - 1, objfile->msymbol_hash);
> -	    }
> +	    *copyto++ = *copyfrom++;
>  	}
>        *copyto++ = *copyfrom++;
>        mcount = copyto - msymbol;
> @@ -813,6 +809,38 @@ compact_minimal_symbols (struct minimal_
>    return (mcount);
>  }
> 
> +/* Build (or rebuild) the minimal symbol hash tables.  This is necessary
> +   after compacting or sorting the table since the entries move around
> +   thus causing the internal minimal_symbol pointers to become jumbled. */
> +  
> +static void
> +build_minimal_symbol_hash_tables (struct objfile *objfile)
> +{
> +  int i;
> +  struct minimal_symbol *msym;
> +
> +  /* Clear the hash tables. */
> +  for (i = 0; i < MINIMAL_SYMBOL_HASH_SIZE; i++)
> +    {
> +      objfile->msymbol_hash[i] = 0;
> +      objfile->msymbol_demangled_hash[i] = 0;
> +    }
> +
> +  /* Now, (re)insert the actual entries. */
> +  for (i = objfile->minimal_symbol_count, msym = objfile->msymbols;
> +       i > 0;
> +       i--, msym++)
> +    {
> +      msym->hash_next = 0;
> +      add_minsym_to_hash_table (msym, objfile->msymbol_hash);
> +
> +      msym->demangled_hash_next = 0;
> +      if (SYMBOL_DEMANGLED_NAME (msym) != NULL)
> +	add_minsym_to_demangled_hash_table (msym,
> +                                            objfile->msymbol_demangled_hash);
> +    }
> +}
> +
>  /* Add the minimal symbols in the existing bunches to the objfile's official
>     minimal symbol table.  In most cases there is no minimal symbol table yet
>     for this objfile, and the existing bunches are used to create one.  Once
> @@ -928,12 +956,13 @@ install_minimal_symbols (struct objfile 
>           ones and attempting to cache their C++ demangled names. */
> 
>        for (; mcount-- > 0; msymbols++)
> -	{
> -	  SYMBOL_INIT_DEMANGLED_NAME (msymbols, &objfile->symbol_obstack);
> -	  if (SYMBOL_DEMANGLED_NAME (msymbols) != NULL)
> -          add_minsym_to_demangled_hash_table (msymbols,
> -                                              objfile->msymbol_demangled_hash);
> -	}
> +	SYMBOL_INIT_DEMANGLED_NAME (msymbols, &objfile->symbol_obstack);
> +
> +      /* Now build the hash tables; we can't do this incrementally
> +         at an earlier point since we weren't finished with the obstack
> +	 yet.  (And if the msymbol obstack gets moved, all the internal
> +	 pointers to other msymbols need to be adjusted.) */
> +      build_minimal_symbol_hash_tables (objfile);
>      }
>  }
> 
> @@ -944,6 +973,7 @@ msymbols_sort (struct objfile *objfile)
>  {
>    qsort (objfile->msymbols, objfile->minimal_symbol_count,
>  	 sizeof (struct minimal_symbol), compare_minimal_symbols);
> +  build_minimal_symbol_hash_tables (objfile);
>  }
> 
>  /* Check if PC is in a shared library trampoline code stub.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]