RFA: don't read coff line number unless we have symbols too

RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

2002-03-06  Jim Blandy  <jimb@redhat.com>

	* coffread.c (coff_symfile_read): Don't try to read the line
	number table from disk if the image file doesn't have a symbol
	table; we'll never actually look at the info anyway, and Windows
	ships DLL's with bogus file offsets for the line number data.

Index: gdb/coffread.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/coffread.c,v
retrieving revision 1.182
diff -c -r1.182 coffread.c
*** gdb/coffread.c	2001/09/25 02:35:52	1.182
--- gdb/coffread.c	2002/03/06 23:12:00
***************
*** 622,637 ****
  
  /* End of warning */
  
!   /* Read the line number table, all at once.  */
!   info->min_lineno_offset = 0;
!   info->max_lineno_offset = 0;
!   bfd_map_over_sections (abfd, find_linenos, (PTR) info);
  
!   make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
!   val = init_lineno (abfd, info->min_lineno_offset,
! 		     info->max_lineno_offset - info->min_lineno_offset);
!   if (val < 0)
!     error ("\"%s\": error reading line numbers\n", name);
  
    /* Now read the string table, all at once.  */
  
--- 622,654 ----
  
  /* End of warning */
  
!   /* Only read line number information if we have symbols.
  
!      On Windows NT, some of the system's DLL's have sections with
!      PointerToLinenumbers fields that are non-zero, but point at
!      random places within the image file.  (In the case I found,
!      KERNEL32.DLL's .text section has a line number info pointer that
!      points into the middle of the string `lib\\i386\kernel32.dll'.)
! 
!      However, these DLL's also have no symbols.  The line number
!      tables are meaningless without symbols.  And in fact, GDB never
!      uses the line number information unless there are symbols.  So we
!      can avoid spurious error messages (and maybe run a little
!      faster!) by not even reading the line number table unless we have
!      symbols.  */
!   if (num_symbols > 0)
!     {
!       /* Read the line number table, all at once.  */
!       info->min_lineno_offset = 0;
!       info->max_lineno_offset = 0;
!       bfd_map_over_sections (abfd, find_linenos, (PTR) info);
! 
!       make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
!       val = init_lineno (abfd, info->min_lineno_offset,
!                          info->max_lineno_offset - info->min_lineno_offset);
!       if (val < 0)
!         error ("\"%s\": error reading line numbers\n", name);
!     }
  
    /* Now read the string table, all at once.  */
  

Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

Ping to the COFF maintainer: this patch needs review.

Jim Blandy <jimb@redhat.com> writes:

> 2002-03-06  Jim Blandy  <jimb@redhat.com>
> 
> 	* coffread.c (coff_symfile_read): Don't try to read the line
> 	number table from disk if the image file doesn't have a symbol
> 	table; we'll never actually look at the info anyway, and Windows
> 	ships DLL's with bogus file offsets for the line number data.
> 
> Index: gdb/coffread.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/coffread.c,v
> retrieving revision 1.182
> diff -c -r1.182 coffread.c
> *** gdb/coffread.c	2001/09/25 02:35:52	1.182
> --- gdb/coffread.c	2002/03/06 23:12:00
> ***************
> *** 622,637 ****
>   
>   /* End of warning */
>   
> !   /* Read the line number table, all at once.  */
> !   info->min_lineno_offset = 0;
> !   info->max_lineno_offset = 0;
> !   bfd_map_over_sections (abfd, find_linenos, (PTR) info);
>   
> !   make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> !   val = init_lineno (abfd, info->min_lineno_offset,
> ! 		     info->max_lineno_offset - info->min_lineno_offset);
> !   if (val < 0)
> !     error ("\"%s\": error reading line numbers\n", name);
>   
>     /* Now read the string table, all at once.  */
>   
> --- 622,654 ----
>   
>   /* End of warning */
>   
> !   /* Only read line number information if we have symbols.
>   
> !      On Windows NT, some of the system's DLL's have sections with
> !      PointerToLinenumbers fields that are non-zero, but point at
> !      random places within the image file.  (In the case I found,
> !      KERNEL32.DLL's .text section has a line number info pointer that
> !      points into the middle of the string `lib\\i386\kernel32.dll'.)
> ! 
> !      However, these DLL's also have no symbols.  The line number
> !      tables are meaningless without symbols.  And in fact, GDB never
> !      uses the line number information unless there are symbols.  So we
> !      can avoid spurious error messages (and maybe run a little
> !      faster!) by not even reading the line number table unless we have
> !      symbols.  */
> !   if (num_symbols > 0)
> !     {
> !       /* Read the line number table, all at once.  */
> !       info->min_lineno_offset = 0;
> !       info->max_lineno_offset = 0;
> !       bfd_map_over_sections (abfd, find_linenos, (PTR) info);
> ! 
> !       make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> !       val = init_lineno (abfd, info->min_lineno_offset,
> !                          info->max_lineno_offset - info->min_lineno_offset);
> !       if (val < 0)
> !         error ("\"%s\": error reading line numbers\n", name);
> !     }
>   
>     /* Now read the string table, all at once.  */
>   

