From mboxrd@z Thu Jan 1 00:00:00 1970 From: Elena Zannoni To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] eliminate some annoying mdebug-related symtab crashes Date: Fri, 20 Jul 2001 18:25:00 -0000 Message-id: <15192.59353.70860.131763@krustylu.cygnus.com> References: <20010709151122.A18937@nevyn.them.org> <15192.48720.958756.789421@krustylu.cygnus.com> <20010720154708.A16109@nevyn.them.org> X-SW-Source: 2001-07/msg00541.html 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