[patch] Fix uninitialized section index internal error

[patch] Fix uninitialized section index internal error

[Joel Brobecker]

Hi,

  it has been pointed to me that it is better to put the change log
inside the mail body rather than putting it inside the patch. So, I'll
start over. Please discard my previous message. Thanks.

While working with gdb on Tru64 5.1, we noticed the following internal
error sometimes happening at the begining of a gdb session:

| gdb/mdebugread.c:2448: gdb-internal-error: Section index is uninitialized
|
| An internal GDB error was detected.  This may make further
| debugging unreliable.  Continue this debugging session? (y or n) n
    
To reproduce the problem, simply compile the following C program:

mach.c:
<<
int
main (void)
{
   return 0;
};
>>

Make sure libmach is linked in when building the program:
% gcc -o mach mach.c -lmach   

After for investigation, I found that gdb was looking for a symbol in
the .bss section of libmach.so, but there is none (which explains why
the section index is not initialized). Instead, there is a .sbss
section, where the symbol is localized.

I modified default_symfile_offsets () to use the .sbss section if the
.bss one does not exist. In that change, I am assuming that a bss and a
sbss section are mutually exclusive.

This change has been integrated in ACT's version of gdb a few months
ago, and has worked well so far.

Here is the change log:

2001-05-10  J. Brobecker <brobecker@act-europe.fr>

        * symfile.c (default_symfile_offsets): use the .sbss section in
        place of the .bss section when the latter does not exist.

-- 
Joel
From ac131313@cygnus.com Thu May 10 15:30:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: [patch] Re-org mi/mi-out.c so that it is (almost) tupple aware
Date: Thu, 10 May 2001 15:30:00 -0000
Message-id: <3AFB167F.4030807@cygnus.com>
X-SW-Source: 2001-05/msg00188.html
Content-length: 152

FYI,

The attached patch tweeks mi/mi-out.c so that it has the tupple/list 
parameter available where needed.  It doesn't actually use it yet.

	Andrew
From ac131313@cygnus.com Thu May 10 15:56:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: Andrew Cagney <ac131313@cygnus.com>
Cc: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: Re: [rfc] Move Makefile.in:VERSION to VERSION file
Date: Thu, 10 May 2001 15:56:00 -0000
Message-id: <3AFB1C97.30009@cygnus.com>
References: <3AB3CB1D.85E1E043@cygnus.com>
X-SW-Source: 2001-05/msg00189.html
Content-length: 743

FYI,  I'm about to check in the below but with s/VERSION/version/ for 
the file.

Anyone like to come up with a 5 line /bin/sh script that updates this 
file using CVS.  It would need to remember to use date -u and should 
probably check that it hasn't already been run today (1).  The path to 
the CVS repository and the path to the file would be arguments.


	Andrew

(1) Due to the timezone scrample and the possibility of the CVS server 
going down, I'm thinking of cheating and running the cronjob twice (if 
not more :-)

2001-03-17  Andrew Cagney  <ac131313@redhat.com>

* Makefile.in (VERSION): Delete.  Moved to file VERSION.
	(version.c): Depends on file VERSION.  Extract version number from
	VERSION file.
	* VERSION: New file.
	

Re: [patch] Fix uninitialized section index internal error

[Elena Zannoni]

Hi Joel, thanks for the patch. 
See my comments below.


Joel Brobecker writes:
 > Hi,
 > 
 >   it has been pointed to me that it is better to put the change log
 > inside the mail body rather than putting it inside the patch. So, I'll
 > start over. Please discard my previous message. Thanks.
 > 
 > While working with gdb on Tru64 5.1, we noticed the following internal
 > error sometimes happening at the begining of a gdb session:
 > 
 > | gdb/mdebugread.c:2448: gdb-internal-error: Section index is uninitialized
 > |
 > | An internal GDB error was detected.  This may make further
 > | debugging unreliable.  Continue this debugging session? (y or n) n
 >     

I see what's happening, your symbol is in section scSBss and not in scBss.
This macro in mdebugread.c:
SC_IS_BSS(sc) ((sc) == scBss || (sc) == scSBss)