Re: RFA: don't read coff line number unless we have symbols too

[Daniel Jacobowitz]

You may want to contact him (MAINTAINERS says Philippe De Muyter,
phdm@macqel.be) directly.  I've never once seen him on this list.

On Thu, Jun 20, 2002 at 01:26:48PM -0500, Jim Blandy wrote:
> 
> Ping to the COFF maintainer: this patch needs review.
> 
> Jim Blandy <jimb@redhat.com> writes:
> 
> > 2002-03-06  Jim Blandy  <jimb@redhat.com>
> > 
> > 	* coffread.c (coff_symfile_read): Don't try to read the line
> > 	number table from disk if the image file doesn't have a symbol
> > 	table; we'll never actually look at the info anyway, and Windows
> > 	ships DLL's with bogus file offsets for the line number data.
> > 
> > Index: gdb/coffread.c
> > ===================================================================
> > RCS file: /cvs/cvsfiles/devo/gdb/coffread.c,v
> > retrieving revision 1.182
> > diff -c -r1.182 coffread.c
> > *** gdb/coffread.c	2001/09/25 02:35:52	1.182
> > --- gdb/coffread.c	2002/03/06 23:12:00
> > ***************
> > *** 622,637 ****
> >   
> >   /* End of warning */
> >   
> > !   /* Read the line number table, all at once.  */
> > !   info->min_lineno_offset = 0;
> > !   info->max_lineno_offset = 0;
> > !   bfd_map_over_sections (abfd, find_linenos, (PTR) info);
> >   
> > !   make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> > !   val = init_lineno (abfd, info->min_lineno_offset,
> > ! 		     info->max_lineno_offset - info->min_lineno_offset);
> > !   if (val < 0)
> > !     error ("\"%s\": error reading line numbers\n", name);
> >   
> >     /* Now read the string table, all at once.  */
> >   
> > --- 622,654 ----
> >   
> >   /* End of warning */
> >   
> > !   /* Only read line number information if we have symbols.
> >   
> > !      On Windows NT, some of the system's DLL's have sections with
> > !      PointerToLinenumbers fields that are non-zero, but point at
> > !      random places within the image file.  (In the case I found,
> > !      KERNEL32.DLL's .text section has a line number info pointer that
> > !      points into the middle of the string `lib\\i386\kernel32.dll'.)
> > ! 
> > !      However, these DLL's also have no symbols.  The line number
> > !      tables are meaningless without symbols.  And in fact, GDB never
> > !      uses the line number information unless there are symbols.  So we
> > !      can avoid spurious error messages (and maybe run a little
> > !      faster!) by not even reading the line number table unless we have
> > !      symbols.  */
> > !   if (num_symbols > 0)
> > !     {
> > !       /* Read the line number table, all at once.  */
> > !       info->min_lineno_offset = 0;
> > !       info->max_lineno_offset = 0;
> > !       bfd_map_over_sections (abfd, find_linenos, (PTR) info);
> > ! 
> > !       make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> > !       val = init_lineno (abfd, info->min_lineno_offset,
> > !                          info->max_lineno_offset - info->min_lineno_offset);
> > !       if (val < 0)
> > !         error ("\"%s\": error reading line numbers\n", name);
> > !     }
> >   
> >     /* Now read the string table, all at once.  */
> >   
> 

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

Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> You may want to contact him (MAINTAINERS says Philippe De Muyter,
> phdm@macqel.be) directly.  I've never once seen him on this list.

