Re: [rfa] eliminate some annoying mdebug-related symtab crashes

[Elena Zannoni <ezannoni at cygnus dot com>]

Daniel Jacobowitz writes:
 > I'm really looking forward to getting away from mdebug and back to straight
 > ELF stabs, but I need mdebug for one last project.  This patch addresses two
 > of the crashes I've been having - properly, this time.
 > 
 > The init_header_files fix is almost trivial, although it might be preferable
 > to rename the functions now that I've had to make them non-static.  The list
 > was NULL, mdebugread's psymtab_to_symtab_1 was calling dbxread's
 > process_one_symbol which called add_new_header_file, and we crashed.  I'm
 > not sure if the extra:
 > 
 > +  stabsread_new_init ();
 > +  buildsym_new_init ();
 > 
 > is really necessary, since elfread_new_init() calls them, but analogy with
 > every other existing symbol reader suggests that it is correct, or at least
 > customary.
 > 

Is the above another fix for the same problem you submitted in the
previous patch?  If you rewrite a patch could you please withdraw the
first one, so that no time goes into reviewing it?  Anyway, I consider
this addressed. It occurred to me to do it this way, but I would be
curious to see if the other way I suggested works as well.

The below is for another problem. They should really be two separate
patches... Anyway.  For this I would be really hesitant to commit
changes to the other readers.  Unless you can show that there are no
regressions. I think overall is better to leave those alone.  Even
though the change seems to make sense, and the comments seem to imply
that both list should be empty. But the code has been like that for ages, I
just looked at the cvs repository, and it was like that already in
1995. I know this is not a real argument, but....

Can you elaborate some more on what's happening here?  Do you have 2
different debug info sections, mdebug + stabs in the same object file?
Do you end up with two psymtabs chains (one created by
elfmdebug_build_psymtabs, the other by elfstabs_build_psymtabs)
pointing to the same object file, therefore sharing the
static_psymbols and global_psymbol lists? And this is why at the point
of the call to init_psymbol_list from dbx_symfile_read (ultimately
deriving from the call to elfstab_build_psymtabs) there is information
in the psymbol lists already?  

According to elf_symfile_read there can be even more than two
different debugging sections per object file. I wonder why this hasn't
created problems for others before. What incorrect behavior do you
see, i.e. what's the symptom, where do you get the crash? Can I see a
stack trace?

Is there any way for me to reproduce this?  What are the platform and
the target? Do you have an example program that exhibits these
problems?


