[PATCH RFA] partial-stab.h patch amendment

[PATCH RFA] partial-stab.h patch amendment

[Kevin Buettner]

The patch below is an amendment to the partial-stab.h patch that I
submitted earlier in the week:

    http://sources.redhat.com/ml/gdb-patches/2001-09/msg00003.html

In the above patch, I added some code which tests the return value of
find_stab_function_addr() to make sure that we're not attempting to
use a result which indicates an error condition.

However, there are actually two calls to find_stab_function_addr() in
partial-stab.h.  One is used to determine the address of global
function symbols (when this information is not provided in the debug
info) and the other is used to help set textlow for static function
symbols.  I added code to test the return value for the global
function symbol case.

In a conversation with Jim Blandy earlier today, he made two
observations:

    1) The call to find_stab_function_addr() should be error
       checked in both cases.

    2) We probably ought to be using the same mechanism to
       set CUR_SYMBOL_VALUE for the static function case as
       well as the global.

The patch below addresses both of these points.

	* partial-stab.h (case 'f'): Make SOFUN_ADDRESS_MAYBE_MISSING
	code match that used for case 'F'.

diff -u -p -r1.12 partial-stab.h
--- partial-stab.h	2001/08/15 05:02:28	1.12
+++ partial-stab.h	2001/09/05 21:35:30
@@ -595,10 +595,22 @@ switch (CUR_SYMBOL_TYPE)
 #ifdef SOFUN_ADDRESS_MAYBE_MISSING
 	/* Do not fix textlow==0 for .o or NLM files, as 0 is a legit
 	   value for the bottom of the text seg in those cases. */
-	if (pst && textlow_not_set)
+	if (CUR_SYMBOL_VALUE == ANOFFSET (objfile->section_offsets, 
+	                                  SECT_OFF_TEXT (objfile)))
 	  {
-	    pst->textlow =
+	    CORE_ADDR minsym_valu = 
 	      find_stab_function_addr (namestring, pst->filename, objfile);
+	    /* find_stab_function_addr will return 0 if the minimal
+	       symbol wasn't found.  (Unfortunately, this might also
+	       be a valid address.)  Anyway, if it *does* return 0,
+	       it is likely that the value was set correctly to begin
+	       with... */
+	    if (minsym_valu != 0)
+	      CUR_SYMBOL_VALUE = minsym_valu;
+	  }
+	if (pst && textlow_not_set)
+	  {
+	    pst->textlow = CUR_SYMBOL_VALUE;
 	    textlow_not_set = 0;
 	  }
 #endif

Re: [PATCH RFA] partial-stab.h patch amendment

[Kevin Buettner]

On Sep 5,  3:43pm, Kevin Buettner wrote:

> However, there are actually two calls to find_stab_function_addr() in
> partial-stab.h.  One is used to determine the address of global
> function symbols (when this information is not provided in the debug
> info) and the other is used to help set textlow for static function
> symbols.  I added code to test the return value for the global
> function symbol case.
> 
> In a conversation with Jim Blandy earlier today, he made two
> observations:
[...]
>     2) We probably ought to be using the same mechanism to
>        set CUR_SYMBOL_VALUE for the static function case as
>        well as the global.

The fact that these cases somehow diverged really bothered me.  So
I did some CVS digging to try to figure out why and when they diverged.
It turns out that they diverged after the following fix was committed:

1999-09-14  Kevin Buettner  <kevinb@cygnus.com>

	* symtab.h, minsyms.c (find_stab_function_addr): Changed
	type of second parameter from partial_symtab * to char *.
	Fixed all callers.
	* minsyms.c (find_stab_function_addr): Look for minimal
	symbol without filename if filename based search fails.
	* dbxread.c (process_one_symbol): Call find_stab_function_addr()
	in place of inline code with identical functionality.
	* partial-stab.h (case N_FUN, descriptors 'F' and 'f'): Look
	up symbol's address from minimal symbol table when N_FUN
	address is missing.  Also, make sure this value is used for
	calculating the value of the texthigh field.

(Yes, the culprit was me.)

Prior to that the code was identical...