Yeah, I know.  He was the addressee of my post.  :)

Re: RFA: don't read coff line number unless we have symbols too

[Philippe De Muyter]

Seems OK to me.

Philippe

Philippe De Muyter  phdm@macqel.be  Tel +32 27029044
Macq Electronique SA  rue de l'Aeronef 2  B-1140 Bruxelles  Fax +32 27029077

Jim Blandy wrote :
> 
> Ping to the COFF maintainer: this patch needs review.
> 
> Jim Blandy <jimb@redhat.com> writes:
> 
> > 2002-03-06  Jim Blandy  <jimb@redhat.com>
> > 
> > 	* coffread.c (coff_symfile_read): Don't try to read the line
> > 	number table from disk if the image file doesn't have a symbol
> > 	table; we'll never actually look at the info anyway, and Windows
> > 	ships DLL's with bogus file offsets for the line number data.
> > 
> > Index: gdb/coffread.c
> > ===================================================================
> > RCS file: /cvs/cvsfiles/devo/gdb/coffread.c,v
> > retrieving revision 1.182
> > diff -c -r1.182 coffread.c
> > *** gdb/coffread.c	2001/09/25 02:35:52	1.182
> > --- gdb/coffread.c	2002/03/06 23:12:00
> > ***************
> > *** 622,637 ****
> >   
> >   /* End of warning */
> >   
> > !   /* Read the line number table, all at once.  */
> > !   info->min_lineno_offset = 0;
> > !   info->max_lineno_offset = 0;
> > !   bfd_map_over_sections (abfd, find_linenos, (PTR) info);
> >   
> > !   make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> > !   val = init_lineno (abfd, info->min_lineno_offset,
> > ! 		     info->max_lineno_offset - info->min_lineno_offset);
> > !   if (val < 0)
> > !     error ("\"%s\": error reading line numbers\n", name);
> >   
> >     /* Now read the string table, all at once.  */
> >   
> > --- 622,654 ----
> >   
> >   /* End of warning */
> >   
> > !   /* Only read line number information if we have symbols.
> >   
> > !      On Windows NT, some of the system's DLL's have sections with
> > !      PointerToLinenumbers fields that are non-zero, but point at
> > !      random places within the image file.  (In the case I found,
> > !      KERNEL32.DLL's .text section has a line number info pointer that
> > !      points into the middle of the string `lib\\i386\kernel32.dll'.)
> > ! 
> > !      However, these DLL's also have no symbols.  The line number
> > !      tables are meaningless without symbols.  And in fact, GDB never
> > !      uses the line number information unless there are symbols.  So we
> > !      can avoid spurious error messages (and maybe run a little
> > !      faster!) by not even reading the line number table unless we have
> > !      symbols.  */
> > !   if (num_symbols > 0)
> > !     {
> > !       /* Read the line number table, all at once.  */
> > !       info->min_lineno_offset = 0;
> > !       info->max_lineno_offset = 0;
> > !       bfd_map_over_sections (abfd, find_linenos, (PTR) info);
> > ! 
> > !       make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> > !       val = init_lineno (abfd, info->min_lineno_offset,
> > !                          info->max_lineno_offset - info->min_lineno_offset);
> > !       if (val < 0)
> > !         error ("\"%s\": error reading line numbers\n", name);
> > !     }
> >   
> >     /* Now read the string table, all at once.  */
> >   
> 

