[RFC] GDB's mdebug support vs. GCC 3.0

[RFC] GDB's mdebug support vs. GCC 3.0

[Daniel Jacobowitz]

The stabs support for mips-linux is now much more complete than it was; from
what I can see this is because we started using config/elfos.h in gcc.  Two
of the gems we get from this are EINCL/BINCL stabs, which as far as I can
see we did not before, and a trailing SO at the end of a file.

The way we call process_one_symbol in dbxread.c from psymtab_to_symtab_1 in
mdebugread.c is, to say the least, a little iffy.  We aren't doing the
initialization it expects.  This patch is a pair of bandaids, but I think
that they're about as correct bandaids as I can manage.  I spent a while
going through the stabs reader, trying to figure out how these things are
actually supposed to work, and couldn't.  With the patch, we no longer crash
every time we try to read in a symtab; one obvious NULL pointer dereference,
and then doubled calls to end_symtab.

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

2001-06-29  Daniel Jacobowitz  <drow@mvista.com>
	* dbxread.c (add_this_object_header_file): Make sure the header file
	list is allocated.
	* mdebugread.c (psymtab_to_symtab_1): Handle N_SO stabs without
	a name specially.

Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.18
diff -u -r1.18 dbxread.c
--- dbxread.c	2001/04/27 00:19:09	1.18
+++ dbxread.c	2001/06/29 19:32:28
@@ -345,6 +345,8 @@
 static void
 add_this_object_header_file (int i)
 {
+  if (n_allocated_this_object_header_files == 0)
+      init_header_files ();
   if (n_this_object_header_files == n_allocated_this_object_header_files)
     {
       n_allocated_this_object_header_files *= 2;
Index: mdebugread.c
===================================================================
RCS file: /cvs/src/src/gdb/mdebugread.c,v
retrieving revision 1.13
diff -u -r1.13 mdebugread.c
--- mdebugread.c	2001/05/29 10:45:10	1.13
+++ mdebugread.c	2001/06/29 19:32:29
@@ -3210,10 +3210,11 @@
   void (*swap_sym_in) (bfd *, PTR, SYMR *);
   void (*swap_pdr_in) (bfd *, PTR, PDR *);
   int i;
-  struct symtab *st;
+  struct symtab *st = NULL;
   FDR *fh;
   struct linetable *lines;
   CORE_ADDR lowest_pdr_addr = 0;
+  int last_symtab_ended = 0;
 
   if (pst->readin)
     return;
@@ -3319,8 +3320,27 @@
 	         complaining about them.  */
 	      if (type_code & N_STAB)
 		{
-		  process_one_symbol (type_code, 0, valu, name,
-				      pst->section_offsets, pst->objfile);
+		  /* If we found a trailing N_SO with no name, process it here
+		     instead of in process_one_symbol, so we can keep its symtab. */
+		  if (type_code == N_SO
+		      && last_source_file
+		      && previous_stab_code != (unsigned char) N_SO
+		      && *name == '\000')
+		    {
+		      valu += ANOFFSET (pst->section_offsets,
+					SECT_OFF_TEXT (pst->objfile));
+		      previous_stab_code = N_SO;
+		      st = end_symtab (valu, pst->objfile,
+				       SECT_OFF_TEXT (pst->objfile));
+		      end_stabs ();
+		      last_symtab_ended = 1;
+		    }
+		  else
+		    {
+		      last_symtab_ended = 0;
+		      process_one_symbol (type_code, 0, valu, name,
+					  pst->section_offsets, pst->objfile);
+		    }
 		}
 	      /* Similarly a hack.  */
 	      else if (name[0] == '#')
@@ -3368,9 +3388,13 @@
 	    ;
 	  else
 	    complain (&stab_unknown_complaint, name);
+	}
+
+      if (! last_symtab_ended)
+	{
+	  st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
+	  end_stabs ();
 	}
-      st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
-      end_stabs ();
 
       /* Sort the symbol table now, we are done adding symbols to it.
          We must do this before parse_procedure calls lookup_symbol.  */

Re: [RFC] GDB's mdebug support vs. GCC 3.0

[H . J . Lu]

On Fri, Jun 29, 2001 at 12:39:44PM -0700, Daniel Jacobowitz wrote:
> The stabs support for mips-linux is now much more complete than it was; from
> what I can see this is because we started using config/elfos.h in gcc.  Two
> of the gems we get from this are EINCL/BINCL stabs, which as far as I can
> see we did not before, and a trailing SO at the end of a file.
> 

Since you mentioned mdebug and mips-linux, I assume you are aware that
Linux/MIPS no longer uses mdebug. I have changed it to MIPS_STABS_ELF.


H.J.

Re: [RFC] GDB's mdebug support vs. GCC 3.0

[Daniel Jacobowitz]

On Fri, Jun 29, 2001 at 01:09:12PM -0700, H . J . Lu wrote:
> On Fri, Jun 29, 2001 at 12:39:44PM -0700, Daniel Jacobowitz wrote:
> > The stabs support for mips-linux is now much more complete than it was; from
> > what I can see this is because we started using config/elfos.h in gcc.  Two
> > of the gems we get from this are EINCL/BINCL stabs, which as far as I can
> > see we did not before, and a trailing SO at the end of a file.
> > 
> 
> Since you mentioned mdebug and mips-linux, I assume you are aware that
> Linux/MIPS no longer uses mdebug. I have changed it to MIPS_STABS_ELF.

I'm aware of that, yes.  GCC 3.0 emits mdebug, however.  With 3.0.1
hopefully we can stop using the mdebug code.  Or simply switch to
DWARF2; I'm still not clear on why we haven't done that already.

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

Re: [RFC] GDB's mdebug support vs. GCC 3.0

[H . J . Lu]

On Fri, Jun 29, 2001 at 01:13:00PM -0700, Daniel Jacobowitz wrote:
> On Fri, Jun 29, 2001 at 01:09:12PM -0700, H . J . Lu wrote:
> > On Fri, Jun 29, 2001 at 12:39:44PM -0700, Daniel Jacobowitz wrote:
> > > The stabs support for mips-linux is now much more complete than it was; from
> > > what I can see this is because we started using config/elfos.h in gcc.  Two
> > > of the gems we get from this are EINCL/BINCL stabs, which as far as I can
> > > see we did not before, and a trailing SO at the end of a file.
> > > 
> > 
> > Since you mentioned mdebug and mips-linux, I assume you are aware that
> > Linux/MIPS no longer uses mdebug. I have changed it to MIPS_STABS_ELF.
> 
> I'm aware of that, yes.  GCC 3.0 emits mdebug, however.  With 3.0.1
> hopefully we can stop using the mdebug code.  Or simply switch to
> DWARF2; I'm still not clear on why we haven't done that already.

I don't think so. GCC 3.0 doesn't emit mdebug, only stabs. It is gas
who does mdebug or MIPS_STABS_ELF.

BTW, gcc 3.0 doesn't work tool well on Linux/mips. I submitted a few
patches. But .....


H.J.

Re: [RFC] GDB's mdebug support vs. GCC 3.0

[Elena Zannoni]

Wow, what a messy control flow. Makes me dizzy.  I am starting to
understand this patch a bit.  Just a few questions. Do you go through
mipsread.c at all?  If so, does mipscoff_new_init get called?  If so,
can you try to add the call to init_header_files in there instead?
Like this:

Index: mipsread.c
===================================================================
RCS file: /cvs/src/src/gdb/mipsread.c,v
retrieving revision 1.9
diff -u -p -r1.9 mipsread.c
--- mipsread.c	2001/05/29 10:45:10	1.9
+++ mipsread.c	2001/07/19 20:49:45
@@ -68,6 +68,7 @@ mipscoff_new_init (struct objfile *ignor
   sigtramp_address = 0;
   stabsread_new_init ();
   buildsym_new_init ();
+  init_header_files ();
 }
 
 /* Initialize to read a symbol file (nothing to do).  */



Does this work? It's a guess. I cannot test the code, really.

Next problem:

Can you explain a bit more what happens there?  I see that your new
code in the if branch does the same things that process_one_symbol would do.
. Change valu by the offset
. call end_symtab
. call end_stabs

Are you saying that the symtab would be ended twice in that case? Once
in process_one_symbol and once in psymtab_to_symtab_1?
I think this the problem right?
I am going to think some more.

thanks
Elena



Daniel Jacobowitz writes:
 > The stabs support for mips-linux is now much more complete than it was; from
 > what I can see this is because we started using config/elfos.h in gcc.  Two
 > of the gems we get from this are EINCL/BINCL stabs, which as far as I can
 > see we did not before, and a trailing SO at the end of a file.
 > 
 > The way we call process_one_symbol in dbxread.c from psymtab_to_symtab_1 in
 > mdebugread.c is, to say the least, a little iffy.  We aren't doing the
 > initialization it expects.  This patch is a pair of bandaids, but I think
 > that they're about as correct bandaids as I can manage.  I spent a while
 > going through the stabs reader, trying to figure out how these things are
 > actually supposed to work, and couldn't.  With the patch, we no longer crash
 > every time we try to read in a symtab; one obvious NULL pointer dereference,
 > and then doubled calls to end_symtab.
 > 
 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2001-06-29  Daniel Jacobowitz  <drow@mvista.com>
 > 	* dbxread.c (add_this_object_header_file): Make sure the header file
 > 	list is allocated.
 > 	* mdebugread.c (psymtab_to_symtab_1): Handle N_SO stabs without
 > 	a name specially.
 > 
 > Index: dbxread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dbxread.c,v
 > retrieving revision 1.18
 > diff -u -r1.18 dbxread.c
 > --- dbxread.c	2001/04/27 00:19:09	1.18
 > +++ dbxread.c	2001/06/29 19:32:28
 > @@ -345,6 +345,8 @@
 >  static void
 >  add_this_object_header_file (int i)
 >  {
 > +  if (n_allocated_this_object_header_files == 0)
 > +      init_header_files ();
 >    if (n_this_object_header_files == n_allocated_this_object_header_files)
 >      {
 >        n_allocated_this_object_header_files *= 2;
 > Index: mdebugread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mdebugread.c,v
 > retrieving revision 1.13
 > diff -u -r1.13 mdebugread.c
 > --- mdebugread.c	2001/05/29 10:45:10	1.13
 > +++ mdebugread.c	2001/06/29 19:32:29
 > @@ -3210,10 +3210,11 @@
 >    void (*swap_sym_in) (bfd *, PTR, SYMR *);
 >    void (*swap_pdr_in) (bfd *, PTR, PDR *);
 >    int i;
 > -  struct symtab *st;
 > +  struct symtab *st = NULL;
 >    FDR *fh;
 >    struct linetable *lines;
 >    CORE_ADDR lowest_pdr_addr = 0;
 > +  int last_symtab_ended = 0;
 >  
 >    if (pst->readin)
 >      return;
 > @@ -3319,8 +3320,27 @@
 >  	         complaining about them.  */
 >  	      if (type_code & N_STAB)
 >  		{
 > -		  process_one_symbol (type_code, 0, valu, name,
 > -				      pst->section_offsets, pst->objfile);
 > +		  /* If we found a trailing N_SO with no name, process it here
 > +		     instead of in process_one_symbol, so we can keep its symtab. */
 > +		  if (type_code == N_SO
 > +		      && last_source_file
 > +		      && previous_stab_code != (unsigned char) N_SO
 > +		      && *name == '\000')
 > +		    {
 > +		      valu += ANOFFSET (pst->section_offsets,
 > +					SECT_OFF_TEXT (pst->objfile));
 > +		      previous_stab_code = N_SO;
 > +		      st = end_symtab (valu, pst->objfile,
 > +				       SECT_OFF_TEXT (pst->objfile));
 > +		      end_stabs ();
 > +		      last_symtab_ended = 1;
 > +		    }
 > +		  else
 > +		    {
 > +		      last_symtab_ended = 0;
 > +		      process_one_symbol (type_code, 0, valu, name,
 > +					  pst->section_offsets, pst->objfile);
 > +		    }
 >  		}
 >  	      /* Similarly a hack.  */
 >  	      else if (name[0] == '#')
 > @@ -3368,9 +3388,13 @@
 >  	    ;
 >  	  else
 >  	    complain (&stab_unknown_complaint, name);
 > +	}
 > +
 > +      if (! last_symtab_ended)
 > +	{
 > +	  st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
 > +	  end_stabs ();
 >  	}
 > -      st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
 > -      end_stabs ();
 >  
 >        /* Sort the symbol table now, we are done adding symbols to it.
 >           We must do this before parse_procedure calls lookup_symbol.  */

Re: [RFC] GDB's mdebug support vs. GCC 3.0

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Thu, Jul 19, 2001 at 06:29:53PM -0400, Elena Zannoni wrote:
 > > 
 > > Wow, what a messy control flow. Makes me dizzy.  I am starting to
 > > understand this patch a bit.  Just a few questions. Do you go through
 > > mipsread.c at all?  If so, does mipscoff_new_init get called?  If so,
 > > can you try to add the call to init_header_files in there instead?
 > 
 > The problem is that I don't go through mipsread at all.  What we have
 > here is mdebug-in-ELF; elfmdebug_read_psymtab is where we enter mdebug
 > from.

Ahhh, OK. Ulgh. So you have an elf file, and you go through elfread.c.
Let's see if I get the call stack right. Kind of hard to do w/o a
stack trace.

syms_from_objfile calls
elf_symfile_read calls
elfmdebug_build_psymtabs calls
mdebug_build_psymtabs calls
parse_partial_symbols which sets up the symtab_read pointer to 
mdebug_psymtab_to_symtab.

The function pointer is called by PSYMTAB_TO_SYMTAB
then psymtab_to_symtab_1 is called,
then process_one_symbol, 
then add_new_header_file and there you get the problem with the headers.

Since process_one_symbol is called by other readers as well, and I
assume the N_BINCL symbol is not new, there must be something upstream
that gets screwed up.

I'll go back to something similar to my initial suggestion, then, can you
try adding a call to init_header_files() from elf_new_init()?

This may fix your problem.

 > 
 > > Next problem:
 > > 
 > > Can you explain a bit more what happens there?  I see that your new
 > > code in the if branch does the same things that process_one_symbol would do.
 > > . Change valu by the offset
 > > . call end_symtab
 > > . call end_stabs
 > > 
 > > Are you saying that the symtab would be ended twice in that case? Once
 > > in process_one_symbol and once in psymtab_to_symtab_1?
 > > I think this the problem right?
 > > I am going to think some more.
 > 
 > That bit I'm not thrilled with.  You're right; we used to not get the
 > final N_SO at all, and so process_one_symbol would not call end_symtab,
 > and we'd be safe when we called it ourselves after the loop.  But GCC
 > 3.0 does emit these N_SO's.  We need to prevent process_one_symbol
 > (whose logic I'm not convinced we should be reusing on this path at
 > all, it's heinous!) from ending the symtab prematurely.
 > 

So if I read things correctly, the behavior of process_one_symbol is
correct.  It figures that the N_SO marks the end of the file, and
returns, w/o starting a new symtab. The problem is with the function
that calls it. 

Could you do something like this instead? It is a little cleaner.

Elena

 	      if (type_code & N_STAB)
 		{
-		  process_one_symbol (type_code, 0, valu, name,
-				      pst->section_offsets, pst->objfile);
+		  /* If we found a trailing N_SO with no name, we record this here,
                     so that the symtab will not be ended twice, once in process_one_symbol,
                     and once after this loop. */
+		  if (type_code == N_SO
+		      && last_source_file
+		      && previous_stab_code != (unsigned char) N_SO
+		      && *name == '\000')
+		    last_symtab_ended = 1;
+		  else
+		    last_symtab_ended = 0;
+
+		  process_one_symbol (type_code, 0, valu, name,
+				      pst->section_offsets, pst->objfile);

 		}
 	      /* Similarly a hack.  */
 	      else if (name[0] == '#')
@@ -3368,9 +3388,13 @@
 	    ;
 	  else
 	    complain (&stab_unknown_complaint, name);
+	}
+
+      if (! last_symtab_ended)
+	{
+	  st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
+	  end_stabs ();
 	}
-      st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
-      end_stabs ();

Re: [RFC] GDB's mdebug support vs. GCC 3.0

[Daniel Jacobowitz]

On Fri, Jul 20, 2001 at 12:17:17AM -0400, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Thu, Jul 19, 2001 at 06:29:53PM -0400, Elena Zannoni wrote:
>  > > 
>  > > Wow, what a messy control flow. Makes me dizzy.  I am starting to
>  > > understand this patch a bit.  Just a few questions. Do you go through
>  > > mipsread.c at all?  If so, does mipscoff_new_init get called?  If so,
>  > > can you try to add the call to init_header_files in there instead?
>  > 
>  > The problem is that I don't go through mipsread at all.  What we have
>  > here is mdebug-in-ELF; elfmdebug_read_psymtab is where we enter mdebug
>  > from.
> 
> Ahhh, OK. Ulgh. So you have an elf file, and you go through elfread.c.
> Let's see if I get the call stack right. Kind of hard to do w/o a
> stack trace.
> 
> syms_from_objfile calls
> elf_symfile_read calls
> elfmdebug_build_psymtabs calls
> mdebug_build_psymtabs calls
> parse_partial_symbols which sets up the symtab_read pointer to 
> mdebug_psymtab_to_symtab.
> 
> The function pointer is called by PSYMTAB_TO_SYMTAB
> then psymtab_to_symtab_1 is called,
> then process_one_symbol, 
> then add_new_header_file and there you get the problem with the headers.
> 
> Since process_one_symbol is called by other readers as well, and I
> assume the N_BINCL symbol is not new, there must be something upstream
> that gets screwed up.
> 
> I'll go back to something similar to my initial suggestion, then, can you
> try adding a call to init_header_files() from elf_new_init()?
> 
> This may fix your problem.

Actually, I fixed this problem in two places.  The other patch I sent
you a URL for (Subject: [rfa] eliminate some annoying mdebug-related
symtab crashes) calls init_header_files from mdebug_psymtab_to_symtab,
which was the best place I could find to do it from analogy with the
other readers.

> So if I read things correctly, the behavior of process_one_symbol is
> correct.  It figures that the N_SO marks the end of the file, and
> returns, w/o starting a new symtab. The problem is with the function
> that calls it. 
> 
> Could you do something like this instead? It is a little cleaner.

Won't work, I think.  If you do this:

> @@ -3368,9 +3388,13 @@
>  	    ;
>  	  else
>  	    complain (&stab_unknown_complaint, name);
> +	}
> +
> +      if (! last_symtab_ended)
> +	{
> +	  st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
> +	  end_stabs ();
>  	}
> -      st = end_symtab (pst->texthigh, pst->objfile, SECT_OFF_TEXT (pst->objfile));
> -      end_stabs ();

Then st may or may not be initialized.  We'll die when we go to sort
the symtab a little further down.

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