[RFA, doc RFA] Fix symtab/15885

Doug Evans
2013-08-24T01:12:00Z

Hi.
This patch fixes symtab/15885: gdb isn't using .gdb_index when it
reloads symbols for a file.

There are a lot of problems in the symbol reloading code.
I'd rather not have this fix get bogged down in fixing everything
else so I've gone with something hopefully straightforward.

With the patch I added a regexp argument to "mt print objfiles",
most other such routines already take one.
I wanted to keep the testcase simple, at least conceptually, and only
print objfile data for the binary under test.

Regression tested with/without fission (which was important because
the testcase needs to handle the case of the toolchain generating
.gdb_index itself).

Ok to check in?

btw, one of the funnier aspects of this bug is that when gdb
loads symbols the first time objfile->sf is set to point to the
.gdb_index routines. That in turns causes objfile_has_partial_symbols
to return true (because dw2_has_symbols returns 1), and that in turn
causes gdb to decide it shouldn't use .gdb_index the next time. :-)

if (!objfile_has_partial_symbols (objfile) <<<<
&& dwarf2_initialize_objfile (objfile))
objfile->sf = &elf_sym_fns_gdb_index;

I think there is more cleanup to be had beyond reread_symbols,
but again I don't want this patch bogged down in that.

2013-08-23 Doug Evans <dje@google.com>

PR symtab/15885
* NEWS: Document "mt print objfiles" now takes optional regexp.
* dwarf2read.c (dw2_dump): Print some minimal information indicating
.gdb_index is in use.
* symfile.c (reread_symbols): Reset objfile->sf.
* symmisc.c (maintenance_print_objfiles): Argument is now an optional
regexp of objfiles to print.
(_initialize_symmisc): Update doc string for "mt print objfiles".

doc/
* gdb.texinfo (Maintenance Commands): "maint print objfiles" now takes
an optional regexp.

testsuite/
* lib/gdb.exp (run_on_host): Moved here from gnu-debugdata.exp.
* gdb.base/gnu-debugdata.exp (run): Moved to gdb.exp and renamed to
run_on_host. All callers updated.
* gdb.dwarf2/Makefile.in (EXECUTABLES): Add gdb-index.
(clean): rm -f *.gdb-index *.with-index.
* gdb.dwarf2/gdb-index.exp: New testcase.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.609
diff -u -p -r1.609 NEWS
--- NEWS 22 Aug 2013 20:32:54 -0000 1.609
+++ NEWS 24 Aug 2013 00:55:05 -0000
@@ -3,6 +3,8 @@

*** Changes since GDB 7.6

+* The "maintenance print objfiles" command now takes an optional regexp.
+
* The "catch syscall" command now works on arm*-linux* targets.

* Python scripting
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.827
diff -u -p -r1.827 dwarf2read.c
--- dwarf2read.c 20 Aug 2013 18:57:00 -0000 1.827
+++ dwarf2read.c 24 Aug 2013 00:55:05 -0000
@@ -3357,10 +3357,25 @@ dw2_print_stats (struct objfile *objfile
printf_filtered (_(" Number of unread CUs: %d\n"), count);
}

+/* This dumps minimal information about the index.
+ It is called via "mt print objfiles".
+ One use is to verify .gdb_index has been loaded by the
+ gdb.dwarf2/gdb-index.exp testcase. */
+
static void
dw2_dump (struct objfile *objfile)
{
- /* Nothing worth printing. */
+ dw2_setup (objfile);
+ gdb_assert (dwarf2_per_objfile->using_index);
+ printf_filtered (".gdb_index:");
+ if (dwarf2_per_objfile->index_table != NULL)
+ {
+ printf_filtered (" version %d\n",
+ dwarf2_per_objfile->index_table->version);
+ }
+ else
+ printf_filtered (" faked for \"readnow\"\n");
+ printf_filtered ("\n");
}

static void
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.384
diff -u -p -r1.384 symfile.c
--- symfile.c 20 Aug 2013 15:04:51 -0000 1.384
+++ symfile.c 24 Aug 2013 00:55:05 -0000
@@ -2504,6 +2504,12 @@ reread_symbols (void)
empty. We could use obstack_specify_allocation but
gdb_obstack.h specifies the alloc/dealloc functions. */
obstack_init (&objfile->objfile_obstack);
+
+ /* Reset the sym_fns pointer. The ELF reader can change it
+ based on whether .gdb_index is present, and we need it to
+ start over. PR symtab/15885 */
+ objfile->sf = find_sym_fns (objfile->obfd);
+
build_objfile_section_table (objfile);
terminate_minimal_symbol_table (objfile);