Re: RFA: don't read coff line number unless we have symbols too

[Joel Brobecker]

Hello,

> Seems OK to me.

This change seems to be causing some problems. Unfortunately, I don't
have all the details yet. Can we put this patch on hold until I collect
all the information, please?

Thanks,
-- 
Joel

Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:
> Hello,
> > Seems OK to me.
> 
> This change seems to be causing some problems. Unfortunately, I don't
> have all the details yet. Can we put this patch on hold until I collect
> all the information, please?

Sure.

It's supposed to be semantically undetectable, except for not reading
the line number info.  The code simply makes GDB not read info from
the executable that it will never use.  If that expectation is
incorrect, I definitely want to know.
 

Re: RFA: don't read coff line number unless we have symbols too

[Philippe De Muyter]

Jim Blandy wrote :
> 
> Joel Brobecker <brobecker@gnat.com> writes:
> > Hello,
> > > Seems OK to me.
> > 
> > This change seems to be causing some problems. Unfortunately, I don't
> > have all the details yet. Can we put this patch on hold until I collect
> > all the information, please?
> 
> Sure.
> 
> It's supposed to be semantically undetectable, except for not reading
> the line number info.  The code simply makes GDB not read info from
> the executable that it will never use.  If that expectation is
> incorrect, I definitely want to know.
>  
> 
Should the following lines not be kept outside of the conditional block ?

       info->min_lineno_offset = 0;
       info->max_lineno_offset = 0;

Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

"Philippe De Muyter" <phdm@macqel.be> writes:
> Should the following lines not be kept outside of the conditional block ?
> 
>        info->min_lineno_offset = 0;
>        info->max_lineno_offset = 0;

Sure, those can be moved outside.  

I was just thinking of those as just establishing the pre-condition
for the loop that bfd_map_over_sections does.  Those fields are never
used outside the "read the line number table" block, and the worker
function for bfd_map_over_sections --- effectively, the loop body.  In
that light, it makes more sense to keep them right next to the call,
as they are in the original code.

Here's a revised patch, if you prefer the initializations outside.

2002-03-06  Jim Blandy  <jimb@redhat.com>

	* coffread.c (coff_symfile_read): Don't try to read the line
	number table from disk if the image file doesn't have a symbol
	table; we'll never actually look at the info anyway, and Windows
	ships DLL's with bogus file offsets for the line number data.

Index: gdb/coffread.c
===================================================================
RCS file: /cvs/src/src/gdb/coffread.c,v
retrieving revision 1.26
diff -c -r1.26 coffread.c
*** gdb/coffread.c	19 Mar 2002 19:00:03 -0000	1.26
--- gdb/coffread.c	26 Jun 2002 18:19:07 -0000
***************
*** 593,608 ****
  
  /* End of warning */
  
-   /* Read the line number table, all at once.  */
    info->min_lineno_offset = 0;
    info->max_lineno_offset = 0;
-   bfd_map_over_sections (abfd, find_linenos, (void *) info);
  
!   make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
!   val = init_lineno (abfd, info->min_lineno_offset,
! 		     info->max_lineno_offset - info->min_lineno_offset);
!   if (val < 0)
!     error ("\"%s\": error reading line numbers\n", name);
  
    /* Now read the string table, all at once.  */
  
--- 593,626 ----
  
  /* End of warning */
  
    info->min_lineno_offset = 0;
    info->max_lineno_offset = 0;
  
