This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [PATCH] Eliminate duplicate dwarf2 data


Hi Daniel,

  Thanks very much for implementing this feature.  I have had a look
  at your patches to the linker, and I have a few comments:

--------------------------------------------------------------------------
+    for (msec = abfd->sections; msec != NULL; msec = msec->next)
+    {
+ 	   if (strncmp(msec->name, ".debug_info", strlen(".debug_info")) 
+ 			   && strncmp(msec->name, ".gnu.linkonce.wi.", strlen(".gnu.linkonce.wi")))
+ 		   continue;
        
! 	   if (! msec)
! 	   {
  	  /* No dwarf2 info.  Note that at this point the stash
  	     has been allocated, but contains zeros, this lets
  	     future calls to this function fail quicker. */

  I believe that this is wrong for three reasons:

     1.  It uses strncmp() to check for a section .debug_info when it
         should be using strcmp().

     2.  The test for (! msec) is redundant, since it can never be
         NULL.

     3.  If there are no debugging sections present in this file the
         for() loop will finish, but the execution will drop through
         into the rest of the function, where it will try to use an
         uninitialised value for stash->info_ptr.

  Instead I would suggest that you recode this patch as a separate
  function, like this:  (Note - this assumes that there will never be
  both a .debug_info *and* a .gnu.linkonce.wi... section in the same
  bfd.)

Index: bfd/dwarf2.c
===================================================================
RCS file: /cvs/src//src/bfd/dwarf2.c,v
retrieving revision 1.14
diff -p -r1.14 dwarf2.c
*** dwarf2.c	2000/04/19 10:53:01	1.14
--- dwarf2.c	2000/08/27 19:00:32
*************** comp_unit_find_nearest_line (unit, addr,
*** 1463,1468 ****
--- 1463,1495 ----
    return line_p || func_p;
  }
  
+ /* Locate the section containing the debugging info.  There are two posisble
+    section names.  The first is .debug_info, which is the standard DWARF2 name.
+    The second is a section whoes name starts with .gnu.linkonce.wi.  This is a
+    variation on the  .debug_info section which has a checksum describing the
+    contents encoded into the name.  This allows the linker to identify and discard
+    duplicate debugging sections for different compilation units.  */
+ #define DWARF2_DEBUG_INFO ".debug_info"
+ #define GNU_LINKONCE_INFO ".gnu.linkonce.wi."
+ 
+ static asection *
+ find_debug_info (abfd)
+      bfd * abfd;
+ {
+   asection * msec;
+   
+   for (msec = abfd->sections; msec != NULL; msec = msec->next)
+     {
+       if (strcmp (msec->name, DWARF2_DEBUG_INFO) == 0)
+ 	return msec;
+ 
+       if (strncmp (msec->name, GNU_LINKONCE_INFO, strlen (GNU_LINKONCE_INFO)) == 0)
+ 	return msec;
+     }
+ 	  
+   return NULL;
+ }
+ 
  /* The DWARF2 version of find_nearest line.  Return true if the line
     is found without error.  ADDR_SIZE is the number of bytes in the
     initial .debug_info length field and in the abbreviation offset.
*************** _bfd_dwarf2_find_nearest_line (abfd, sec
*** 1522,1528 ****
        if (! stash)
  	return false;
        
!       msec = bfd_get_section_by_name (abfd, ".debug_info");
        if (! msec)
  	{
  	  /* No dwarf2 info.  Note that at this point the stash
--- 1549,1555 ----
        if (! stash)
  	return false;
        
!       msec = find_debug_info (abfd);
        if (! msec)
  	{
  	  /* No dwarf2 info.  Note that at this point the stash
--------------------------------------------------------------------------
*** elf-bfd.h	2000/07/20 03:16:18	1.26
--- elf-bfd.h	2000/08/07 01:46:08
*************** extern Elf_Internal_Shdr *bfd_elf_find_s
*** 978,984 ****
    PARAMS ((bfd *abfd, Elf_Internal_Shdr *hdr, const char *name));
  extern boolean _bfd_elf_make_section_from_phdr
!   PARAMS ((bfd *abfd, Elf_Internal_Phdr *hdr, int index, const char *typename));
  extern struct bfd_hash_entry *_bfd_elf_link_hash_newfunc
    PARAMS ((struct bfd_hash_entry *, struct bfd_hash_table *, const char *));
--- 978,984 ----
    PARAMS ((bfd *abfd, Elf_Internal_Shdr *hdr, const char *name));
  extern boolean _bfd_elf_make_section_from_phdr
!   PARAMS ((bfd *abfd, Elf_Internal_Phdr *hdr, int index, const char *typenam2e));
  extern struct bfd_hash_entry *_bfd_elf_link_hash_newfunc
    PARAMS ((struct bfd_hash_entry *, struct bfd_hash_table *, const char *));

  I assume that this is just a typo, and that you do not really want
  to change the name in the prototype to typenam2e... :-)
--------------------------------------------------------------------------
*** elf.sc	2000/07/17 22:41:08	1.13
--- elf.sc	2000/08/07 01:46:14
*************** SECTIONS
*** 327,333 ****
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
--- 327,333 ----
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }

  This is fine, but the same patch needs to be applied to the other
  ELF linker scripts:

Index: elf32avr.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/elf32avr.sc,v
retrieving revision 1.2
diff -p -r1.2 elf32avr.sc
*** elf32avr.sc	2000/05/27 15:36:58	1.2
--- elf32avr.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 137,143 ****
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
--- 137,143 ----
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
Index: elfd10v.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/elfd10v.sc,v
retrieving revision 1.2
diff -p -r1.2 elfd10v.sc
*** elfd10v.sc	2000/02/23 23:38:47	1.2
--- elfd10v.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 150,156 ****
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
--- 150,156 ----
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
Index: elfd30v.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/elfd30v.sc,v
retrieving revision 1.3
diff -p -r1.3 elfd30v.sc
*** elfd30v.sc	2000/06/19 02:05:53	1.3
--- elfd30v.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 203,209 ****
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
--- 203,209 ----
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
Index: elfi370.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/elfi370.sc,v
retrieving revision 1.1
diff -p -r1.1 elfi370.sc
*** elfi370.sc	2000/02/23 13:52:23	1.1
--- elfi370.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 198,204 ****
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
--- 198,204 ----
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
Index: elfm68hc11.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/elfm68hc11.sc,v
retrieving revision 1.2
diff -p -r1.2 elfm68hc11.sc
*** elfm68hc11.sc	2000/08/08 22:04:32	1.2
--- elfm68hc11.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 350,356 ****
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
--- 350,356 ----
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
Index: elfm68hc12.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/elfm68hc12.sc,v
retrieving revision 1.2
diff -p -r1.2 elfm68hc12.sc
*** elfm68hc12.sc	2000/08/08 22:04:32	1.2
--- elfm68hc12.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 350,356 ****
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
--- 350,356 ----
    .debug_pubnames 0 : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 : { *(.debug_abbrev) }
    .debug_line     0 : { *(.debug_line) }
    .debug_frame    0 : { *(.debug_frame) }
Index: i386beos.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/i386beos.sc,v
retrieving revision 1.1.1.1
diff -p -r1.1.1.1 i386beos.sc
*** i386beos.sc	1999/05/03 07:29:08	1.1.1.1
--- i386beos.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 177,183 ****
    .debug_pubnames 0 ${RELOCATING+(NOLOAD)} : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 ${RELOCATING+(NOLOAD)} : { *(.debug_info) }
    .debug_abbrev   0 ${RELOCATING+(NOLOAD)} : { *(.debug_abbrev) }
    .debug_line     0 ${RELOCATING+(NOLOAD)} : { *(.debug_line) }
    .debug_frame    0 ${RELOCATING+(NOLOAD)} : { *(.debug_frame) }
--- 177,183 ----
    .debug_pubnames 0 ${RELOCATING+(NOLOAD)} : { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0 ${RELOCATING+(NOLOAD)} : { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0 ${RELOCATING+(NOLOAD)} : { *(.debug_abbrev) }
    .debug_line     0 ${RELOCATING+(NOLOAD)} : { *(.debug_line) }
    .debug_frame    0 ${RELOCATING+(NOLOAD)} : { *(.debug_frame) }
Index: v850.sc
===================================================================
RCS file: /cvs/src//src/ld/scripttempl/v850.sc,v
retrieving revision 1.2
diff -p -r1.2 v850.sc
*** v850.sc	1999/06/02 20:47:23	1.2
--- v850.sc	2000/08/27 19:18:07
*************** SECTIONS
*** 184,190 ****
    .debug_pubnames 0	: { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0	: { *(.debug_info) }
    .debug_abbrev   0	: { *(.debug_abbrev) }
    .debug_line     0	: { *(.debug_line) }
    .debug_frame    0	: { *(.debug_frame) }
--- 184,190 ----
    .debug_pubnames 0	: { *(.debug_pubnames) }
  
    /* DWARF 2 */
!   .debug_info     0	: { *(.debug_info) *(.gnu.linkonce.wi.*) }
    .debug_abbrev   0	: { *(.debug_abbrev) }
    .debug_line     0	: { *(.debug_line) }
    .debug_frame    0	: { *(.debug_frame) }
---------------------------------------------------------------------------

  The only other thing missing from your patch is ChangeLog entries
  for the various files that you have changed.

  If you can make these changes I will be happy to apply (the binutils
  part of) your patch to the sourceware repository.

Cheers
	Nick




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]