Re: [patch] Fix separate-debug with non-unique section names (PR 11409)

Jan Kratochvil
2010-03-25T14:59:25Z

On Wed, 24 Mar 2010 21:37:34 +0100, Tom Tromey wrote:
> Can we stably sort the sections by name, using the initial index as a
> secondary key?

OK, attached.

build_section_addr_info_from_objfile could also be kept intact (just creating
new build_section_addr_info_from_bfd).

While there is some new code I have not found easier way. Also the complexity
is now reduced (n^2 -> n*log(n)); not that it would matter.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.

Thanks,
Jan

gdb/
2010-03-25 Jan Kratochvil <jan.kratochvil@redhat.com>

* symfile.c (build_section_addr_info_from_bfd): New.
(build_section_addr_info_from_objfile): Base it on
build_section_addr_info_from_bfd.
(addrs_section_compar, addrs_section_sort): New.
(addr_info_make_relative): New variables my_cleanup, abfd_addrs,
addrs_sorted, abfd_addrs_sorted and addrs_to_abfd_addrs. Build
addrs_to_abfd_addrs. Use it for recalculating ADDRS.

gdb/testsuite/
2010-03-25 Jan Kratochvil <jan.kratochvil@redhat.com>

* gdb.base/dup-sect.exp, gdb.base/dup-sect.S: New.

--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -319,29 +319,47 @@ build_section_addr_info_from_section_table (const struct target_section *start,
return sap;
}

-/* Create a section_addr_info from section offsets in OBJFILE. */
+/* Create a section_addr_info from section offsets in ABFD. */

-struct section_addr_info *
-build_section_addr_info_from_objfile (const struct objfile *objfile)
+static struct section_addr_info *
+build_section_addr_info_from_bfd (bfd *abfd)
{
struct section_addr_info *sap;
int i;
struct bfd_section *sec;

- sap = alloc_section_addr_info (objfile->num_sections);
- for (i = 0, sec = objfile->obfd->sections; sec != NULL; sec = sec->next)
- if (bfd_get_section_flags (objfile->obfd, sec) & (SEC_ALLOC | SEC_LOAD))
+ sap = alloc_section_addr_info (bfd_count_sections (abfd));
+ for (i = 0, sec = abfd->sections; sec != NULL; sec = sec->next)
+ if (bfd_get_section_flags (abfd, sec) & (SEC_ALLOC | SEC_LOAD))
{
- sap->other[i].addr = (bfd_get_section_vma (objfile->obfd, sec)
- + objfile->section_offsets->offsets[i]);
- sap->other[i].name = xstrdup (bfd_get_section_name (objfile->obfd,
- sec));
+ sap->other[i].addr = bfd_get_section_vma (abfd, sec);
+ sap->other[i].name = xstrdup (bfd_get_section_name (abfd, sec));
sap->other[i].sectindex = sec->index;
i++;
}
return sap;
}

+/* Create a section_addr_info from section offsets in OBJFILE. */
+
+struct section_addr_info *
+build_section_addr_info_from_objfile (const struct objfile *objfile)
+{
+ struct section_addr_info *sap;
+ int i;
+
+ /* Before reread_symbols gets rewritten it is not safe to call:
+ gdb_assert (objfile->num_sections == bfd_count_sections (objfile->obfd));
+ */
+ sap = build_section_addr_info_from_bfd (objfile->obfd);
+ for (i = 0; i < sap->num_sections && sap->other[i].name; i++)
+ {
+ int sectindex = sap->other[i].sectindex;
+
+ sap->other[i].addr += objfile->section_offsets->offsets[sectindex];
+ }
+ return sap;
+}

/* Free all memory allocated by build_section_addr_info_from_section_table. */

@@ -519,6 +537,46 @@ relative_addr_info_to_section_offsets (struct section_offsets *section_offsets,
}
}

+/* qsort comparator for addrs_section_sort. Sort entries in ascending order by
+ their (name, sectindex) pair. sectindex makes the sort by name stable. */
+
+static int
+addrs_section_compar (const void *ap, const void *bp)
+{
+ const struct other_sections *a = *((struct other_sections **) ap);
+ const struct other_sections *b = *((struct other_sections **) bp);
+ int retval, a_idx, b_idx;
+
+ retval = strcmp (a->name, b->name);
+ if (retval)
+ return retval;
+
+ /* SECTINDEX is underfined iff ADDR is zero. */
+ a_idx = a->addr == 0 ? 0 : a->sectindex;
+ b_idx = b->addr == 0 ? 0 : b->sectindex;
+ return a_idx - b_idx;
+}
+
+/* Provide sorted array of pointers to sections of ADDRS. The array is
+ terminated by NULL. Caller is responsible to call xfree for it. */
+
+static struct other_sections **
+addrs_section_sort (struct section_addr_info *addrs)
+{
+ struct other_sections **array;
+ int i;
+
+ /* `+ 1' for the NULL terminator. */
+ array = xmalloc (sizeof (*array) * (addrs->num_sections + 1));
+ for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++)
+ array[i] = &addrs->other[i];
+ array[i] = NULL;
+
+ qsort (array, i, sizeof (*array), addrs_section_compar);
+
+ return array;
+}
+
/* Relativize absolute addresses in ADDRS into offsets based on ABFD. Fill-in
also SECTINDEXes specific to ABFD there. This function can be used to
rebase ADDRS to start referencing different BFD than before. */
@@ -529,6 +587,10 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
asection *lower_sect;
CORE_ADDR lower_offset;
int i;
+ struct cleanup *my_cleanup;
+ struct section_addr_info *abfd_addrs;
+ struct other_sections **addrs_sorted, **abfd_addrs_sorted;
+ struct other_sections **addrs_to_abfd_addrs;