!   /* Only read line number information if we have symbols.
! 
!      On Windows NT, some of the system's DLL's have sections with
!      PointerToLinenumbers fields that are non-zero, but point at
!      random places within the image file.  (In the case I found,
!      KERNEL32.DLL's .text section has a line number info pointer that
!      points into the middle of the string `lib\\i386\kernel32.dll'.)
! 
!      However, these DLL's also have no symbols.  The line number
!      tables are meaningless without symbols.  And in fact, GDB never
!      uses the line number information unless there are symbols.  So we
!      can avoid spurious error messages (and maybe run a little
!      faster!) by not even reading the line number table unless we have
!      symbols.  */
!   if (num_symbols > 0)
!     {
!       /* Read the line number table, all at once.  */
!       bfd_map_over_sections (abfd, find_linenos, (void *) info);
! 
!       make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
!       val = init_lineno (abfd, info->min_lineno_offset,
!                          info->max_lineno_offset - info->min_lineno_offset);
!       if (val < 0)
!         error ("\"%s\": error reading line numbers\n", name);
!     }
  
    /* Now read the string table, all at once.  */
  

Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

Joel Brobecker has said in private E-mail that his earlier concerns
have been addressed, so I've committed this change.

Jim Blandy <jimb@redhat.com> writes:

> "Philippe De Muyter" <phdm@macqel.be> writes:
> > Should the following lines not be kept outside of the conditional block ?
> > 
> >        info->min_lineno_offset = 0;
> >        info->max_lineno_offset = 0;
> 
> Sure, those can be moved outside.  
> 
> I was just thinking of those as just establishing the pre-condition
> for the loop that bfd_map_over_sections does.  Those fields are never
> used outside the "read the line number table" block, and the worker
> function for bfd_map_over_sections --- effectively, the loop body.  In
> that light, it makes more sense to keep them right next to the call,
> as they are in the original code.
> 
> Here's a revised patch, if you prefer the initializations outside.
> 
> 2002-03-06  Jim Blandy  <jimb@redhat.com>
> 
> 	* coffread.c (coff_symfile_read): Don't try to read the line
> 	number table from disk if the image file doesn't have a symbol
> 	table; we'll never actually look at the info anyway, and Windows
> 	ships DLL's with bogus file offsets for the line number data.
> 
> Index: gdb/coffread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/coffread.c,v
> retrieving revision 1.26
> diff -c -r1.26 coffread.c
> *** gdb/coffread.c	19 Mar 2002 19:00:03 -0000	1.26
> --- gdb/coffread.c	26 Jun 2002 18:19:07 -0000
> ***************
> *** 593,608 ****
>   
>   /* End of warning */
>   
> -   /* Read the line number table, all at once.  */
>     info->min_lineno_offset = 0;
>     info->max_lineno_offset = 0;
> -   bfd_map_over_sections (abfd, find_linenos, (void *) info);
>   
> !   make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> !   val = init_lineno (abfd, info->min_lineno_offset,
> ! 		     info->max_lineno_offset - info->min_lineno_offset);
> !   if (val < 0)
> !     error ("\"%s\": error reading line numbers\n", name);
>   
>     /* Now read the string table, all at once.  */
>   
> --- 593,626 ----
>   
>   /* End of warning */
>   
>     info->min_lineno_offset = 0;
>     info->max_lineno_offset = 0;
>   
> !   /* Only read line number information if we have symbols.
> ! 
> !      On Windows NT, some of the system's DLL's have sections with
> !      PointerToLinenumbers fields that are non-zero, but point at
> !      random places within the image file.  (In the case I found,
> !      KERNEL32.DLL's .text section has a line number info pointer that
> !      points into the middle of the string `lib\\i386\kernel32.dll'.)
> ! 
> !      However, these DLL's also have no symbols.  The line number
> !      tables are meaningless without symbols.  And in fact, GDB never
> !      uses the line number information unless there are symbols.  So we
> !      can avoid spurious error messages (and maybe run a little
> !      faster!) by not even reading the line number table unless we have
> !      symbols.  */
> !   if (num_symbols > 0)
> !     {
> !       /* Read the line number table, all at once.  */
> !       bfd_map_over_sections (abfd, find_linenos, (void *) info);
> ! 
> !       make_cleanup (free_linetab_cleanup, 0 /*ignore*/);
> !       val = init_lineno (abfd, info->min_lineno_offset,
> !                          info->max_lineno_offset - info->min_lineno_offset);
> !       if (val < 0)
> !         error ("\"%s\": error reading line numbers\n", name);
> !     }
>   
>     /* Now read the string table, all at once.  */
>   

