[RFA] Fix for searching for data symbols

[RFA] Fix for searching for data symbols

[Daniel Jacobowitz]

This patch fixes the problem I described in:
	http://sources.redhat.com/ml/gdb/2002-01/msg00223.html

Basically, since we use the bounds of the text segment to search for symbols
in find_pc_*, don't let them return a bogus answer for a symbol we know is
off in data land.

OK to commit, or does anyone prefer a different approach to this?

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

2002-01-19  Daniel Jacobowitz  <drow@mvista.com>

	* symtab.c (find_pc_sect_psymtab): Do not search psymtabs for
	data symbols, since we search based on textlow and texthigh.
	(find_pc_sect_symtab): Likewise.

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.51
diff -u -p -r1.51 symtab.c
--- symtab.c	2001/12/21 22:32:37	1.51
+++ symtab.c	2002/01/19 17:29:09
@@ -354,12 +354,22 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 {
   register struct partial_symtab *pst;
   register struct objfile *objfile;
+  struct minimal_symbol *msymbol;
 
+  /* If we know that this is not a text address, return failure.  */
+  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
+  if (msymbol
+      && (msymbol->type == mst_data
+	  || msymbol->type == mst_bss
+	  || msymbol->type == mst_abs
+	  || msymbol->type == mst_file_data
+	  || msymbol->type == mst_file_bss))
+    return NULL;
+
   ALL_PSYMTABS (objfile, pst)
   {
     if (pc >= pst->textlow && pc < pst->texthigh)
       {
-	struct minimal_symbol *msymbol;
 	struct partial_symtab *tpst;
 
 	/* An objfile that has its functions reordered might have
@@ -370,7 +380,6 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 	    section == 0)	/* can't validate section this way */
 	  return (pst);
 
-	msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
 	if (msymbol == NULL)
 	  return (pst);
 
@@ -1385,6 +1394,17 @@ find_pc_sect_symtab (CORE_ADDR pc, asect
   register struct partial_symtab *ps;
   register struct objfile *objfile;
   CORE_ADDR distance = 0;
+  struct minimal_symbol *msymbol;
+
+  /* If we know that this is not a text address, return failure.  */
+  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
+  if (msymbol
+      && (msymbol->type == mst_data
+	  || msymbol->type == mst_bss
+	  || msymbol->type == mst_abs
+	  || msymbol->type == mst_file_data
+	  || msymbol->type == mst_file_bss))
+    return NULL;
 
   /* Search all symtabs for the one whose file contains our address, and which
      is the smallest of all the ones containing the address.  This is designed

Re: [RFA] Fix for searching for data symbols

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> This patch fixes the problem I described in:
>         http://sources.redhat.com/ml/gdb/2002-01/msg00223.html
> 
> Basically, since we use the bounds of the text segment to search for symbols
> in find_pc_*, don't let them return a bogus answer for a symbol we know is
> off in data land.
> 
> OK to commit, or does anyone prefer a different approach to this?

Is there any chance that a data symbol that we don't want
would shadow a text symbol that we do want?  Eg. maybe in
one module we have
	static int foo;
and in another we have
	extern int foo(void);
Maybe they are even in separate objfiles (shared libs).
Maybe lookup_minimal_symbol will find the wrong one first, 
and your change will terminate the search before we would
have found the right one?

Just speculating...

> 
> --
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2002-01-19  Daniel Jacobowitz  <drow@mvista.com>
> 
>         * symtab.c (find_pc_sect_psymtab): Do not search psymtabs for
>         data symbols, since we search based on textlow and texthigh.
>         (find_pc_sect_symtab): Likewise.
> 
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 symtab.c
> --- symtab.c    2001/12/21 22:32:37     1.51
> +++ symtab.c    2002/01/19 17:29:09
> @@ -354,12 +354,22 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
>  {
>    register struct partial_symtab *pst;
>    register struct objfile *objfile;
> +  struct minimal_symbol *msymbol;
> 
> +  /* If we know that this is not a text address, return failure.  */
> +  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
> +  if (msymbol
> +      && (msymbol->type == mst_data
> +         || msymbol->type == mst_bss
> +         || msymbol->type == mst_abs
> +         || msymbol->type == mst_file_data
> +         || msymbol->type == mst_file_bss))
> +    return NULL;
> +
>    ALL_PSYMTABS (objfile, pst)
>    {
>      if (pc >= pst->textlow && pc < pst->texthigh)
>        {
> -       struct minimal_symbol *msymbol;
>         struct partial_symtab *tpst;
> 
>         /* An objfile that has its functions reordered might have
> @@ -370,7 +380,6 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
>             section == 0)       /* can't validate section this way */
>           return (pst);
> 
> -       msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
>         if (msymbol == NULL)
>           return (pst);
> 
> @@ -1385,6 +1394,17 @@ find_pc_sect_symtab (CORE_ADDR pc, asect
>    register struct partial_symtab *ps;
>    register struct objfile *objfile;
>    CORE_ADDR distance = 0;
> +  struct minimal_symbol *msymbol;
> +
> +  /* If we know that this is not a text address, return failure.  */
> +  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
> +  if (msymbol
> +      && (msymbol->type == mst_data
> +         || msymbol->type == mst_bss
> +         || msymbol->type == mst_abs
> +         || msymbol->type == mst_file_data
> +         || msymbol->type == mst_file_bss))
> +    return NULL;
> 
>    /* Search all symtabs for the one whose file contains our address, and which
>       is the smallest of all the ones containing the address.  This is designed

Re: [RFA] Fix for searching for data symbols

[Daniel Jacobowitz]

On Sat, Jan 19, 2002 at 09:52:22AM -0800, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> > 
> > This patch fixes the problem I described in:
> >         http://sources.redhat.com/ml/gdb/2002-01/msg00223.html
> > 
> > Basically, since we use the bounds of the text segment to search for symbols
> > in find_pc_*, don't let them return a bogus answer for a symbol we know is
> > off in data land.
> > 
> > OK to commit, or does anyone prefer a different approach to this?
> 
> Is there any chance that a data symbol that we don't want
> would shadow a text symbol that we do want?  Eg. maybe in
> one module we have
> 	static int foo;
> and in another we have
> 	extern int foo(void);
> Maybe they are even in separate objfiles (shared libs).
> Maybe lookup_minimal_symbol will find the wrong one first, 
> and your change will terminate the search before we would
> have found the right one?
> 
> Just speculating...

Well, we aren't using lookup_minimal_symbol; we're using
lookup_minimal_symbol_by_pc_section.  I don't think it's possible for
it to get the wrong one, since it's by address instead of by name.


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

Re: [RFA] Fix for searching for data symbols

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> On Sat, Jan 19, 2002 at 09:52:22AM -0800, Michael Snyder wrote:
> > Daniel Jacobowitz wrote:
> > >
> > > This patch fixes the problem I described in:
> > >         http://sources.redhat.com/ml/gdb/2002-01/msg00223.html
> > >
> > > Basically, since we use the bounds of the text segment to search for symbols
> > > in find_pc_*, don't let them return a bogus answer for a symbol we know is
> > > off in data land.
> > >
> > > OK to commit, or does anyone prefer a different approach to this?
> >
> > Is there any chance that a data symbol that we don't want
> > would shadow a text symbol that we do want?  Eg. maybe in
> > one module we have
> >       static int foo;
> > and in another we have
> >       extern int foo(void);
> > Maybe they are even in separate objfiles (shared libs).
> > Maybe lookup_minimal_symbol will find the wrong one first,
> > and your change will terminate the search before we would
> > have found the right one?
> >
> > Just speculating...
> 
> Well, we aren't using lookup_minimal_symbol; we're using
> lookup_minimal_symbol_by_pc_section.  I don't think it's possible for
> it to get the wrong one, since it's by address instead of by name.

OK, good, that answers that concern.  Thanks.

Re: [RFA] Fix for searching for data symbols

[Daniel Jacobowitz]

On Sat, Jan 19, 2002 at 12:33:18PM -0500, Daniel Jacobowitz wrote:
> This patch fixes the problem I described in:
> 	http://sources.redhat.com/ml/gdb/2002-01/msg00223.html
> 
> Basically, since we use the bounds of the text segment to search for symbols
> in find_pc_*, don't let them return a bogus answer for a symbol we know is
> off in data land.
> 
> OK to commit, or does anyone prefer a different approach to this?

Nobody ever commented on this, except for Michael Snyder's one
objection which I believe I've answered to his satisfaction.  Could
this be approved, please?


> 2002-01-19  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* symtab.c (find_pc_sect_psymtab): Do not search psymtabs for
> 	data symbols, since we search based on textlow and texthigh.
> 	(find_pc_sect_symtab): Likewise.
> 
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 symtab.c
> --- symtab.c	2001/12/21 22:32:37	1.51
> +++ symtab.c	2002/01/19 17:29:09
> @@ -354,12 +354,22 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
>  {
>    register struct partial_symtab *pst;
>    register struct objfile *objfile;
> +  struct minimal_symbol *msymbol;
>  
> +  /* If we know that this is not a text address, return failure.  */
> +  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
> +  if (msymbol
> +      && (msymbol->type == mst_data
> +	  || msymbol->type == mst_bss
> +	  || msymbol->type == mst_abs
> +	  || msymbol->type == mst_file_data
> +	  || msymbol->type == mst_file_bss))
> +    return NULL;
> +
>    ALL_PSYMTABS (objfile, pst)
>    {
>      if (pc >= pst->textlow && pc < pst->texthigh)
>        {
> -	struct minimal_symbol *msymbol;
>  	struct partial_symtab *tpst;
>  
>  	/* An objfile that has its functions reordered might have
> @@ -370,7 +380,6 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
>  	    section == 0)	/* can't validate section this way */
>  	  return (pst);
>  
> -	msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
>  	if (msymbol == NULL)
>  	  return (pst);
>  
> @@ -1385,6 +1394,17 @@ find_pc_sect_symtab (CORE_ADDR pc, asect
>    register struct partial_symtab *ps;
>    register struct objfile *objfile;
>    CORE_ADDR distance = 0;
> +  struct minimal_symbol *msymbol;
> +
> +  /* If we know that this is not a text address, return failure.  */
> +  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
> +  if (msymbol
> +      && (msymbol->type == mst_data
> +	  || msymbol->type == mst_bss
> +	  || msymbol->type == mst_abs
> +	  || msymbol->type == mst_file_data
> +	  || msymbol->type == mst_file_bss))
> +    return NULL;
>  
>    /* Search all symtabs for the one whose file contains our address, and which
>       is the smallest of all the ones containing the address.  This is designed
> 

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

Re: [RFA] Fix for searching for data symbols

[Elena Zannoni]

Daniel Jacobowitz writes:
 > This patch fixes the problem I described in:
 > 	http://sources.redhat.com/ml/gdb/2002-01/msg00223.html
 > 
 > Basically, since we use the bounds of the text segment to search for symbols
 > in find_pc_*, don't let them return a bogus answer for a symbol we know is
 > off in data land.
 > 
 > OK to commit, or does anyone prefer a different approach to this?
 > 

I think it's fine. Just could you add comments with an explanation of
the odd case that you described in the mail above? 

Thanks
Elena

 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2002-01-19  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* symtab.c (find_pc_sect_psymtab): Do not search psymtabs for
 > 	data symbols, since we search based on textlow and texthigh.
 > 	(find_pc_sect_symtab): Likewise.
 > 
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.51
 > diff -u -p -r1.51 symtab.c
 > --- symtab.c	2001/12/21 22:32:37	1.51
 > +++ symtab.c	2002/01/19 17:29:09
 > @@ -354,12 +354,22 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 >  {
 >    register struct partial_symtab *pst;
 >    register struct objfile *objfile;
 > +  struct minimal_symbol *msymbol;
 >  
 > +  /* If we know that this is not a text address, return failure.  */
 > +  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
 > +  if (msymbol
 > +      && (msymbol->type == mst_data
 > +	  || msymbol->type == mst_bss
 > +	  || msymbol->type == mst_abs
 > +	  || msymbol->type == mst_file_data
 > +	  || msymbol->type == mst_file_bss))
 > +    return NULL;
 > +
 >    ALL_PSYMTABS (objfile, pst)
 >    {
 >      if (pc >= pst->textlow && pc < pst->texthigh)
 >        {
 > -	struct minimal_symbol *msymbol;
 >  	struct partial_symtab *tpst;
 >  
 >  	/* An objfile that has its functions reordered might have
 > @@ -370,7 +380,6 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 >  	    section == 0)	/* can't validate section this way */
 >  	  return (pst);
 >  
 > -	msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
 >  	if (msymbol == NULL)
 >  	  return (pst);
 >  
 > @@ -1385,6 +1394,17 @@ find_pc_sect_symtab (CORE_ADDR pc, asect
 >    register struct partial_symtab *ps;
 >    register struct objfile *objfile;
 >    CORE_ADDR distance = 0;
 > +  struct minimal_symbol *msymbol;
 > +
 > +  /* If we know that this is not a text address, return failure.  */
 > +  msymbol = lookup_minimal_symbol_by_pc_section (pc, section);
 > +  if (msymbol
 > +      && (msymbol->type == mst_data
 > +	  || msymbol->type == mst_bss
 > +	  || msymbol->type == mst_abs
 > +	  || msymbol->type == mst_file_data
 > +	  || msymbol->type == mst_file_bss))
 > +    return NULL;
 >  
 >    /* Search all symtabs for the one whose file contains our address, and which
 >       is the smallest of all the ones containing the address.  This is designed

Re: [RFA] Fix for searching for data symbols

[Daniel Jacobowitz]

On Wed, Jan 30, 2002 at 06:26:08PM -0500, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > This patch fixes the problem I described in:
>  > 	http://sources.redhat.com/ml/gdb/2002-01/msg00223.html
>  > 
>  > Basically, since we use the bounds of the text segment to search for symbols
>  > in find_pc_*, don't let them return a bogus answer for a symbol we know is
>  > off in data land.
>  > 
>  > OK to commit, or does anyone prefer a different approach to this?
>  > 
> 
> I think it's fine. Just could you add comments with an explanation of
> the odd case that you described in the mail above? 

Committed with comments, thanks!

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