Re: [RFA] Fix a "pc ... in psymtab but not in symtab" internal error warning

[Elena Zannoni <ezannoni at redhat dot com>]

Joel Brobecker writes:
 > Hello,
 > 
 > After fixing the problem in mdebugread.c (see one of my recent patches),
 > GDB was no longer crashing, but some internal errors later appeared.
 > Something like this:
 > 
 >    warning: (Internal error: pc 0x3ff800203a8 in read in psymtab, but not in symtab.)
 > 
 > This would happen when trying to do a backtrace, for intance. These
 > warnings are really annoying, as they have a tendency to flood the
 > regular output...
 > 

Yes, and they are silly too, because an internal error is not a warning...

 > I found that the address was a valid PC address for a function inside
 > one of the system libraries (eg libc or libpthread, for instance).  What
 > happened is that GDB was doing a symtab lookup for this address and did
 > not find any. It then did a psymtab lookup, and found one.  But then GDB
 > was surprised to find out that the psymtab was already read in, and
 > hence generated the warning. See find_pc_sect_symtab in symtab.c:
 > 
 >   | ALL_SYMTABS (objfile, s)
 >   |   {
 >   |     if (BLOCK_START (b) <= pc && BLOCK_END (b) > pc && [...])
 >   |       {
 >   |          [...]
 >   |          best_s = s;
 >   |       }
 >   |   }
 >   |
 >   | if (best_s != NULL)
 >   |   return best_s;
 >   |
 >   | ps = find_pc_sect_psymtab (pc, section);
 >   | if (ps)
 >   |   {
 >   |     if (ps->readin)
 >   |       warning ("Internal error: [...]");
 >   |     s = PSYMTAB_TO_SYMTAB (ps);
 >   |   }
 >   |
 >   | return s;
 > 
 > What really striked me when I started debugging this is that the
 > psymtab found by find_pc_sect_psymtab was completely incorrect.
 > At second thought, it should not have found any psymtab, since
 > the symbol in question did not come with any debugging info besides
 > the minimal symbol table.
 > 
 > So I looked at the textlow and texthigh values for the psymtab,
 > and was started by the texthigh value: 0xfffffffffffffffe, or
 > written differently: -2. A "maintenance print symbols" and "maintenance
 > print psymbols" confirmed that many symtabs had a suspisciouly high
 > texthigh value. These dumps also revealed that these entries contained
 > procedures with empty names and and address of 0xfffffffffffffffe.
 > 
 > Debugging a bit furter, I indeed found stProc symbol records whose value
 > are -2. I then looked up the Compaq documentation for the ECOFF format,
 > and it says p171 that (stProc, scInfo) entries represent "a procedure
 > without code, or a function prototype, or a function pointer". In that
 > case, the value field is respectively either -1, -2, or a non-negative
 > value.

I am drooling over this documentation you mention.... All I've ever
seen about mdebug is an old doco from Dave Anderson. Is the one you
are referring to publically available?

 > 
 > I read a bit more about stProc and stStaticProc symbol records.
 > According to this documentation, only a very small subset of the
 > (storage type, storage class) couple is legal: 
 >   - stProc can only be associated with scNil, scText, scUndefined,
 >     and scInfo.
 >   - stStaticProc can only be associated with scText, scInit, and scFini.
 > 
 > It also says that only (stProc, scText) entries are "real" procedures
 > (all combinations of (stStaticProc, sc*) are "real" procedures).
 > 
 > I therefore made some modifications in mdebugread.c to ignore all
 > stProc entries when the storage class was not scText. This fixed
 > the warnings, and did not introduce any regressions.
 > 
 > But there is one flaw in my testing that I have to admit: we don't have
 > a C++ compiler on our Tru64 machine... I still suggest this fix for
 > inclusion in our sources, although I would understand that it be
 > rejected for lack of testing. I checked the output of the "maint print
 > symbols" and "maint print psymbols" commands, and did not find any
 > 0xff[...]fe addresses anymore.
 > 
 > 2003-01-03  J. Brobecker  <brobecker@gnat.com>
 > 
 >         * mdebugread.c (parse_symbol): Skip stProc entries which storage
 >         class is not scText. These do not define "real" procedures.
 >         (parse_partial_symbols): Likewise.
 > 
 > Ahem, ok to commit?

I am not opposed to checking this in. Maybe Daniel J has some machines
available where he can do some further testing?  Only, I would like to
see some of the explanantions you wrote above, being included as
comments in the code.