Re: RFA: don't read coff line number unless we have symbols too

[Paul N. Hilfinger]

Jim Blandy wrote :
> 
> Ping to the COFF maintainer: this patch needs review.
> 
> Jim Blandy <jimb@redhat.com> writes:
> 
> > 2002-03-06  Jim Blandy  <jimb@redhat.com>
> > 
> >     * coffread.c (coff_symfile_read): Don't try to read the line
> >     number table from disk if the image file doesn't have a symbol
> >     table; we'll never actually look at the info anyway, and Windows
> >     ships DLL's with bogus file offsets for the line number data.

But apparently, on Interix, you can get files with valid line numbers and 
no symbols when all the .o's come from VC and are linked by ld.  So the 
line-number info is perhaps not entirely useless.  Is this change a 
space-saving move, or a fix for a more serious problem?

Paul

Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

"Paul N. Hilfinger" <hilfingr@otisco.mckusick.com> writes:
> Jim Blandy wrote :
> > 
> > Ping to the COFF maintainer: this patch needs review.
> > 
> > Jim Blandy <jimb@redhat.com> writes:
> > 
> > > 2002-03-06  Jim Blandy  <jimb@redhat.com>
> > > 
> > >     * coffread.c (coff_symfile_read): Don't try to read the line
> > >     number table from disk if the image file doesn't have a symbol
> > >     table; we'll never actually look at the info anyway, and Windows
> > >     ships DLL's with bogus file offsets for the line number data.
> 
> But apparently, on Interix, you can get files with valid line numbers and 
> no symbols when all the .o's come from VC and are linked by ld.  So the 
> line-number info is perhaps not entirely useless.  Is this change a 
> space-saving move, or a fix for a more serious problem?

See the comment from the patch:

!   /* Only read line number information if we have symbols.
! 
!      On Windows NT, some of the system's DLL's have sections with
!      PointerToLinenumbers fields that are non-zero, but point at
!      random places within the image file.  (In the case I found,
!      KERNEL32.DLL's .text section has a line number info pointer that
!      points into the middle of the string `lib\\i386\kernel32.dll'.)
! 
!      However, these DLL's also have no symbols.  The line number
!      tables are meaningless without symbols.  And in fact, GDB never
!      uses the line number information unless there are symbols.  So we
!      can avoid spurious error messages (and maybe run a little
!      faster!) by not even reading the line number table unless we have
!      symbols.  */

Re: RFA: don't read coff line number unless we have symbols too

[Paul N. Hilfinger]

OK. Further investigation (by Donn Terry) indicates that this change is
not a problem.    In the cases on Interix that we were worried about, there
is still a linker symbol table with non-debugging symbols, and so the line
numbers aren't tossed after all.

Paul Hilfinger

Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

"Paul N. Hilfinger" <hilfingr@otisco.mckusick.com> writes:
> OK. Further investigation (by Donn Terry) indicates that this change is
> not a problem.    In the cases on Interix that we were worried about, there
> is still a linker symbol table with non-debugging symbols, and so the line
> numbers aren't tossed after all.

I'm remembering more about this now --- COFF line numbers aren't
*meaningful* in the absence of linker symbols.  They indicate source
files by symtab indices.  Or some such.  So I think it's always got to
be harmless to ignore line number info when you don't have symbols.

(I'm so out of it.  I could be completely wrong about what this patch
even does.  If what I'm saying is nonsense, just say so and I'll go
figure out what I was really thinking.)