Re: [PATCH RFA] elfread.c: Fix dwarf2read.c related crash...

[Jim Blandy <jimb at zwingli dot cygnus dot com>]

I approve this change.

Kevin Buettner <kevinb@cygnus.com> writes:

> 
> The patch below fixes a crash (actually, an internal error) that
> occurs when attempting to use gdb to debug certain programs created
> with current development versions of gcc/g++.  For more information,
> see:
> 
>     http://sources.redhat.com/ml/gdb/2000-11/msg00285.html
>     http://sources.redhat.com/ml/gdb/2000-11/msg00286.html
>     http://sources.redhat.com/ml/gdb/2000-11/msg00287.html
>     http://sources.redhat.com/ml/gdb/2000-11/msg00290.html
>     http://sources.redhat.com/ml/gdb/2000-12/msg00000.html
>     http://sources.redhat.com/ml/gdb/2000-12/msg00001.html
> 
> In a nutshell, new_symbol() in dwarf2read.c is using
> fixup_symbol_section() to set the section index for the symbol that
> it's creating.  The only problem is that it was setting it to -1 which
> was causing the ANOFFSET macro (on the lines following the call to
> fixup_symbol_section()) to generate an internal error.
> 
> fixup_symbol_section() looks up the corresponding minimal symbol and
> uses the minsym's section index to set the index for the symbol
> needing "fixing up".  In the case under consideration, the mimimal
> symbol *was* being found, but the section index was -1.
> 
> The reason for this is that the symbol in question was in a data
> section (i.e, it wasn't code), but had the BSF_WEAK flag set.  It
> turns out that this case was simply not being handled by the portion
> of the code in elf_symtab_read() which was responsible for computing
> the value of ms_type.  The last one line change in my patch addresses
> this problem.  (This change by itself is sufficient for fixing the
> current bug.)
> 
> I've also substantially modified record_minimal_symbol_and_info() in
> order to prevent a symbol's section index from being set to -1 for
> some future "corner" case.  In the past, the switch statement in this
> function was used to compute the section index based upon the value of
> ms_type.  The resulting section index was one of either SECT_OFF_TEXT,
> SECT_OFF_DATA, or SECT_OFF_BSS.  [Do ``cvs annotate -r1.4 elfread.c''
> to see what it used to look like.] Earlier this year, the code was
> changed so that the section index for the interesting ms_type cases
> was computed by simply using bfd's index.  However, the default case
> in which the section index was set to -1 was retained.  My change
> smooshes all of these cases (including the default case) together so
> that we always use the bfd section index even for the
> "non-interesting" values of ms_type. (This change by itself is also
> sufficient for fixing the bug in question.)
> 
> The other approach which could be taken here is to generate an
> internal error for the default case.  I'm certainly willing to
> (re)consider this approach, but I would recommend to anyone wanting to
> suggest this that you first look at the means by which ms_type is
> computed in elf_symtab_read().  This code is truly scary and I have
> doubts about its overall correctness.  (The comments in the code admit
> that gdb is basically guessing.)  It seems to me that we can do better
> than rely on a guess; in this case doing better means simply relying
> on bfd for the section index.
> 
> I've tested on linux/x86 with both dwarf2 and stabs and see no
> regressions.
> 
> So... with the explanation out of the way, I ask for approval to
> commit the following changes:
> 
> 	* elfread.c (record_minimal_symbol_and_info): Don't guess
> 	at the section index; instead just always use the bfd index.
> 	(elf_symtab_read): Handle weak symbols appearing in data
> 	sections.
> 
> Index: elfread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/elfread.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 elfread.c
> --- elfread.c	2000/08/07 15:02:48	1.11
> +++ elfread.c	2000/12/02 04:58:55
> @@ -171,32 +171,13 @@ record_minimal_symbol_and_info (char *na
>  				enum minimal_symbol_type ms_type, char *info,	/* FIXME, is this really char *? */
>  				asection *bfd_section, struct objfile *objfile)
>  {
> -  int section;
> -
> -  /* Guess the section from the type.  This is likely to be wrong in
> -     some cases.  */
> -  switch (ms_type)
> -    {
> -    case mst_text:
> -    case mst_file_text:
> -      section = bfd_section->index;
>  #ifdef SMASH_TEXT_ADDRESS
> -      SMASH_TEXT_ADDRESS (address);
> +  if (ms_type == mst_text || ms_type == mst_file_text)
> +    SMASH_TEXT_ADDRESS (address);
>  #endif
> -      break;
> -    case mst_data:
> -    case mst_file_data:
> -    case mst_bss:
> -    case mst_file_bss:
> -      section = bfd_section->index;
> -      break;
> -    default:
> -      section = -1;
> -      break;
> -    }
>  
>    return prim_record_minimal_symbol_and_info
> -    (name, address, ms_type, info, section, bfd_section, objfile);
> +    (name, address, ms_type, info, bfd_section->index, bfd_section, objfile);
>  }
>  
>  /*
> @@ -423,7 +404,7 @@ elf_symtab_read (struct objfile *objfile
>  		}
>  	      else if (sym->section->flags & SEC_ALLOC)
>  		{
> -		  if (sym->flags & BSF_GLOBAL)
> +		  if (sym->flags & (BSF_GLOBAL | BSF_WEAK))
>  		    {
>  		      if (sym->section->flags & SEC_LOAD)
>  			{
> 
>