This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [non-commit patch] Fix gdbserver qXfer:libraries-svr4 regression in special cases
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Paul Pluzhnikov <ppluzhnikov at google dot com>
- Date: Tue, 10 Apr 2012 22:06:04 +0200
- Subject: Re: [non-commit patch] Fix gdbserver qXfer:libraries-svr4 regression in special cases
- References: <20120303182730.GA14062@host2.jankratochvil.net> <4F54E15E.4020004@redhat.com>
On Mon, 05 Mar 2012 16:53:02 +0100, Pedro Alves wrote:
> GDBserver can ask GDB the address of _r_debug, using the qSymbols mechanism,
> so it doesn't look that much trouble. WDYT of the alternative patch below?
> It fixes the testcase too.
I worked on it and it works great; just the testcase was bogus.
The testcase did:
gdbserver ld.so prog &
gdb:
file ld.so
target remote ...
file prog
But in reality people do:
gdbserver ld.so prog &
gdb:
file prog
target remote ...
In this case qSymbol for _r_debug cannot work as GDB will never find any new
library, therefore GDB will never find ld.so to find _r_debug there.
A chicken-and-egg problem.
There was another issue being dealt with in the patch below that originally
GDB send qXfer:libraries-svr4:read before qSymbol and therefor GDB did the
expensive (and unsuccessful) non-accelerated reading from gdbserver for the
first time.
> @@ -4522,6 +4523,8 @@ linux_look_up_symbols (void)
> #ifdef USE_THREAD_DB
> struct process_info *proc = current_process ();
>
> + svr4_look_up_symbols ();
> +
> if (proc->private->thread_db != NULL)
> return;
>
- Also here it is put into USE_THREAD_DB conditional, I think it should not
be related to USE_THREAD_DB.
If you move 'gdb_test "file ${binfile}"'... down before the "continue" command
it works great; but it is not useful in practice.
So I plan to use the former patch of mine to just always fall-back to the
non-accelerated linux-nat reading of memory.
Thanks,
Jan
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 043451d..56d0d26 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -205,6 +205,7 @@ static int finish_step_over (struct lwp_info *lwp);
static CORE_ADDR get_stop_pc (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
static void linux_enable_event_reporting (int pid);
+static void svr4_look_up_symbols (void);
/* True if the low target can hardware single-step. Such targets
don't need a BREAKPOINT_REINSERT_ADDR callback. */
@@ -4656,6 +4657,8 @@ linux_look_up_symbols (void)
#ifdef USE_THREAD_DB
struct process_info *proc = current_process ();
+ svr4_look_up_symbols ();
+
if (proc->private->thread_db != NULL)
return;
@@ -5278,6 +5281,23 @@ linux_get_min_fast_tracepoint_insn_len (void)
return (*the_low_target.get_min_fast_tracepoint_insn_len) ();
}
+/* Cache all future symbols that svr4 handling might request. We can
+ not request symbols at arbitrary states in the remote protocol,
+ only when the client tells us that new symbols are available. */
+
+static void
+svr4_look_up_symbols (void)
+{
+ struct process_info *proc = current_process ();
+ struct process_info_private *priv = proc->private;
+
+ if (priv->r_debug != 0 && priv->r_debug != (CORE_ADDR) -1)
+ return;
+
+ clear_symbol_cache (¤t_process ()->symbol_cache);
+ look_up_one_symbol ("_r_debug", &priv->r_debug_from_syms, 1);
+}
+
/* Extract &phdr and num_phdr in the inferior. Return 0 on success. */
static int
@@ -5418,11 +5438,12 @@ get_dynamic (const int pid, const int is_elf64)
return 0;
}
-/* Return &_r_debug in the inferior, or -1 if not present. Return value
- can be 0 if the inferior does not yet have the library list initialized. */
+/* Return &_r_debug in the inferior (via PT_DYNAMIC), or -1 if not
+ present. Return value can be 0 if the inferior does not yet have
+ the library list initialized. */
static CORE_ADDR
-get_r_debug (const int pid, const int is_elf64)
+get_r_debug_dynamic (const int pid, const int is_elf64)
{
CORE_ADDR dynamic_memaddr;
const int dyn_size = is_elf64 ? sizeof (Elf64_Dyn) : sizeof (Elf32_Dyn);
@@ -5528,6 +5549,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
struct process_info_private *const priv = current_process ()->private;
char filename[PATH_MAX];
int pid, is_elf64;
+ CORE_ADDR r_debug;
static const struct link_map_offsets lmo_32bit_offsets =
{
@@ -5562,10 +5584,28 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
is_elf64 = elf_64_file_p (filename);
lmo = is_elf64 ? &lmo_64bit_offsets : &lmo_32bit_offsets;
+ /* We'll only try this once per inferior. */
if (priv->r_debug == 0)
- priv->r_debug = get_r_debug (pid, is_elf64);
+ {
+ priv->r_debug = get_r_debug_dynamic (pid, is_elf64);
- if (priv->r_debug == (CORE_ADDR) -1 || priv->r_debug == 0)
+ /* If there's no DT_DEBUG, r_debug will now be -1. */
+ }
+
+ /* This may be a static executable. Look for the symbol
+ conventionally named _r_debug, as a last resort. */
+ if (priv->r_debug == (CORE_ADDR) -1 && priv->r_debug_from_syms != 0)
+ r_debug = priv->r_debug_from_syms;
+ else
+ r_debug = priv->r_debug;
+
+ /* We failed to find DT_DEBUG. Such situation will not change for this
+ inferior - do not retry it. Report it to GDB as E01, see for the reasons
+ at the GDB solib-svr4.c side. */
+ if (r_debug == (CORE_ADDR) -1)
+ return -1;
+
+ if (r_debug == 0)
{
document = xstrdup ("<library-list-svr4 version=\"1.0\"/>\n");
}
@@ -5582,7 +5622,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
p = document + strlen (document);
r_version = 0;
- if (linux_read_memory (priv->r_debug + lmo->r_version_offset,
+ if (linux_read_memory (r_debug + lmo->r_version_offset,
(unsigned char *) &r_version,
sizeof (r_version)) != 0
|| r_version != 1)
@@ -5591,11 +5631,11 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
goto done;
}
- if (read_one_ptr (priv->r_debug + lmo->r_map_offset,
+ if (read_one_ptr (r_debug + lmo->r_map_offset,
&lm_addr, ptr_size) != 0)
{
warning ("unable to read r_map from 0x%lx",
- (long) priv->r_debug + lmo->r_map_offset);
+ (long) r_debug + lmo->r_map_offset);
goto done;
}
@@ -5661,6 +5701,11 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
p = p + strlen (p);
}
+ /* We have found at least one valid entry. No longer try to look up
+ R_DEBUG_FROM_SYMS which is expensive. */
+ if (priv->r_debug == (CORE_ADDR) -1 && priv->r_debug_from_syms != 0)
+ priv->r_debug = priv->r_debug_from_syms;
+
if (l_next == 0)
break;
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 07eda12..e25458b 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -56,8 +56,14 @@ struct process_info_private
thread_db, and it is active. */
struct thread_db *thread_db;
- /* &_r_debug. 0 if not yet determined. -1 if no PT_DYNAMIC in Phdrs. */
+ /* &_r_debug. 0 if not yet determined. -1 if no PT_DYNAMIC in
+ Phdrs, and not found by querying GDB's symbols for _r_debug's
+ address either. */
CORE_ADDR r_debug;
+
+ /* Fallback &_r_debug, from qSymbols, for static executables. 0 if
+ not asked/found yet. */
+ CORE_ADDR r_debug_from_syms;
};
struct lwp_info;
diff --git a/gdb/remote.c b/gdb/remote.c
index caf6116..2cf5657 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3406,6 +3406,9 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
get_offsets (); /* Get text, data & bss offsets. */
+ if (exec_bfd) /* No use without an exec file. */
+ remote_check_symbols (symfile_objfile);
+
/* If we could not find a description using qXfer, and we know
how to do it some other way, try again. This is not
supported for non-stop; it could be, but it is tricky if
@@ -3492,13 +3495,13 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
/* Report all signals during attach/startup. */
remote_pass_signals (0, NULL);
- }
- /* If we connected to a live target, do some additional setup. */
- if (target_has_execution)
- {
- if (exec_bfd) /* No use without an exec file. */
- remote_check_symbols (symfile_objfile);
+ /* If we connected to a live target, do some additional setup. */
+ if (target_has_execution)
+ {
+ if (exec_bfd) /* No use without an exec file. */
+ remote_check_symbols (symfile_objfile);
+ }
}
/* Possibly the target has been engaged in a trace run started
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 69d3cb5..9049443 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1260,6 +1260,14 @@ svr4_current_sos (void)
int ignore_first;
struct svr4_library_list library_list;
+ /* Fall back to manual examination of the target if the packet is not
+ supported or gdbserver failed to find DT_DEBUG. gdb.server/solib-list.exp
+ tests a case where gdbserver cannot find the shared libraries list while
+ GDB itself is able to find it via SYMFILE_OBJFILE.
+
+ Unfortunately statically linked inferiors will also fall back through this
+ suboptimal code path. */
+
if (svr4_current_sos_via_xfer_libraries (&library_list))
{
if (library_list.main_lm)
diff --git a/gdb/testsuite/gdb.server/solib-list-lib.c b/gdb/testsuite/gdb.server/solib-list-lib.c
new file mode 100644
index 0000000..504a34e
--- /dev/null
+++ b/gdb/testsuite/gdb.server/solib-list-lib.c
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2012 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/>. */
+
+#include <signal.h>
+
+static int libvar = 23;
+
+int
+libfunc (void)
+{
+ raise (SIGUSR1);
+
+ return libvar;
+}
diff --git a/gdb/testsuite/gdb.server/solib-list-main.c b/gdb/testsuite/gdb.server/solib-list-main.c
new file mode 100644
index 0000000..8abee3f
--- /dev/null
+++ b/gdb/testsuite/gdb.server/solib-list-main.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2012 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/>. */
+
+extern int libfunc (void);
+
+int
+main (void)
+{
+ libfunc ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.server/solib-list.exp b/gdb/testsuite/gdb.server/solib-list.exp
new file mode 100644
index 0000000..6d98549
--- /dev/null
+++ b/gdb/testsuite/gdb.server/solib-list.exp
@@ -0,0 +1,104 @@
+# Copyright 2010-2012 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/>. */
+
+# In some cases gdbserver cannot find the DT_DEBUG library list on its own and
+# it needs to fall back to GDB solib-svr4.c look up via symbols from the main
+# executable.
+# gdbserver :1234 ld-linux-x86-64.so.2 /bin/prog
+# gdb /bin/prog
+# (gdb) target remote ...
+
+load_lib gdbserver-support.exp
+load_lib prelink-support.exp
+
+if {[skip_gdbserver_tests] || [skip_shlib_tests]} {
+ return
+}
+
+set testfile "solib-list"
+set srclibfile ${testfile}-lib.c
+set srcfile ${testfile}-main.c
+set binlibfile ${objdir}/${subdir}/${testfile}.so
+set executable ${testfile}
+set objfile ${objdir}/${subdir}/${executable}.o
+set binfile ${objdir}/${subdir}/${executable}
+
+if { [get_compiler_info unused]
+ || [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfile}" "${binlibfile}" [list debug ldflags=-Wl,-soname,${binlibfile}]] != ""
+ || [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object {debug}] != ""
+ || [gdb_compile "${objfile} ${binlibfile}" "${binfile}" executable {}] != "" } {
+ untested "could not compile sources"
+ return -1
+}
+
+set interp_system [section_get ${binfile} .interp]
+verbose -log "system interpreter is: $interp_system"
+
+foreach nonstop { 0 1 } { with_test_prefix "non-stop $nonstop" {
+
+ # clean_restart assumes ${objdir}/${subdir}/ which is not applicable
+ # for ${interp_system}.
+ gdb_exit
+ gdb_start
+ gdb_reinitialize_dir $srcdir/$subdir
+ gdb_load ${interp_system}
+ gdb_load_shlibs ${binfile}
+ gdb_load_shlibs ${binlibfile}
+
+ # Make sure we're disconnected, in case we're testing with an
+ # extended-remote board, therefore already connected.
+ gdb_test "disconnect" ".*"
+
+ gdb_test "set non-stop $nonstop"
+ gdb_test "set target-async $nonstop"
+ #gdb_test "set debug remote 1"
+
+ # It is required for the non-stop mode, GDB would try to step over
+ # _dl_debug_state breakpoint will still only ld.so loaded in gdbserver.
+ # But GDB having symbols from the main executable it would try to use
+ # displaced-stepping buffer at unmapped that time address _start.
+ gdb_test "set displaced-stepping off"
+
+ set res [gdbserver_spawn ${binfile}]
+ set gdbserver_protocol [lindex $res 0]
+ set gdbserver_gdbport [lindex $res 1]
+
+ # Load BINFILE after gdbserver_spawn (so that gdbserver executes ld.so)
+ # but before "target remote" below so that qSymbol data get already
+ # initialized from BINFILE (and not from ld.so first needing a change to
+ # BINFILE later).
+ gdb_test "file ${binfile}" {Reading symbols from .*\.\.\.done\..*} "file binfile" \
+ {(Are you sure you want to change the file|Load new symbol table from ".*")\? \(y or n\) } "y"
+
+ set test "target $gdbserver_protocol"
+ gdb_test_multiple "target $gdbserver_protocol $gdbserver_gdbport" $test {
+ -re "Remote debugging using .*\r\n$gdb_prompt " {
+ # Do not anchor end, there may be more output in non-stop mode.
+ pass $test
+ }
+ }
+
+ if $nonstop {
+ set test "non-stop interior stop"
+ gdb_test_multiple "" $test {
+ -re " #1 stopped\\.\r\n" {
+ pass $test
+ }
+ }
+ }
+
+ gdb_test "continue" "Program received signal SIGUSR1, .*"
+ gdb_test "p libvar" " = 23"
+}}