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

Jan Kratochvil
2010-04-22T23:22:24Z

On Wed, 21 Apr 2010 22:03:11 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> Now the question remains what to use for gdb_7_1-branch as it fixes
> Jan> a regression (by me) from gdb-7.0 as described in the first mail:
> Jan> http://sourceware.org/ml/gdb-patches/2010-03/msg00799.html
>
> Jan> I would even suggest the first simple variant of the patch from that mail.
>
> Was there ever an answer to this?

No.

> I think the simple variant is probably safe enough. In any case it is
> an improvement.

Checked-in the branch gdb_7_1-branch.

Had to make a slight port for "sect_name".

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

Thanks,
Jan

http://sourceware.org/ml/gdb-cvs/2010-04/msg00229.html

--- src/gdb/ChangeLog 2010/04/08 17:15:08 1.11378.2.36
+++ src/gdb/ChangeLog 2010/04/22 23:20:13 1.11378.2.37
@@ -1,3 +1,9 @@
+2010-04-22 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ * symfile.c (addr_info_make_relative): Move sect declaration to the
+ outer block. Initialize it to NULL. Prefer SECT->next more than
+ bfd_get_section_by_name.
+
2010-04-08 Sami Wagiaalla <swagiaal@redhat.com>

PR Breakpoints/11408:
--- src/gdb/symfile.c 2010/03/17 00:05:23 1.272.2.3
+++ src/gdb/symfile.c 2010/04/22 23:20:14 1.272.2.4
@@ -566,6 +566,7 @@
asection *lower_sect;
CORE_ADDR lower_offset;
int i;
+ asection *sect;

/* Find lowest loadable section to be used as starting point for
continguous sections. */
@@ -590,10 +591,23 @@
(the loadable section directly below it in memory).
this_offset = lower_offset = lower_addr - lower_orig_addr */

+ sect = NULL;
for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++)
{
- asection *sect = bfd_get_section_by_name (abfd, addrs->other[i].name);
+ const char *sect_name = addrs->other[i].name;
+
+ /* Prefer the next section of that we have found last. The separate
+ debug info files have either the same section layout or just a few
+ sections are missing there. On the other hand the section name is not
+ unique and we could find an inappropraite section by its name. */
+
+ if (sect)
+ sect = sect->next;
+ if (sect && strcmp (sect_name, bfd_get_section_name (abfd, sect)) != 0)
+ sect = NULL;

+ if (sect == NULL)
+ sect = bfd_get_section_by_name (abfd, sect_name);
if (sect)
{
/* This is the index used by BFD. */
--- src/gdb/testsuite/ChangeLog 2010/04/08 17:15:11 1.2147.2.5
+++ src/gdb/testsuite/ChangeLog 2010/04/22 23:20:14 1.2147.2.6
@@ -1,3 +1,7 @@
+2010-04-22 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ * gdb.base/dup-sect.exp, gdb.base/dup-sect.S: New.
+
2010-04-08 Sami Wagiaalla <swagiaal@redhat.com>

* gdb.cp/gdb2384-base.h: Created 'namespace B'.
--- src/gdb/testsuite/gdb.base/dup-sect.S
+++ src/gdb/testsuite/gdb.base/dup-sect.S 2010-04-22 23:20:26.349536000 +0000
@@ -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
--- src/gdb/testsuite/gdb.base/dup-sect.exp
+++ src/gdb/testsuite/gdb.base/dup-sect.exp 2010-04-22 23:20:26.676628000 +0000
@@ -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"