evaluates to true, so you get to execute:

	  else if (SC_IS_BSS (ext_in->asym.sc))
	    {
	      ms_type = mst_bss;
	      svalue += ANOFFSET (objfile->section_offsets, SECT_OFF_BSS (objfile));

or (a few lines below):

	  else if (SC_IS_BSS (ext_in->asym.sc))
	    {
	      ms_type = mst_file_bss;
	      svalue += ANOFFSET (objfile->section_offsets, SECT_OFF_BSS (objfile));
	    }


The SECT_OFF_BSS then is -1. It shouldn't be used in this case,
because the symbol may not be in the bss section.

That said, may I suggest a different approach?  I think the bug is
really in mdebugread.c, not symfile.c.  I think the SC_IS_BSS macro
should be split in 2 separate ones: a proper SC_IS_BSS and another
SC_IS_SBSS for your case.  Then the above code would work ok, if you
don't have the .bss section, because it wouldn't trigger.  As for the
SC_IS_SBSS case, you would have to figure out what the correct value
to be added to svalue needs to be, and for this you need to find the
index of the .sbss section.

Thanks
Elena


 > To reproduce the problem, simply compile the following C program:
 > 
 > mach.c:
 > <<
 > int
 > main (void)
 > {
 >    return 0;
 > };
 > >>
 > 
 > Make sure libmach is linked in when building the program:
 > % gcc -o mach mach.c -lmach   
 > 
 > After for investigation, I found that gdb was looking for a symbol in
 > the .bss section of libmach.so, but there is none (which explains why
 > the section index is not initialized). Instead, there is a .sbss
 > section, where the symbol is localized.
 > 
 > I modified default_symfile_offsets () to use the .sbss section if the
 > .bss one does not exist. In that change, I am assuming that a bss and a
 > sbss section are mutually exclusive.
 > 
 > This change has been integrated in ACT's version of gdb a few months
 > ago, and has worked well so far.
 > 
 > Here is the change log:
 > 
 > 2001-05-10  J. Brobecker <brobecker@act-europe.fr>
 > 
 >         * symfile.c (default_symfile_offsets): use the .sbss section in
 >         place of the .bss section when the latter does not exist.
 > 
 > -- 
 > Joel
 > Index: gdb/symfile.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symfile.c,v
 > retrieving revision 1.31
 > diff -c -3 -p -r1.31 symfile.c
 > *** symfile.c	2001/04/05 02:02:13	1.31
 > --- symfile.c	2001/05/10 19:41:39
 > *************** default_symfile_offsets (struct objfile 
 > *** 529,534 ****
 > --- 529,536 ----
 >       objfile->sect_index_data = sect->index;
 >   
 >     sect = bfd_get_section_by_name (objfile->obfd, ".bss");
 > +   if (!sect)
 > +     sect = bfd_get_section_by_name (objfile->obfd, ".sbss");
 >     if (sect) 
 >       objfile->sect_index_bss = sect->index;
 >   

Re: [patch] Fix uninitialized section index internal error

[Joel Brobecker]

Hi Elena,

  thank you very much for reviewing this patch.

> I see what's happening, your symbol is in section scSBss and not in scBss.
> This macro in mdebugread.c:
> SC_IS_BSS(sc) ((sc) == scBss || (sc) == scSBss)
> 
> evaluates to true, so you get to execute:
> 
> 	  else if (SC_IS_BSS (ext_in->asym.sc))
> 	    {
> 	      ms_type = mst_bss;
> 	      svalue += ANOFFSET (objfile->section_offsets, SECT_OFF_BSS (objfile));
> 
> or (a few lines below):
> 
> 	  else if (SC_IS_BSS (ext_in->asym.sc))
> 	    {
> 	      ms_type = mst_file_bss;
> 	      svalue += ANOFFSET (objfile->section_offsets, SECT_OFF_BSS (objfile));
> 	    }
> 
> 
> The SECT_OFF_BSS then is -1. It shouldn't be used in this case,
> because the symbol may not be in the bss section.
> 
> That said, may I suggest a different approach?  I think the bug is
> really in mdebugread.c, not symfile.c.  I think the SC_IS_BSS macro
> should be split in 2 separate ones: a proper SC_IS_BSS and another
> SC_IS_SBSS for your case.  Then the above code would work ok, if you
> don't have the .bss section, because it wouldn't trigger.  As for the
> SC_IS_SBSS case, you would have to figure out what the correct value
> to be added to svalue needs to be, and for this you need to find the
> index of the .sbss section.

  Ok, our patch was based on the assumption that the .bss and .sbss
sections were mutually exclusive. Your approach is more solid. 

May I suggest then to go a little further with your approach, which
cries out for the addition of a new sect_index_sbss in the objfile
structure:
  - in struct objfile (objfiles.h), add a new section index called
    sect_index_sbss
  - still in objfiles.h, add a new SECT_OFF_SBSS macro
  - initialize the sbss section index in default_symfile_offset
    (symfile.c) at the same time we're initializing the indexes of
    the other common sections
  - in mdebugread.c, fix the SC_IS_BSS macro, add a new SC_IS_SBSS
    macro, and then update the parse_partial_symbols () procedure
    to use the .sbss section if the symbol is there

If you agree with this approach, I'll then submit the new patch.

Thanks,
-- 
Joel

Re: [patch] Fix uninitialized section index internal error

[Elena Zannoni]

Joel Brobecker writes:
 > Hi Elena,
 > 
 >   thank you very much for reviewing this patch.
 > 
 > > I see what's happening, your symbol is in section scSBss and not in scBss.
 > > This macro in mdebugread.c:
 > > SC_IS_BSS(sc) ((sc) == scBss || (sc) == scSBss)
 > > 
 > > evaluates to true, so you get to execute:
 > > 
 > > 	  else if (SC_IS_BSS (ext_in->asym.sc))
 > > 	    {
 > > 	      ms_type = mst_bss;
 > > 	      svalue += ANOFFSET (objfile->section_offsets, SECT_OFF_BSS (objfile));
 > > 
 > > or (a few lines below):
 > > 
 > > 	  else if (SC_IS_BSS (ext_in->asym.sc))
 > > 	    {
 > > 	      ms_type = mst_file_bss;
 > > 	      svalue += ANOFFSET (objfile->section_offsets, SECT_OFF_BSS (objfile));
 > > 	    }
 > > 
 > > 
 > > The SECT_OFF_BSS then is -1. It shouldn't be used in this case,
 > > because the symbol may not be in the bss section.
 > > 
 > > That said, may I suggest a different approach?  I think the bug is
 > > really in mdebugread.c, not symfile.c.  I think the SC_IS_BSS macro
 > > should be split in 2 separate ones: a proper SC_IS_BSS and another
 > > SC_IS_SBSS for your case.  Then the above code would work ok, if you
 > > don't have the .bss section, because it wouldn't trigger.  As for the
 > > SC_IS_SBSS case, you would have to figure out what the correct value
 > > to be added to svalue needs to be, and for this you need to find the
 > > index of the .sbss section.
 > 
 >   Ok, our patch was based on the assumption that the .bss and .sbss
 > sections were mutually exclusive. Your approach is more solid. 
 > 
 > May I suggest then to go a little further with your approach, which
 > cries out for the addition of a new sect_index_sbss in the objfile
 > structure:
 >   - in struct objfile (objfiles.h), add a new section index called
 >     sect_index_sbss
 >   - still in objfiles.h, add a new SECT_OFF_SBSS macro
 >   - initialize the sbss section index in default_symfile_offset
 >     (symfile.c) at the same time we're initializing the indexes of
 >     the other common sections
 >   - in mdebugread.c, fix the SC_IS_BSS macro, add a new SC_IS_SBSS
 >     macro, and then update the parse_partial_symbols () procedure
 >     to use the .sbss section if the symbol is there
 > 
 > If you agree with this approach, I'll then submit the new patch.

Joel,
Sorry, but we are trying to move away from having gdb knowing that
some sections are special. Adding a new SECT_OFF_SBSS macro, would not
be in line with that. Would you mind just doing that bit I suggested?
That would solve your immediate problem, right?

Thanks a bunch

Elena


 > 
 > Thanks,
 > -- 
 > Joel
 > 

Re: [patch] Fix uninitialized section index internal error

[Joel Brobecker]

> Sorry, but we are trying to move away from having gdb knowing that
> some sections are special. Adding a new SECT_OFF_SBSS macro, would not
> be in line with that. Would you mind just doing that bit I suggested?
> That would solve your immediate problem, right?

Elena,

  I implemented your suggestion, but I stumbled accross another problem
that puzzled me.

  the program that I provided as an example to reproduce the problem
does in fact contain some symbols located in the .bss section ('end' for
instance), but gdb could had not located it. I verified this by using nm:

List of symbols in the .bss section:
<<
% nm -B mach | grep ' B '
0x00000140000160 B .bss
0x00000140000160 B _ebss
0x00000140000160 B _end
0x00000140000160 B end
>>

List of symbols in the .sbss section:
<<
% nm -B mach | grep ' S '
0x00000140000140 S .sbss
0x00000140000140 S __Argc
0x00000140000158 S __Argv
0x00000140000150 S _auxv
0x00000140000140 S _fbss
0x00000140000148 S _ldr_present
>>

Unfortunately, I don't have enough time to persue this right now. In
think my changes improve a bit the current situation, but do not fix all
problems, so I'm not sure they should be submitted yet.

I'll look into why gdb can not find this .bss section when I have more time
available.

-- 
Joel

Re: [patch] Fix uninitialized section index internal error

[Elena Zannoni]

Joel Brobecker writes:
 > > Sorry, but we are trying to move away from having gdb knowing that
 > > some sections are special. Adding a new SECT_OFF_SBSS macro, would not
 > > be in line with that. Would you mind just doing that bit I suggested?
 > > That would solve your immediate problem, right?
 > 
 > Elena,
 > 
 >   I implemented your suggestion, but I stumbled accross another problem
 > that puzzled me.
 > 
 >   the program that I provided as an example to reproduce the problem
 > does in fact contain some symbols located in the .bss section ('end' for
 > instance), but gdb could had not located it. I verified this by using nm:
 > 
 > List of symbols in the .bss section:
 > <<
 > % nm -B mach | grep ' B '
 > 0x00000140000160 B .bss
 > 0x00000140000160 B _ebss
 > 0x00000140000160 B _end
 > 0x00000140000160 B end
 > >>
 > 
 > List of symbols in the .sbss section:
 > <<
 > % nm -B mach | grep ' S '
 > 0x00000140000140 S .sbss
 > 0x00000140000140 S __Argc
 > 0x00000140000158 S __Argv
 > 0x00000140000150 S _auxv
 > 0x00000140000140 S _fbss
 > 0x00000140000148 S _ldr_present
 > >>
 > 
 > Unfortunately, I don't have enough time to persue this right now. In
 > think my changes improve a bit the current situation, but do not fix all
 > problems, so I'm not sure they should be submitted yet.

Hmm, I think that if your changes are an improvement, and don't break
anything else, it is worth including them in the sources, so they
don't get lost. 

 > 
 > I'll look into why gdb can not find this .bss section when I have more time
 > available.
 > 

Thanks for doing this.

Elena


 > -- 
 > Joel
 > 

Re: [patch] Fix uninitialized section index internal error

[Joel Brobecker]

Hi Elena,

  I finally had some time to look at the problem we encounter on Tru64
5.1... It's been a long sime since we last discussed this, so here is
a pointer to the original message:

    http://sources.redhat.com/ml/gdb-patches/2001-05/msg00187.html

  I can actually reproduce the internal error with an empty program
without needing to link in libmach.so...

  So, I modified mdebugread.c to separate the handling of symbols
located in the BSS section and the symbols located in the SBSS section,
just as you suggested. I thought that would make the internal-error
disappear, but they did not :-(.

  After investigating a bit more, I found that the linker creates a
certain number of special symbols, such as _ebss, or _fdata for
instance. The Compacq documentation says that all these symbols are
labels. These labels are used as adresses to mark the begining or the
end of a particular part of the an object file. For instance, _ebss
marks the end of the .bss section.

  The "trick" that confuses gdb in our example is that the symbol
table in the executable contains an entry for the "_ebss" symbol, which
storage class is scBss (and I verified this with the native odump
utility). However, the .bss section does not exist! This is not the only
symbol that is causing trouble, others are in a similar case. I noticed
that the problem arrises with the .data section when we try to load the
symbols of /usr/shlib/libm.so, for instance.

  So it appears that having a symbol referencing a section does not
necessarily means that this section actually exists, not for Label
symbols. I'm not sure how to fix this in GDB. I can see several options.

The solution that seems the most reasonable to me is to check whether
the section exists before making the address of the Label symbol relative to
the section start address. Otherwise, if the symbol refers to a
non-existent section, then keep the address as-is and set the storage
class to "unknown". This seems a bit "hacky" to me, because we are
not being very consistant on how we consider the address of the label
(relative or not relative, that is the question).

Another approach, which has the merit of simplifying the code (maybe
a bit too much) is to say: all label addresses are absolute, and we
don't really care which section they are refering to. So the whole
stLabel case code-section collapses to the following line:
   
   ms_type = mst_unknown;
   /* value is already set to ext_in->asym.value */

I find this approach quite attractive, but I am afraid that it might be
a bit too simplistic to be correct...

Let me know what you think,
Thanks.
-- 
Joel