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

[Daniel Jacobowitz <dmj+ at andrew dot cmu dot edu>]

On Fri, Jul 20, 2001 at 07:27:12PM -0400, Elena Zannoni wrote:
> 
> 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 include files fix here is indeed for the same problem as in the
other patch.  I didn't withdraw the first patch because it seemed best
to me to fix it in both places - initialize properly, and don't die if
we didn't.  Defensive programming; I'm sick of GDB crashing on the
(admittedly somewhat bizarre) binaries I've been giving it.

> The below is for another problem. They should really be two separate
> patches... Anyway.  For this I would be really hesitant to commit

Yes, you're right.  Sorry.

> 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?

Reproducing it would be somewhat difficult.  It happened when the user
program was built with GCC 3.0 and had stabs debug info in normal
.stabs sections, and the C library (and specifically crt*.o) were built
by GCC 2.95 and an early binutils, and so had .mdedug sections.  I can
easily reproduce the other two crashes that I've submitted patches for,
but not this one.  There's also no useful backtrace; global_psymbol
gets corrupted when init_psymbol_list is called, and then a later
attempt to access a global symbol from crtbegin.o causes the crash.

The reason it doesn't seem to have caused problems before is that most
other symbol readers have a rough idea of how many symbols there will
be, and call init_psymbol_list with some non-zero value.  This
initializes both global and static sizes, and so the old check prevents
us from re-calling init_psymbol_lists, at a cost of some waste of space
(there seem to be substantially fewer static than global psymbols in
most of the tests I tried).

mdebugread fills in only a few (11, I believe) global symbols from
crtbegin.o and no static symbols.  Then when we go into
dbx_symfile_read, we lose.

Does that make sense?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer