Re: [PATCH RFA] DWARF v2.1 64-bit support

[Kevin Buettner <kevinb@cygnus.com>]

On Aug 4,  9:30am, Kevin Buettner wrote:

>      - Kevin, Sept 4, 2000 ]
                ^^^^
Okay, so I really seem to have a problem with the month of August.  (I
also sent a recent message claiming that August was also July.)  Anyway,
thanks to Elena for pointing this out to me.  It's fixed now.
From ezannoni@cygnus.com Fri Aug 04 10:28:00 2000
From: Elena Zannoni <ezannoni@cygnus.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFA]: Fix hash table bugs in minsyms.c
Date: Fri, 04 Aug 2000 10:28:00 -0000
Message-id: <14730.64814.976208.581246@kwikemart.cygnus.com>
References: <1000804003817.ZM3200@ocotillo.lan>
X-SW-Source: 2000-08/msg00098.html
Content-length: 5640

Kevin Buettner writes:
 > I request approval for committing the patch below.
 > 

Go ahead, it makes sense. 
Thanks
Elena 

 > 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.
 > 
From eliz@delorie.com Fri Aug 04 11:10:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: kevinb@cygnus.com
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFC] Protoize ch-exp.c, core-regset.c
Date: Fri, 04 Aug 2000 11:10:00 -0000
Message-id: <200008041810.OAA28507@indy.delorie.com>
References: <1000803190941.ZM2697@ocotillo.lan> <200008040931.FAA28059@indy.delorie.com> <1000804151730.ZM4041@ocotillo.lan>
X-SW-Source: 2000-08/msg00099.html
Content-length: 564

> Date: Fri, 4 Aug 2000 08:17:30 -0700
> From: Kevin Buettner <kevinb@cygnus.com>
> 
> Okay... here's what I'll do.  I'll make a reasonable stab at moving
> comments embedded in the parameter list to before the function along
> with some minimal prefatory remarks.  I ask that maintainers of the
> affected files (and anyone else who's interested) look over these
> changes to make sure that they aren't objectionable.

I don't think anyone could expect more.  I agree with this plan, and
will try to review all the patches you post for this as closely as I
can.