[Jim Blandy <jimb@redhat.com>] Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy <jimb@redhat.com>] Re: RFA: don't read coff line number unless we have symbols too

[Jim Blandy]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]


Joel, you said this patch caused you some problems, and asked me to
put it on hold until you had more data.  Can you verify that this does
indeed break something?  I'm pretty confident that you were observing
something else.

I'm skeptical that it causes problems because the patch (as I
understand it, anyway) simply prevents GDB from reading data it will
never use.  The logic goes like this:

If num_symbols > 0, then GDB behaves exactly as before.  This is easy
to see.

If num_symbols is zero, then:
a) GDB does not call bfd_map_over_sections with find_linenos as its
   working function.  find_linenos's only side effect is to set
   info->{min,max}_lineno_offset.  So those are left zero.
b) We don't register free_linetab_cleanup.
c) We don't call init_lineno.  That function's only side effect is to
   set linetab{,_offset,_size}.  Since this is the only place the
   table is ever allocated, b) has no effect.  And this call is the
   only use of info->{min,max}_lineno_offset, so we know that a) has
   no effect.
d) We pass zero as coff_symtab_read's second argument, nsyms.  This is
   just what GDB would do without the patch, so this can't break
   anything new.  But when nsyms is zero, we never enter
   coff_symtab_read's main while loop, so enter_linenos never gets
   called.  That function is the only use of linetab{,_offset,_size},
   so c) has no effect.






[-- Attachment #2: Type: message/rfc822, Size: 5505 bytes --]

From: Jim Blandy <jimb@redhat.com>
To: "Philippe De Muyter" <phdm@macqel.be>
Cc: Joel Brobecker <brobecker@gnat.com>, gdb-patches@sources.redhat.com
Subject: Re: RFA: don't read coff line number unless we have symbols too
Date: 26 Jun 2002 13:22:51 -0500
Message-ID: <npwusl99xg.fsf@zwingli.cygnus.com>


"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: [Jim Blandy <jimb@redhat.com>] Re: RFA: don't read coff line number unless we have symbols too

[Joel Brobecker]

Jim,

> Joel, you said this patch caused you some problems, and asked me to
> put it on hold until you had more data.  Can you verify that this does
> indeed break something?  I'm pretty confident that you were observing
> something else.

I am so sorry you put on hold this change for so long because of me.
Something must have been wrong during the mail transfer because I sent
you a private message on June 25:

<<
   Date: Tue, 25 Jun 2002 15:22:01 -0700
   From: Joel Brobecker <brobecker@gnat.com>
   Subject: Re: RFA: don't read coff line number unless we have symbols too
   To: Jim Blandy <jimb@redhat.com>
   
   > 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.
   
   Thank you Jim. Just to avoid any misunderstanding, Paul Hilfinger took
   over this issue, so you don't need to wait on me. Last I saw, the
   Interix case was fine too, so it looks like our initial fears were
   unjustified.
>>

-- 
Joel