2.65         (shebs    07-Jul-99):       case 'f':
2.66         (fnf      08-Aug-99): 	CUR_SYMBOL_VALUE += ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT);
2.8          (gnu      13-Jun-92): #ifdef DBXREAD_ONLY
2.65         (shebs    07-Jul-99): 	/* Keep track of the start of the last function so we
2.65         (shebs    07-Jul-99): 	   can handle end of function symbols.  */
2.65         (shebs    07-Jul-99): 	last_function_start = CUR_SYMBOL_VALUE;
2.65         (shebs    07-Jul-99): 	/* Kludges for ELF/STABS with Sun ACC */
2.65         (shebs    07-Jul-99): 	last_function_name = namestring;
2.42         (kingdon  09-Feb-95): #ifdef SOFUN_ADDRESS_MAYBE_MISSING
2.65         (shebs    07-Jul-99): 	/* Do not fix textlow==0 for .o or NLM files, as 0 is a legit
2.65         (shebs    07-Jul-99): 	   value for the bottom of the text seg in those cases. */
2.65         (shebs    07-Jul-99): 	if (pst && textlow_not_set)
2.65         (shebs    07-Jul-99): 	  {
2.65         (shebs    07-Jul-99): 	    pst->textlow =
2.65         (shebs    07-Jul-99): 	      find_stab_function_addr (namestring, pst, objfile);
2.65         (shebs    07-Jul-99): 	    textlow_not_set = 0;
2.65         (shebs    07-Jul-99): 	  }
2.42         (kingdon  09-Feb-95): #endif
....
2.65         (shebs    07-Jul-99):       case 'F':
2.66         (fnf      08-Aug-99): 	CUR_SYMBOL_VALUE += ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT);
2.8          (gnu      13-Jun-92): #ifdef DBXREAD_ONLY
2.65         (shebs    07-Jul-99): 	/* Keep track of the start of the last function so we
2.65         (shebs    07-Jul-99): 	   can handle end of function symbols.  */
2.65         (shebs    07-Jul-99): 	last_function_start = CUR_SYMBOL_VALUE;
2.65         (shebs    07-Jul-99): 	/* Kludges for ELF/STABS with Sun ACC */
2.65         (shebs    07-Jul-99): 	last_function_name = namestring;
2.42         (kingdon  09-Feb-95): #ifdef SOFUN_ADDRESS_MAYBE_MISSING
2.65         (shebs    07-Jul-99): 	/* Do not fix textlow==0 for .o or NLM files, as 0 is a legit
2.65         (shebs    07-Jul-99): 	   value for the bottom of the text seg in those cases. */
2.65         (shebs    07-Jul-99): 	if (pst && textlow_not_set)
2.65         (shebs    07-Jul-99): 	  {
2.65         (shebs    07-Jul-99): 	    pst->textlow =
2.65         (shebs    07-Jul-99): 	      find_stab_function_addr (namestring, pst, objfile);
2.65         (shebs    07-Jul-99): 	    textlow_not_set = 0;
2.65         (shebs    07-Jul-99): 	  }
2.8          (gnu      13-Jun-92): #endif

Anyway, since I can't remember any good reason for making these cases
diverge, I'm not bothered anymore about making them the same again.

Kevin

Re: [PATCH RFA] partial-stab.h patch amendment

[Jim Blandy]

These patches are approved as amended.