Thanks
Elena


 > 
 > The init_psymbol_list is a little trickier.  Normally, both the global and
 > static symbol lists for an objfile are pre-allocated based on the expected
 > number of symbols.  mdebugread does not do that, which is, I think, fine. 
 > If global symbols are read but no static symbols are read, which has
 > happened to me in the startfiles several times, we should not be
 > re-initializing the list of symbols - there are psymtabs pointing in to what
 > we're freeing.  We only want to do that if neither global nor static symbols
 > have been read.
 > 
 > 
 > Are these OK to commit?
 > 
 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 2001-07-07  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* dbxread.c (dbx_symfile_read): Only reinitialize
 > 	the psymbol list if mainline or if both static
 > 	and global lists are empty.
 > 	* dwarf2read.c (dwarf2_build_psymtabs): Likewise.
 > 	* dwarfread.c (dwarf_build_psymtabs): Likewise.
 > 	* xcoffread.c (xcoff_initial_scan): Likewise.
 > 	* os9kread.c (os9k_symfile_read): Likewise.
 > 
 > 	* dbxread.c (free_header_files): Make global.
 > 	(init_header_files): Likewise.
 > 	* stabsread.h (free_header_files): Add prototype.
 > 	(init_header_files): Likewise.
 > 	* mdebugread.c (mdebug_build_psymtabs): Initialize
 > 	properly before using the stabs debug reader.
 > 
 > Index: dbxread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dbxread.c,v
 > retrieving revision 1.19
 > diff -u -r1.19 dbxread.c
 > --- dbxread.c	2001/07/07 17:19:50	1.19
 > +++ dbxread.c	2001/07/09 21:46:34
 > @@ -270,10 +270,6 @@
 >  
 >  static void process_now (struct objfile *);
 >  
 > -static void free_header_files (void);
 > -
 > -static void init_header_files (void);
 > -
 >  static void read_ofile_symtab (struct partial_symtab *);
 >  
 >  static void dbx_psymtab_to_symtab (struct partial_symtab *);
 > @@ -319,7 +315,7 @@
 >  
 >  /* Free up old header file tables */
 >  
 > -static void
 > +void
 >  free_header_files (void)
 >  {
 >    if (this_object_header_files)
 > @@ -332,7 +328,7 @@
 >  
 >  /* Allocate new header file tables */
 >  
 > -static void
 > +void
 >  init_header_files (void)
 >  {
 >    n_allocated_this_object_header_files = 10;
 > @@ -582,8 +578,8 @@
 >  
 >    /* If we are reinitializing, or if we have never loaded syms yet, init */
 >    if (mainline
 > -      || objfile->global_psymbols.size == 0
 > -      || objfile->static_psymbols.size == 0)
 > +      || (objfile->global_psymbols.size == 0
 > +	  &&  objfile->static_psymbols.size == 0))
 >      init_psymbol_list (objfile, DBX_SYMCOUNT (objfile));
 >  
 >    symbol_size = DBX_SYMBOL_SIZE (objfile);
 > Index: dwarf2read.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dwarf2read.c,v
 > retrieving revision 1.29
 > diff -u -r1.29 dwarf2read.c
 > --- dwarf2read.c	2001/07/05 16:45:48	1.29
 > +++ dwarf2read.c	2001/07/09 21:46:38
 > @@ -869,8 +869,9 @@
 >  					   dwarf_line_offset,
 >  					   dwarf_line_size);
 >  
 > -  if (mainline || objfile->global_psymbols.size == 0 ||
 > -      objfile->static_psymbols.size == 0)
 > +  if (mainline
 > +      || (objfile->global_psymbols.size == 0
 > +	  && objfile->static_psymbols.size == 0))
 >      {
 >        init_psymbol_list (objfile, 1024);
 >      }
 > Index: dwarfread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dwarfread.c,v
 > retrieving revision 1.7
 > diff -u -r1.7 dwarfread.c
 > --- dwarfread.c	2001/03/06 08:21:07	1.7
 > +++ dwarfread.c	2001/07/09 21:46:41
 > @@ -706,8 +706,9 @@
 >       Since we have no idea how many DIES we are looking at, we just guess
 >       some arbitrary value. */
 >  
 > -  if (mainline || objfile->global_psymbols.size == 0 ||
 > -      objfile->static_psymbols.size == 0)
 > +  if (mainline
 > +      || (objfile->global_psymbols.size == 0
 > +	  && objfile->static_psymbols.size == 0))
 >      {
 >        init_psymbol_list (objfile, 1024);
 >      }
 > Index: mdebugread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mdebugread.c,v
 > retrieving revision 1.13
 > diff -u -r1.13 mdebugread.c
 > --- mdebugread.c	2001/05/29 10:45:10	1.13
 > +++ mdebugread.c	2001/07/09 21:46:46
 > @@ -491,6 +491,11 @@
 >    debug_swap = swap;
 >    debug_info = info;
 >  
 > +  stabsread_new_init ();
 > +  buildsym_new_init ();
 > +  free_header_files ();
 > +  init_header_files ();
 > +        
 >    /* Make sure all the FDR information is swapped in.  */
 >    if (info->fdr == (FDR *) NULL)
 >      {
 > Index: os9kread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/os9kread.c,v
 > retrieving revision 1.9
 > diff -u -r1.9 os9kread.c
 > --- os9kread.c	2001/03/07 02:57:08	1.9
 > +++ os9kread.c	2001/07/09 21:46:49
 > @@ -320,8 +320,9 @@
 >  
 >    sym_bfd = objfile->obfd;
 >    /* If we are reinitializing, or if we have never loaded syms yet, init */
 > -  if (mainline || objfile->global_psymbols.size == 0 ||
 > -      objfile->static_psymbols.size == 0)
 > +  if (mainline
 > +      || (objfile->global_psymbols.size == 0
 > +	  && objfile->static_psymbols.size == 0))
 >      init_psymbol_list (objfile, DBX_SYMCOUNT (objfile));
 >  
 >    free_pending_blocks ();
 > Index: stabsread.h
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/stabsread.h,v
 > retrieving revision 1.5
 > diff -u -r1.5 stabsread.h
 > --- stabsread.h	2001/03/06 08:21:17	1.5
 > +++ stabsread.h	2001/07/09 21:46:50
 > @@ -219,4 +219,8 @@
 >  extern int resolve_cfront_continuation
 >    (struct objfile *objfile, struct symbol *sym, char *p);
 >  
 > +extern void free_header_files (void);
 > +
 > +extern void init_header_files (void);
 > +
 >  #undef EXTERN
 > Index: xcoffread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/xcoffread.c,v
 > retrieving revision 1.14
 > diff -u -r1.14 xcoffread.c
 > --- xcoffread.c	2001/05/01 19:36:11	1.14
 > +++ xcoffread.c	2001/07/09 21:46:52
 > @@ -2684,8 +2684,8 @@
 >  
 >    /* If we are reinitializing, or if we have never loaded syms yet, init */
 >    if (mainline
 > -      || objfile->global_psymbols.size == 0
 > -      || objfile->static_psymbols.size == 0)
 > +      || (objfile->global_psymbols.size == 0
 > +	  && objfile->static_psymbols.size == 0))
 >      /* I'm not sure how how good num_symbols is; the rule of thumb in
 >         init_psymbol_list was developed for a.out.  On the one hand,
 >         num_symbols includes auxents.  On the other hand, it doesn't