This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: [PATCH] Eliminate duplicate dwarf2 data
- To: Nick Clifton <nickc at redhat dot com>
- Subject: Re: [PATCH] Eliminate duplicate dwarf2 data
- From: Daniel Berlin <dberlin at redhat dot com>
- Date: Mon, 28 Aug 2000 19:07:50 -0700 (PDT)
- cc: gcc-patches at gcc dot gnu dot org, binutils at sources dot redhat dot com
On Sun, 27 Aug 2000, Nick Clifton wrote:
> 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.)
With the new gcc, you have at least one .gnu.linkonce.wi, and at least
one debug_info, unless your C++ app is fake.
So this recode won't handle it.
However, changing the
"msec = find_debug_info (" to "while (msec = find_debug_info )", and
putting the rest in the while block, should work fine.
>
> 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.
I'm so used to submitting them seperately that i forgot.
Sorry about that.
I'll work them up immediately.
>
> If you can make these changes I will be happy to apply (the binutils
> part of) your patch to the sourceware repository.
Thanks!
>
> Cheers
> Nick
>
>
>
>