(When I say things like that I feel like I should break out a copy of
Robert's Rules or something.)

Re: [PATCH RFA] partial-stab.h patch amendment

[Kevin Buettner]

On Sep 6, 12:36pm, Jim Blandy wrote:

> These patches are approved as amended.

Committed.  (Thanks.)

> (When I say things like that I feel like I should break out a copy of
> Robert's Rules or something.)

Let the record show that the following patch was committed:

	* dbxread.c (process_one_symbol): Don't use error result from
	find_stab_function_addr().
	* partial-stab.h (case 'F'): Likewise.

	* partial-stab.h (case 'f'): Make SOFUN_ADDRESS_MAYBE_MISSING
	code match that used for case 'F'.  This fixes the divergence
	that was introduced by my 1999-09-14 changes to partial-stab.h.

Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.22
diff -u -p -r1.22 dbxread.c
--- dbxread.c	2001/09/05 02:54:15	1.22
+++ dbxread.c	2001/09/06 20:34:35
@@ -2266,8 +2266,18 @@ process_one_symbol (int type, int desc, 
 	         from N_FUN symbols.  */
 	      if (type == N_FUN
 		  && valu == ANOFFSET (section_offsets, SECT_OFF_TEXT (objfile)))
-		valu = 
-		  find_stab_function_addr (name, last_source_file, objfile);
+		{
+		  CORE_ADDR minsym_valu = 
+		    find_stab_function_addr (name, last_source_file, objfile);
+
+		  /* find_stab_function_addr will return 0 if the minimal
+		     symbol wasn't found.  (Unfortunately, this might also
+		     be a valid address.)  Anyway, if it *does* return 0,
+		     it is likely that the value was set correctly to begin
+		     with... */
+		  if (minsym_valu != 0)
+		    valu = minsym_valu;
+		}
 #endif
 
 #ifdef SUN_FIXED_LBRAC_BUG
Index: partial-stab.h
===================================================================
RCS file: /cvs/src/src/gdb/partial-stab.h,v
retrieving revision 1.12
diff -u -p -r1.12 partial-stab.h
--- partial-stab.h	2001/08/15 05:02:28	1.12
+++ partial-stab.h	2001/09/06 20:34:36
@@ -595,10 +595,22 @@ switch (CUR_SYMBOL_TYPE)
 #ifdef SOFUN_ADDRESS_MAYBE_MISSING
 	/* Do not fix textlow==0 for .o or NLM files, as 0 is a legit
 	   value for the bottom of the text seg in those cases. */
-	if (pst && textlow_not_set)
+	if (CUR_SYMBOL_VALUE == ANOFFSET (objfile->section_offsets, 
+	                                  SECT_OFF_TEXT (objfile)))
 	  {
-	    pst->textlow =
+	    CORE_ADDR minsym_valu = 
 	      find_stab_function_addr (namestring, pst->filename, objfile);
+	    /* find_stab_function_addr will return 0 if the minimal
+	       symbol wasn't found.  (Unfortunately, this might also
+	       be a valid address.)  Anyway, if it *does* return 0,
+	       it is likely that the value was set correctly to begin
+	       with... */
+	    if (minsym_valu != 0)
+	      CUR_SYMBOL_VALUE = minsym_valu;
+	  }
+	if (pst && textlow_not_set)
+	  {
+	    pst->textlow = CUR_SYMBOL_VALUE;
 	    textlow_not_set = 0;
 	  }
 #endif
@@ -652,8 +664,17 @@ switch (CUR_SYMBOL_TYPE)
 	   value for the bottom of the text seg in those cases. */
 	if (CUR_SYMBOL_VALUE == ANOFFSET (objfile->section_offsets, 
 	                                  SECT_OFF_TEXT (objfile)))
-	  CUR_SYMBOL_VALUE = 
-	    find_stab_function_addr (namestring, pst->filename, objfile);
+	  {
+	    CORE_ADDR minsym_valu = 
+	      find_stab_function_addr (namestring, pst->filename, objfile);
+	    /* find_stab_function_addr will return 0 if the minimal
+	       symbol wasn't found.  (Unfortunately, this might also
+	       be a valid address.)  Anyway, if it *does* return 0,
+	       it is likely that the value was set correctly to begin
+	       with... */
+	    if (minsym_valu != 0)
+	      CUR_SYMBOL_VALUE = minsym_valu;
+	  }
 	if (pst && textlow_not_set)
 	  {
 	    pst->textlow = CUR_SYMBOL_VALUE;

Re: [PATCH RFA] partial-stab.h patch amendment

[H . J . Lu]

On Thu, Sep 06, 2001 at 04:20:48PM -0700, Kevin Buettner wrote:
> 
> If you read H.J.'s message, you'll see that __strtol_internal appears
> in libdl.so.2 as a weak undefined symbol.  It got that way presumably
> through a bug somewhere else in the toolchain (linker perhaps?) which
> converted it from a weak defined symbol to a weak undefined symbol.
> Yet, in the process, the stabs entries related to this symbol weren't
> removed.  Thus we end up having stabs entries which describe an
> undefined symbol.

Is there a way to remove a stabs entry in this case?

> 
> I don't really expect any comments on this message, but I wanted
> write down what I know of the matter before moving on to something
> else...
> 

I tried your patch on gdb 5.1. It works for me. Can we have it in
gdb 5.1?

Thanks.


H.J.

Re: [PATCH RFA] partial-stab.h patch amendment

[Kevin Buettner]

On Sep 6, 11:00pm, H . J . Lu wrote:

> > If you read H.J.'s message, you'll see that __strtol_internal appears
> > in libdl.so.2 as a weak undefined symbol.  It got that way presumably
> > through a bug somewhere else in the toolchain (linker perhaps?) which
> > converted it from a weak defined symbol to a weak undefined symbol.
> > Yet, in the process, the stabs entries related to this symbol weren't
> > removed.  Thus we end up having stabs entries which describe an
> > undefined symbol.
> 
> Is there a way to remove a stabs entry in this case?

In theory, yes.  If the minsym reader kept track of the undefined symbols
that it ran across, the stabs reader could check to see if it's attempting
to read an entry for an undefined symbol and discard it.  However, the
minsym reader (e.g. see elf_symtab_read in elfread.c) doesn't keep track
of the undefined symbols...

In my opinion, it'd be better for some other part of the toolchain
(i.e. not GDB) to remove the appropriate stabs entries when a symbol
is converted from being weak defined to (weak) undefined.  (Is there
any difference between ``undefined'' and ``weak undefined''?)


> I tried your patch on gdb 5.1. It works for me. Can we have it in
> gdb 5.1?

Good question.  I think it should go in, but I'm not the keeper of the
branch.

Andrew, what do you say?

Re: [PATCH RFA] partial-stab.h patch amendment

[H . J . Lu]

On Fri, Sep 07, 2001 at 09:53:29AM -0700, Kevin Buettner wrote:
> > 
> > Is there a way to remove a stabs entry in this case?
> 
> In my opinion, it'd be better for some other part of the toolchain
> (i.e. not GDB) to remove the appropriate stabs entries when a symbol

I meent to ask if there was a way to remove a stabs entry by the
linker.

> is converted from being weak defined to (weak) undefined.  (Is there
> any difference between ``undefined'' and ``weak undefined''?)

I don't think so. We are trying to find out if it is a linker bug.
But I don't have access to Solaris to verify it myself. I'd like to
know what the linker should do for a weak definition when

1. There is a strong definition in another relocatable file before it.
2. There is a strong definition in another relocatable file after it.
3. There is a strong definition in a DSO before it.
4. There is a strong definition in a DSO after it.

> 
> 
> > I tried your patch on gdb 5.1. It works for me. Can we have it in
> > gdb 5.1?
> 
> Good question.  I think it should go in, but I'm not the keeper of the
> branch.
> 
> Andrew, what do you say?

Please. I really appreciate it.

Thanks.


H.J.

Re: [PATCH RFA] partial-stab.h patch amendment

[Andrew Cagney]

> I tried your patch on gdb 5.1. It works for me. Can we have it in
>> gdb 5.1?
> 
> 
> Good question.  I think it should go in, but I'm not the keeper of the
> branch.
> 
> Andrew, what do you say?

not my problem :-)  partial-symtab.h is Elena & JimB's call.

	Andrew

Re: [PATCH RFA] partial-stab.h patch amendment

[Elena Zannoni]

Andrew Cagney wrote:
> 
> > I tried your patch on gdb 5.1. It works for me. Can we have it in
> >> gdb 5.1?
> >
> >
> > Good question.  I think it should go in, but I'm not the keeper of the
> > branch.
> >
> > Andrew, what do you say?
> 
> not my problem :-)  partial-symtab.h is Elena & JimB's call.
> 
>         Andrew

Yes, I need to reread the whole thread. It will take a while.
I am also in the middle of changing that stuff.

Elena

Re: [PATCH RFA] partial-stab.h patch amendment

[Jim Blandy]

Andrew Cagney <ac131313@cygnus.com> writes:
> > I tried your patch on gdb 5.1. It works for me. Can we have it in
> >> gdb 5.1?
> > 
> > 
> > Good question.  I think it should go in, but I'm not the keeper of the
> > branch.
> > 
> > Andrew, what do you say?
> 
> not my problem :-)  partial-symtab.h is Elena & JimB's call.

I've already approved it for the mainline sources.  I don't see why it
shouldn't go into the branch as well.

Re: [PATCH RFA] partial-stab.h patch amendment

[Elena Zannoni]

Ok, yes, this patch can go in 5.1. It is not going to interfere
with what I am doing.
If it's OK with Jim, Kevin, can you commit it?

Elena


Andrew Cagney wrote:
> 
> > I tried your patch on gdb 5.1. It works for me. Can we have it in
> >> gdb 5.1?
> >
> >
> > Good question.  I think it should go in, but I'm not the keeper of the
> > branch.
> >
> > Andrew, what do you say?
> 
> not my problem :-)  partial-symtab.h is Elena & JimB's call.
> 
>         Andrew