Elena


 > -- 
 > Joel
 > Index: mdebugread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mdebugread.c,v
 > retrieving revision 1.35
 > diff -c -3 -p -r1.35 mdebugread.c
 > *** mdebugread.c	3 Jan 2003 15:34:59 -0000	1.35
 > --- mdebugread.c	3 Jan 2003 19:05:32 -0000
 > *************** parse_symbol (SYMR *sh, union aux_ext *a
 > *** 751,756 ****
 > --- 751,783 ----
 >   
 >       case stProc:		/* Procedure, usually goes into global block */
 >       case stStaticProc:		/* Static procedure, goes into current block */
 > +       /* Make sure this is a "real" procedure.  Otherwise, skip it.  */
 > +       if (sh->st == stProc && sh->sc != scText)
 > +         {
 > +           char *ext_tsym = ext_sh;
 > +           int keep_counting = 1;
 > +           SYMR tsym;
 > + 
 > +           while (keep_counting)
 > +             {
 > +               ext_tsym += external_sym_size;
 > +               (*swap_sym_in) (cur_bfd, ext_tsym, &tsym);
 > +               count++;
 > +               switch (tsym.st)
 > +                 {
 > +                   case stParam:
 > +                     break;
 > +                   case stEnd:
 > +                     keep_counting = 0;
 > +                     break;
 > +                   default:
 > +                     complaint (&symfile_complaints,
 > +                                "unknown symbol type 0x%x", sh->st);
 > +                     break;
 > +                 }
 > +             }
 > +           break;
 > +         }
 >         s = new_symbol (name);
 >         SYMBOL_NAMESPACE (s) = VAR_NAMESPACE;
 >         SYMBOL_CLASS (s) = LOC_BLOCK;
 > *************** parse_partial_symbols (struct objfile *o
 > *** 3323,3328 ****
 > --- 3350,3383 ----
 >   		  /* FALLTHROUGH */
 >   
 >   		case stProc:
 > + 		  /* Ignore all parameter symbol records.  */
 > + 		  if (sh.index >= hdr->iauxMax)
 > + 		    {
 > + 		      /* Should not happen, but does when cross-compiling
 > + 		         with the MIPS compiler.  FIXME -- pull later.  */
 > + 		      index_complaint (name);
 > + 		      new_sdx = cur_sdx + 1;	/* Don't skip at all */
 > + 		    }
 > + 		  else
 > + 		    new_sdx = AUX_GET_ISYM (fh->fBigendian,
 > + 					    (debug_info->external_aux
 > + 					     + fh->iauxBase
 > + 					     + sh.index));
 > + 
 > + 		  if (new_sdx <= cur_sdx)
 > + 		    {
 > + 		      /* This should not happen either... FIXME.  */
 > + 		      complaint (&symfile_complaints,
 > + 				 "bad proc end in aux found from symbol %s",
 > + 				 name);
 > + 		      new_sdx = cur_sdx + 1;	/* Don't skip backward */
 > + 		    }
 > + 
 > +                   /* Make sure that this stProc entry represents a "real"
 > +                      procedure.  If not, ignore it.  */
 > +                   if (sh.st == stProc && sh.sc != scText)
 > +                     goto skip;
 > + 
 >   		  /* Usually there is a local and a global stProc symbol
 >   		     for a function. This means that the function name
 >   		     has already been entered into the mimimal symbol table
 > *************** parse_partial_symbols (struct objfile *o
 > *** 3345,3373 ****
 >   					 &objfile->static_psymbols,
 >   				    0, sh.value, psymtab_language, objfile);
 >   
 > - 		  /* Skip over procedure to next one. */
 > - 		  if (sh.index >= hdr->iauxMax)
 > - 		    {
 > - 		      /* Should not happen, but does when cross-compiling
 > - 		         with the MIPS compiler.  FIXME -- pull later.  */
 > - 		      index_complaint (name);
 > - 		      new_sdx = cur_sdx + 1;	/* Don't skip at all */
 > - 		    }
 > - 		  else
 > - 		    new_sdx = AUX_GET_ISYM (fh->fBigendian,
 > - 					    (debug_info->external_aux
 > - 					     + fh->iauxBase
 > - 					     + sh.index));
 >   		  procaddr = sh.value;
 > - 
 > - 		  if (new_sdx <= cur_sdx)
 > - 		    {
 > - 		      /* This should not happen either... FIXME.  */
 > - 		      complaint (&symfile_complaints,
 > - 				 "bad proc end in aux found from symbol %s",
 > - 				 name);
 > - 		      new_sdx = cur_sdx + 1;	/* Don't skip backward */
 > - 		    }
 >   
 >   		  cur_sdx = new_sdx;
 >   		  (*swap_sym_in) (cur_bfd,
 > --- 3400,3406 ----