[rfa] eliminate some annoying mdebug-related symtab crashes
Elena Zannoni
ezannoni@cygnus.com
Fri Jul 20 18:25:00 GMT 2001
Daniel Jacobowitz writes:
> 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.
>
Ah, that wasn't clear from the two messages. I was more propense
towards finding a proper place for initialization. That's why I
suggested the other patch instead.
> > 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.
I can see why, yes, there are only one static_psymbol list and one
global_psymbol list and they are all shared and manipulated by the psymtabs
builders that get called for each different debug format.
Whenever you call a psymtab builder you wipe those lists out.
>
> 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).
>
Oh dear, I feel ill. Where is the paper bag, quick! So, the || check
before calling init_psymbol_list works if the two lists are kind of in
synch. They are grown/allocated together. They are both zero at
start, and they get initialized both to the same size in
init_psymbol_lists.
So, in theory they are both of zero length at the same time. Except
that in our call path to building the mdebug psymtabs, we never call
init_psymbol_lists with a real, non zero, size. Then the two lists
get out of synch, because there is another way to grow the lists by
calling extend_psymbol_list, that just changes one list (this function
is called by add_psymbol_to_list).
> 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?
Yes, it is this line in elfread.c that sets the lists to null and their
sizes to 0.
elfread.c:599: init_psymbol_list (objfile, 0);
Dig dig dig, in our internal repository, I found the original
changelog entry (just missed the 10 years annyversary):
Mon Apr 8 23:57:43 1991 John Gilmore (gnu at cygint.cygnus.com)
* dbxread.c (dbx_symfile_read): Initialize psymbol list if this
is the first symbol read, even if not mainline.
I would think that extend_psymbol_list was introduced afterwards, and
this broke the logic of Gilmore's change.
So what to do? I am kind of convinced that your patch is
correct. But..., is there any chance you can run the gdb testsuite
before and after your changes on a couple of platforms that have at
least dwarf2 and stabs debug info (I mean separately, not in the same
objfile)?
Another alternative would be to insert a call to
init_psymbol_lists(objfile, NUM_OF_SYMBOLS) somewhere in mdebugread.c.
Elena
> --
> Daniel Jacobowitz Carnegie Mellon University
> MontaVista Software Debian GNU/Linux Developer
More information about the Gdb-patches
mailing list