Index: symmisc.c
===================================================================
RCS file: /cvs/src/src/gdb/symmisc.c,v
retrieving revision 1.99
diff -u -p -r1.99 symmisc.c
--- symmisc.c 24 Aug 2013 00:29:13 -0000 1.99
+++ symmisc.c 24 Aug 2013 00:55:05 -0000
@@ -687,18 +687,23 @@ maintenance_print_msymbols (char *args,
}

static void
-maintenance_print_objfiles (char *ignore, int from_tty)
+maintenance_print_objfiles (char *regexp, int from_tty)
{
struct program_space *pspace;
struct objfile *objfile;

dont_repeat ();

+ if (regexp)
+ re_comp (regexp);
+
ALL_PSPACES (pspace)
ALL_PSPACE_OBJFILES (pspace, objfile)
{
QUIT;
- dump_objfile (objfile);
+ if (! regexp
+ || re_exec (objfile->name))
+ dump_objfile (objfile);
}
}

@@ -935,7 +940,8 @@ If a SOURCE file is specified, dump only
&maintenanceprintlist);

add_cmd ("objfiles", class_maintenance, maintenance_print_objfiles,
- _("Print dump of current object file definitions."),
+ _("Print dump of current object file definitions.\n\
+With an argument REGEXP, list the object files with matching names."),
&maintenanceprintlist);