/* Find lowest loadable section to be used as starting point for
continguous sections. */
@@ -543,6 +605,55 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
else
lower_offset = bfd_section_vma (bfd_get_filename (abfd), lower_sect);

+ /* Create ADDRS_TO_ABFD_ADDRS array to map the sections in ADDRS to sections
+ in ABFD. Section names are not unique - there can be multiple sections of
+ the same name. Also the sections of the same name do not have to be
+ adjacent to each other. Some sections may be present only in one of the
+ files. Even sections present in both files do not have to be in the same
+ order.
+
+ Use stable sort by name for the sections in both files. Then linearly
+ scan both lists matching as most of the entries as possible. */
+
+ addrs_sorted = addrs_section_sort (addrs);
+ my_cleanup = make_cleanup (xfree, addrs_sorted);
+
+ abfd_addrs = build_section_addr_info_from_bfd (abfd);
+ make_cleanup_free_section_addr_info (abfd_addrs);
+ abfd_addrs_sorted = addrs_section_sort (abfd_addrs);
+ make_cleanup (xfree, abfd_addrs_sorted);
+
+ /* Now create ADDRS_TO_ABFD_ADDRS from ADDRS_SORTED and ABFD_ADDRS_SORTED. */
+
+ addrs_to_abfd_addrs = xzalloc (sizeof (*addrs_to_abfd_addrs)
+ * addrs->num_sections);
+ make_cleanup (xfree, addrs_to_abfd_addrs);
+
+ while (*addrs_sorted)
+ {
+ const char *sect_name = (*addrs_sorted)->name;
+
+ while (*abfd_addrs_sorted
+ && strcmp ((*abfd_addrs_sorted)->name, sect_name) < 0)
+ abfd_addrs_sorted++;
+
+ if (*abfd_addrs_sorted
+ && strcmp ((*abfd_addrs_sorted)->name, sect_name) == 0)
+ {
+ int index_in_addrs;
+
+ /* Make the found item directly addressable from ADDRS. */
+ index_in_addrs = *addrs_sorted - addrs->other;
+ gdb_assert (addrs_to_abfd_addrs[index_in_addrs] == NULL);
+ addrs_to_abfd_addrs[index_in_addrs] = *abfd_addrs_sorted;
+
+ /* Never use the same ABFD entry twice. */
+ abfd_addrs_sorted++;
+ }
+
+ addrs_sorted++;
+ }
+
/* Calculate offsets for the loadable sections.
FIXME! Sections must be in order of increasing loadable section
so that contiguous sections can use the lower-offset!!!
@@ -556,16 +667,16 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++)
{
const char *sect_name = addrs->other[i].name;
- asection *sect = bfd_get_section_by_name (abfd, sect_name);
+ struct other_sections *sect = addrs_to_abfd_addrs[i];

if (sect)
{
/* This is the index used by BFD. */
- addrs->other[i].sectindex = sect->index;
+ addrs->other[i].sectindex = sect->sectindex;

if (addrs->other[i].addr != 0)
{
- addrs->other[i].addr -= bfd_section_vma (abfd, sect);
+ addrs->other[i].addr -= sect->addr;
lower_offset = addrs->other[i].addr;
}
else
@@ -597,6 +708,8 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd)
/* SECTINDEX is invalid if ADDR is zero. */
}
}
+
+ do_cleanups (my_cleanup);
}

/* Parse the user's idea of an offset for dynamic linking, into our idea
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dup-sect.S
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2010 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+ .section sect1, "a"
+var1: .byte 1
+
+ .section sect2, "a"
+var2: .byte 2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dup-sect.exp
@@ -0,0 +1,79 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test inappropriate offseting of multiple sections with the same name.
+# When kept in object file (before final executable link) it still works.
+# When separate debug info file is not used it still works.
+# When the ELF symbol table is kept in the main binary it still works.
+# Used .S file as in .c file we would need __attriute__((section)) which is
+# a GCC extension.
+
+# This test can only be run on targets which support ELF and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+ && ![istarget *-*-gnu*]
+ && ![istarget *-*-elf*]
+ && ![istarget arm-*-eabi*]
+ && ![istarget powerpc-*-eabi*]} {
+ return 0
+}
+
+set testfile dup-sect
+set srcfile ${testfile}.S
+set srcmainfile start.c
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+
+if {[build_executable ${testfile}.exp $executable [list ${srcfile} ${srcmainfile}] {}] == -1} {
+ return -1
+}
+
+set test "rename section"
+set objcopy_program [transform objcopy]
+set result [catch "exec $objcopy_program --rename-section sect2=sect1 $binfile" output]
+verbose "result is $result"
+verbose "output is $output"
+if {$result != 0} {
+ fail $test
+ return
+}
+pass $test
+
+set test "split"
+if {[gdb_gnu_strip_debug $binfile] != 0} {
+ fail $test
+} else {
+ pass $test
+}
+
+# gdb_gnu_strip_debug uses only --strip-debug and keeps the ELF symbol table
+# in $binfile.
+set test "strip"
+set strip_program [transform strip]
+set result [catch "exec $strip_program $binfile" output]
+verbose "result is $result"
+verbose "output is $output"
+if {$result != 0} {
+ fail $test
+ return
+}
+pass $test
+
+clean_restart $executable
+
+gdb_test "p/d *(const char *) &var1" " = 1" "var1 after strip"
+gdb_test "p/d *(const char *) &var2" " = 2" "var2 after strip"