Re: Simple but crucial bug fix to gdb

[Christopher Faylor <cgf@redhat.com>]

On Wed, May 30, 2001 at 02:27:45PM -0700, Charlie Mills wrote:
>Dear gdb maintainer,
>
>I would like to submit a very simple patch to gdb.
>I would like if possible to avoid legal issues (avoid having to
>submit a form to our legal department) by simply describing the fix,
>which is a diff of only a few characters, rather than sending you a
>source file.
>
>Bug description:  gdb 4.xx and 5.0 crashes while reading our executable.
>Our executable is the result of linking objects compiled by gcc with
>other objects compiled using SPARCworks CC.  The stack trace is
>appended at the end of this message.
>
>Unfortunately the executable is large and proprietary.
>Although I can't submit a test case, it is very easy to confirm by
>inspecting the code that the patch is correct and the original code
>is incorrect.  The patch is as follows:
>
>File: gdb-5.0/gdb/partial-stab.h
>
>OLD, lines 602-605:
>
>        if (textlow_not_set
>            || (CUR_SYMBOL_VALUE < pst->textlow
>                && CUR_SYMBOL_VALUE
>                != ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT)))
>          {
>            pst->textlow = CUR_SYMBOL_VALUE;
>
>NEW, lines 602-605:
>
>        if (pst && (textlow_not_set
>            || (CUR_SYMBOL_VALUE < pst->textlow
>                && CUR_SYMBOL_VALUE
>                != ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT))))
>          {
>            pst->textlow = CUR_SYMBOL_VALUE;
>
>OLD crashes because pst is 0 (and is intended to be 0 I think).
>
>I hope this is enough for you to get this included in the next release.
>I hate maintaining patches.  Please let me know if there is anything
>else I can do for you about this.  Thank you!

Assuming that I have properly tracked where this is in the current sources,
I think that this change looks reasonable.  There is a similar check for
pst being non-null a few lines up from this point and pst does not get
set in the intervening space.  So, if the previous check is correct, then
this one is obviously needed.

I've included a diff below with some extended context.  If there are no
objections, I'll be happy to check this in.

cgf

Index: partial-stab.h
===================================================================
RCS file: /cvs/uberbaum/gdb/partial-stab.h,v
retrieving revision 1.7
diff -c -2 -0 -p -r1.7 partial-stab.h
*** partial-stab.h	2001/03/06 08:21:11	1.7
--- partial-stab.h	2001/05/30 21:34:59
*************** switch (CUR_SYMBOL_TYPE)
*** 583,626 ****
  #ifdef SOFUN_ADDRESS_MAYBE_MISSING
  	/* Do not fix textlow==0 for .o or NLM files, as 0 is a legit
  	   value for the bottom of the text seg in those cases. */
  	if (pst && textlow_not_set)
  	  {
  	    pst->textlow =
  	      find_stab_function_addr (namestring, pst->filename, objfile);
  	    textlow_not_set = 0;
  	  }
  #endif
  	/* End kludge.  */
  
  	/* Keep track of the start of the last function so we
  	   can handle end of function symbols.  */
  	last_function_start = CUR_SYMBOL_VALUE;
  
  	/* In reordered executables this function may lie outside
  	   the bounds created by N_SO symbols.  If that's the case
  	   use the address of this function as the low bound for
  	   the partial symbol table.  */
! 	if (textlow_not_set
! 	    || (pst && CUR_SYMBOL_VALUE < pst->textlow
! 		&& CUR_SYMBOL_VALUE
! 		!= ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile))))
  	  {
  	    pst->textlow = CUR_SYMBOL_VALUE;
  	    textlow_not_set = 0;
  	  }
  #endif /* DBXREAD_ONLY */
  	add_psymbol_to_list (namestring, p - namestring,
  			     VAR_NAMESPACE, LOC_BLOCK,
  			     &objfile->static_psymbols,
  			     0, CUR_SYMBOL_VALUE,
  			     psymtab_language, objfile);
  	continue;
  
  	/* Global functions were ignored here, but now they
  	   are put into the global psymtab like one would expect.
  	   They're also in the minimal symbol table.  */
        case 'F':
  	CUR_SYMBOL_VALUE += ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
  #ifdef DBXREAD_ONLY
  	/* Kludges for ELF/STABS with Sun ACC */
  	last_function_name = namestring;
--- 583,627 ----
  #ifdef SOFUN_ADDRESS_MAYBE_MISSING
  	/* Do not fix textlow==0 for .o or NLM files, as 0 is a legit
  	   value for the bottom of the text seg in those cases. */
  	if (pst && textlow_not_set)
  	  {
  	    pst->textlow =
  	      find_stab_function_addr (namestring, pst->filename, objfile);
  	    textlow_not_set = 0;
  	  }
  #endif
  	/* End kludge.  */
  
  	/* Keep track of the start of the last function so we
  	   can handle end of function symbols.  */
  	last_function_start = CUR_SYMBOL_VALUE;
  
  	/* In reordered executables this function may lie outside
  	   the bounds created by N_SO symbols.  If that's the case
  	   use the address of this function as the low bound for
  	   the partial symbol table.  */
! 	if (pst
! 	    && (textlow_not_set
! 		|| (pst && CUR_SYMBOL_VALUE < pst->textlow
! 		  && CUR_SYMBOL_VALUE
! 		  != ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile)))))
  	  {
  	    pst->textlow = CUR_SYMBOL_VALUE;
  	    textlow_not_set = 0;
  	  }
  #endif /* DBXREAD_ONLY */
  	add_psymbol_to_list (namestring, p - namestring,
  			     VAR_NAMESPACE, LOC_BLOCK,
  			     &objfile->static_psymbols,
  			     0, CUR_SYMBOL_VALUE,
  			     psymtab_language, objfile);
  	continue;
  
  	/* Global functions were ignored here, but now they
  	   are put into the global psymtab like one would expect.
  	   They're also in the minimal symbol table.  */
        case 'F':
  	CUR_SYMBOL_VALUE += ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
  #ifdef DBXREAD_ONLY
  	/* Kludges for ELF/STABS with Sun ACC */
  	last_function_name = namestring;