add_cmd ("symtabs", class_maintenance, maintenance_info_symtabs, _("\
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1103
diff -u -p -r1.1103 gdb.texinfo
--- doc/gdb.texinfo 5 Aug 2013 15:39:25 -0000 1.1103
+++ doc/gdb.texinfo 24 Aug 2013 00:55:06 -0000
@@ -36820,10 +36820,11 @@ This command forces @value{GDBN} to flus

@kindex maint print objfiles
@cindex info for known object files
-@item maint print objfiles
-Print a dump of all known object files. For each object file, this
-command prints its name, address in memory, and all of its psymtabs
-and symtabs.
+@item maint print objfiles [@var{regexp}]
+Print a dump of all known object files.
+If @var{regexp} is specified, only print object files with names
+matching @var{regexp}. For each object file, this command prints its name,
+address in memory, and all of its psymtabs and symtabs.

@kindex maint print section-scripts
@cindex info for known .debug_gdb_scripts-loaded scripts
Index: testsuite/gdb.base/gnu-debugdata.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/gnu-debugdata.exp,v
retrieving revision 1.5
diff -u -p -r1.5 gnu-debugdata.exp
--- testsuite/gdb.base/gnu-debugdata.exp 1 Feb 2013 19:39:04 -0000 1.5
+++ testsuite/gdb.base/gnu-debugdata.exp 24 Aug 2013 00:55:06 -0000
@@ -19,40 +19,16 @@ if [build_executable ${testfile}.exp $te
return -1
}

-# A wrapper for 'remote_exec host' that passes or fails a test.
-# Returns 0 if all went well, nonzero on failure.
-# TEST is the name of the test, other arguments are as for
-# remote_exec.
-proc run {test program args} {
- verbose "cmdline is remote_exec host $program $args"
- # remote_exec doesn't work properly if the output is set but the
- # input is the empty string -- so replace an empty input with
- # /dev/null.
- if {[llength $args] > 1 && [lindex $args 1] == ""} {
- set args [lreplace $args 1 1 "/dev/null"]
- }
- set result [eval remote_exec host [list $program] $args]
- verbose "result is $result"
- set status [lindex $result 0]
- set output [lindex $result 1]
- if {$status == 0} {
- pass $test
- return 0
- } else {
- fail $test
- return -1
- }
-}
-
set pipeline_counter 0

-# Run a pipeline of processes through 'run'.
-# TEST is the base name of the test, it is modified and passed to 'run'.
+# Run a pipeline of processes through 'run_on_host'.
+# TEST is the base name of the test, it is modified and passed to 'run_on_host'.
# Each subsequent argument is a list of the form {PROGRAM [ARG]...}.
-# It is passed to 'run'. However, before being passed, if input and output
-# files are not specified in the list, then this proc provides them.
+# It is passed to 'run_on_host'. However, before being passed, if input and
+# output files are not specified in the list, then this proc provides them.
# Each program in the pipeline takes its input from the previous
# program's output.
+
proc pipeline {test args} {
global pipeline_counter

@@ -74,7 +50,7 @@ proc pipeline {test args} {
}
verbose "cooked args are [list $program $arguments $input $output]"

- if {[run "$test - invoke $program" $program $arguments \
+ if {[run_on_host "$test - invoke $program" $program $arguments \
$input $output]} {
return -1
}
@@ -108,7 +84,7 @@ if {[pipeline "nm" \
# Keep all the function symbols not already in the dynamic symbol
# table.
remote_file host delete ${binfile}.keep_symbols
-if {[run "comm" "comm" "-13 ${binfile}.dynsyms ${binfile}.funcsyms" "" \
+if {[run_on_host "comm" "comm" "-13 ${binfile}.dynsyms ${binfile}.funcsyms" "" \
"${binfile}.keep_symbols"]} {
return -1
}
@@ -116,20 +92,20 @@ if {[run "comm" "comm" "-13 ${binfile}.d
# Copy the full debuginfo, keeping only a minimal set of symbols and
# removing some unnecessary sections.
remote_file host delete ${binfile}.mini_debuginfo
-if {[run "objcopy 1" [transform objcopy] "-S --remove-section .gdb_index --remove-section .comment --keep-symbols=${binfile}.keep_symbols ${binfile} ${binfile}.mini_debuginfo"]} {
+if {[run_on_host "objcopy 1" [transform objcopy] "-S --remove-section .gdb_index --remove-section .comment --keep-symbols=${binfile}.keep_symbols ${binfile} ${binfile}.mini_debuginfo"]} {
return -1
}

# GDB specific - we do not have split executable in advance.
remote_file host delete ${binfile}.strip
-if {[run "strip" [transform strip] \
+if {[run_on_host "strip" [transform strip] \
"--strip-all -o ${binfile}.strip ${binfile}"]} {
return -1
}

# Separate full debug info into ${binfile}.debug.
remote_file host delete ${binfile}.debug
-if {[run "copydebug" [transform objcopy] \
+if {[run_on_host "copydebug" [transform objcopy] \
"--only-keep-debug ${binfile} ${binfile}.debug"]} {
return -1
}
@@ -138,7 +114,7 @@ if {[run "copydebug" [transform objcopy]
# .gnu_debuglink is normally not present in the .gnu_debugdata section but in
# some files there may be PT_NOTE with NT_GNU_BUILD_ID and GDB could look up
# the .debug file from it.
-if {[run "addlink" [transform objcopy] \
+if {[run_on_host "addlink" [transform objcopy] \
"--add-gnu-debuglink=${binfile}.debug ${binfile}.mini_debuginfo ${binfile}.mini_debuginfo-debuglink"]} {
return -1
}
@@ -146,11 +122,11 @@ if {[run "addlink" [transform objcopy] \
# Inject the compressed data into the .gnu_debugdata section of the
# original binary.
remote_file host delete ${binfile}.mini_debuginfo-debuglink.xz
-if {[run "xz" "xz" "-k ${binfile}.mini_debuginfo-debuglink"]} {
+if {[run_on_host "xz" "xz" "-k ${binfile}.mini_debuginfo-debuglink"]} {
return -1
}
remote_file host delete ${binfile}.test
-if {[run "objcopy 2" [transform objcopy] "--add-section .gnu_debugdata=${binfile}.mini_debuginfo-debuglink.xz ${binfile}.strip ${binfile}.test"]} {
+if {[run_on_host "objcopy 2" [transform objcopy] "--add-section .gnu_debugdata=${binfile}.mini_debuginfo-debuglink.xz ${binfile}.strip ${binfile}.test"]} {
return -1
}

Index: testsuite/gdb.dwarf2/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.dwarf2/Makefile.in,v
retrieving revision 1.12
diff -u -p -r1.12 Makefile.in
--- testsuite/gdb.dwarf2/Makefile.in 1 Jan 2013 06:33:28 -0000 1.12
+++ testsuite/gdb.dwarf2/Makefile.in 24 Aug 2013 00:55:06 -0000
@@ -21,7 +21,7 @@ VPATH = @srcdir@
srcdir = @srcdir@

EXECUTABLES = *.x pr10770 dw2-cp-infcall-ref-static dw2-inline-param \
- dw2-noloc dw2-ranges dw2-ref-missing-frame dw2-unresolved
+ dw2-noloc dw2-ranges dw2-ref-missing-frame dw2-unresolved gdb-index

all info install-info dvi install uninstall installcheck check:
@echo "Nothing to be done for $@..."
@@ -31,6 +31,7 @@ clean mostlyclean:
-rm -f core core.coremaker coremaker.core corefile
-rm -f *.dwo *.dwp
-rm -f $(EXECUTABLES)
+ -rm -f *.gdb-index *.with-index

distclean maintainer-clean realclean: clean
-rm -f *~ core
Index: testsuite/gdb.dwarf2/gdb-index.exp
===================================================================
RCS file: testsuite/gdb.dwarf2/gdb-index.exp
diff -N testsuite/gdb.dwarf2/gdb-index.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/gdb-index.exp 24 Aug 2013 00:55:06 -0000
@@ -0,0 +1,101 @@
+# Copyright 2013 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/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2.
+if {![dwarf2_support]} {
+ return 0
+}
+
+standard_testfile main.c
+
+if { [prepare_for_testing "${testfile}.exp" "${testfile}" \
+ [list ${srcfile}]] } {
+ return -1
+}
+
+# Add a .gdb_index section to PROGRAM.
+# PROGRAM is assumed to be the output of standard_output_file.
+# Returns the path of the program or "" if there is a failure.
+# If there is a failure it will have already been logged.
+
+proc add_gdb_index { program } {
+ set index_file ${program}.gdb-index
+ verbose -log "index_file: ${index_file}"
+ remote_file host delete ${index_file}
+ gdb_test_no_output "save gdb-index [file dirname ${index_file}]"
+
+ # No point in continuing if generating the index failed.
+ # N.B.: There are times when gdb won't create an index, and it's not a
+ # failure. However, in our case we need an index. So if you find one
+ # not being generated, you'll either have to tweak the .c file or maybe
+ # add a flag to force an index to be generated.
+ if { [remote_file host exists ${index_file}] } {
+ pass "gdb-index file created"
+ } else {
+ fail "gdb-index file created"
+ return ""
+ }
+
+ set program_with_index ${program}.with-index
+ if {[run_on_host "objcopy" [transform objcopy] "--remove-section .gdb_index --add-section .gdb_index=$index_file --set-section-flags .gdb_index=readonly ${program} ${program_with_index}"]} {
+ return ""
+ }
+ return ${program_with_index}
+}
+
+# Build a copy of the program with .gdb_index.
+# But only if the toolchain didn't already create one: gdb doesn't support
+# building an index from a program already using one.
+
+set test "check if index present"
+gdb_test_multiple "mt print objfiles ${testfile}" $test {
+ -re "gdb_index.*${gdb_prompt} $" {
+ set binfile_with_index $binfile
+ }
+ -re "Psymtabs.*${gdb_prompt} $" {
+ set binfile_with_index [add_gdb_index $binfile]
+ if { ${binfile_with_index} == "" } {
+ return -1
+ }
+ }
+}
+
+# Ok, we have a copy of $binfile with .gdb_index.
+# Restart gdb and verify the index was used.
+
+clean_restart ${binfile_with_index}
+gdb_test "mt print objfiles ${testfile}" \
+ "gdb_index.*" \
+ ".gdb_index used"
+
+# Make gdb re-read symbols and see if .gdb_index still gets used.
+# symtab/15885
+
+# There is gdb_touch_execfile, but it doesn't handle remote hosts.
+# Is touch portable enough?
+# First make sure enough time has passed, file mtime resolution is seconds.
+gdb_test_no_output "shell sleep 1"
+if {[run_on_host "touch binary" touch ${binfile_with_index}]} {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+gdb_test "mt print objfiles ${testfile}" \
+ "gdb_index.*" \
+ ".gdb_index used after symbol reloading"
Index: testsuite/lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.242
diff -u -p -r1.242 gdb.exp
--- testsuite/lib/gdb.exp 22 Aug 2013 14:01:04 -0000 1.242
+++ testsuite/lib/gdb.exp 24 Aug 2013 00:55:06 -0000
@@ -4443,5 +4443,30 @@ proc gdb_target_symbol_prefix_flags {} {
}
}

+# A wrapper for 'remote_exec host' that passes or fails a test.
+# Returns 0 if all went well, nonzero on failure.
+# TEST is the name of the test, other arguments are as for remote_exec.
+
+proc run_on_host { test program args } {
+ verbose -log "run_on_host: $program $args"
+ # remote_exec doesn't work properly if the output is set but the
+ # input is the empty string -- so replace an empty input with
+ # /dev/null.
+ if {[llength $args] > 1 && [lindex $args 1] == ""} {
+ set args [lreplace $args 1 1 "/dev/null"]
+ }
+ set result [eval remote_exec host [list $program] $args]
+ verbose "result is $result"
+ set status [lindex $result 0]
+ set output [lindex $result 1]
+ if {$status == 0} {
+ pass $test
+ return 0
+ } else {
+ fail $test
+ return -1
+ }
+}
+
# Always load compatibility stuff.
load_lib future.exp