[PATCH] Multi-process + multi-arch: GDB

Pedro Alves
2012-11-09T01:52:05Z

This patch was part of a series I had posted back in
<http://sourceware.org/ml/gdb-patches/2012-04/msg00202.html>. I've
been distracted from that work since then, but since questions similar
or related to "how can we have more than one gdbarch" showed up on the
python architecture thread, I remembered I could push this part in
without the rest. The rest of the series depends on this (and will
need further porting work, since we have more gdbserver ports in the
tree now, and at least a couple more pending...), but this can go in
first and independently. It fixes the issue for native targets.

I had a chance of chatting with Ulrich at FOSDEM earlier in the year,
and he also thought this was a good direction, so I'm putting it in.

Although GDB currently supports debugging multiple inferiors
simultaneously, GDB is not prepared to handle the case of debugging
programs of different architectures (gdbarch's) simultaneously. A
simple example is debugging both a 32-bit and a 64-bit program
simultaneously on x86_64 (think following forks, and 32-bit and/or
64-bit children being spawned), though more complicated examples could
be multi-core setups with processors on the same target (e.g.,
x86_64 + ARM isn't uncommon these days).

E.g., loading both a 64-bit inferior and a 32-bit inferior into GDB
shows (from the test in the patch):

(gdb) file test64
Reading symbols from test64...done.
(gdb) delete breakpoints
(gdb) break main
Breakpoint 1 at 0x40053a: file main.c, line 40.
(gdb) run
Starting program: test64

Breakpoint 1, main () at main.c:40
40 bar();
(gdb) add-inferior
Added inferior 2
inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) file test32
warning: Selected architecture i386 is not compatible with reported target architecture i386:x86-64
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Architecture of file not recognized.

The "Architecture of file not recognized" warning is misleading. The
real issue is that target_gdbarch and the target description are
globals. So instead of fetching the target description for the 32-bit
process that was created for the `test32' program, the target
description that was previously fetched for inferior 1 (64-bit) is
assumed, and then things (rightfully) break, as it doesn't really
match inferior 2.

This patch makes target_gdbarch (and the associated description) be
per-inferior instead.

The new test fails without the rest of the patch.

I've made the adjustment Yao pointed out in the old thread.

(Re)tested on x86_64 Fedora 17, and checked in.

gdb/
2012-11-09 Pedro Alves <palves@redhat.com>

* gdbarch.sh (target_gdbarch) <gdbarch.h>: Reimplement as macro.
(get_target_gdbarch) <gdbarch.h>: New function.
(startup_gdbarch) <gdbarch.h>: Declare.
<gdbarch.c> (target_gdbarch): Delete.
<gdbarch.c> (deprecated_target_gdbarch_select_hack): Set the
current inferior's gdbarch.
<gdbarch.c> (get_target_gdbarch): New function.
* inferior.c: Include target-descriptions.h.
(free_inferior): Free target description info.
(add_inferior_with_spaces): Set the inferior's initial
architecture.
(clone_inferior_command): Copy the original inferior's target
description if it was user specified.
(initialize_inferiors): Add comment.
* inferior.h (struct target_desc_info): Forward declare.
(struct inferior) <gdbarch>: New field.
* linux-nat.c: Include target-descriptions.h.
(linux_child_follow_fork): Copy the parent's architecture and
target description to the child.
* target-descriptions.c: Include inferior.h.
(struct target_desc_info): New structure, holding the equivalents
of ...
(target_desc_fetched, current_target_desc)
(target_description_filename): ... these removed globals.
(get_tdesc_info, target_desc_info_from_user_p)
(copy_inferior_target_desc_info, target_desc_info_free): New.
(target_desc_fetched, current_target_desc)
(target_description_filename): Reimplemented as convenience
macros.
(tdesc_filename_cmd_string): New global.
(set_tdesc_filename_cmd): Copy the string manipulated by the "set
tdescs filename ..." commands to the per-inferior equivalent.
(show_tdesc_filename_cmd): Get the value to show from the
per-inferior description filename.
(_initilize_target_descriptions): Change the "set/show tdesc
filename" commands' variable.
* target-descriptions.h (struct target_desc, struct target_desc_info)
(struct inferior): Forward declare.
(target_find_description, target_clear_description)
(target_current_description): Adjust comments.
(copy_inferior_target_desc_info, target_desc_info_free)
(target_desc_info_from_user_p). Declare.

gdb/testsuite/
2012-11-09 Pedro Alves <palves@redhat.com>

* gdb.multi/multi-arch.exp: New.
---
gdb/gdbarch.c | 11 +++-
gdb/gdbarch.h | 31 +++++++---
gdb/gdbarch.sh | 42 ++++++++++----
gdb/inferior.c | 20 +++++++
gdb/inferior.h | 18 ++++++
gdb/linux-nat.c | 5 ++
gdb/target-descriptions.c | 96 +++++++++++++++++++++++++++-----
gdb/target-descriptions.h | 34 +++++++++--
gdb/testsuite/gdb.multi/multi-arch.exp | 76 +++++++++++++++++++++++++
9 files changed, 285 insertions(+), 48 deletions(-)
create mode 100644 gdb/testsuite/gdb.multi/multi-arch.exp

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 6bc57af..25e4750 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -457,7 +457,6 @@ struct gdbarch startup_gdbarch =
/* startup_gdbarch() */
};

-struct gdbarch *target_gdbarch = &startup_gdbarch;

/* Create a new ``struct gdbarch'' based on information provided by
``struct gdbarch_info''. */
@@ -4671,11 +4670,19 @@ deprecated_target_gdbarch_select_hack (struct gdbarch *new_gdbarch)
{
gdb_assert (new_gdbarch != NULL);
gdb_assert (new_gdbarch->initialized_p);
- target_gdbarch = new_gdbarch;
+ current_inferior ()->gdbarch = new_gdbarch;
observer_notify_architecture_changed (new_gdbarch);
registers_changed ();
}

+/* Helper for 'target_gdbarch'. */
+
+struct gdbarch *
+get_target_gdbarch (void)
+{
+ return current_inferior ()->gdbarch;
+}
+
extern void _initialize_gdbarch (void);

void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 696bf14..5afb3db 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -58,17 +58,26 @@ struct agent_expr;
struct axs_value;
struct stap_parse_info;

-/* The architecture associated with the connection to the target.
-
- The architecture vector provides some information that is really
- a property of the target: The layout of certain packets, for instance;
- or the solib_ops vector. Etc. To differentiate architecture accesses
- to per-target properties from per-thread/per-frame/per-objfile properties,
- accesses to per-target properties should be made through target_gdbarch.
-
- Eventually, when support for multiple targets is implemented in
- GDB, this global should be made target-specific. */
-extern struct gdbarch *target_gdbarch;
+/* The architecture associated with the inferior through the
+ connection to the target.
+
+ The architecture vector provides some information that is really a
+ property of the inferior, accessed through a particular target:
+ ptrace operations; the layout of certain RSP packets; the solib_ops
+ vector; etc. To differentiate architecture accesses to
+ per-inferior/target properties from
+ per-thread/per-frame/per-objfile properties, accesses to
+ per-inferior/target properties should be made through this
+ gdbarch. */
+
+/* This is a convenience wrapper for 'current_inferior ()->gdbarch'. */
+#define target_gdbarch get_target_gdbarch ()
+extern struct gdbarch *get_target_gdbarch (void);
+
+/* The initial, default architecture. It uses host values (for want of a better
+ choice). */
+extern struct gdbarch startup_gdbarch;
+

/* Callback type for the 'iterate_over_objfiles_in_search_order'
gdbarch method. */
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index e3e6329..13d5cc6 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1076,17 +1076,26 @@ struct agent_expr;
struct axs_value;
struct stap_parse_info;

-/* The architecture associated with the connection to the target.
-
- The architecture vector provides some information that is really
- a property of the target: The layout of certain packets, for instance;
- or the solib_ops vector. Etc. To differentiate architecture accesses
- to per-target properties from per-thread/per-frame/per-objfile properties,
- accesses to per-target properties should be made through target_gdbarch.
-
- Eventually, when support for multiple targets is implemented in
- GDB, this global should be made target-specific. */
-extern struct gdbarch *target_gdbarch;
+/* The architecture associated with the inferior through the
+ connection to the target.
+
+ The architecture vector provides some information that is really a
+ property of the inferior, accessed through a particular target:
+ ptrace operations; the layout of certain RSP packets; the solib_ops
+ vector; etc. To differentiate architecture accesses to
+ per-inferior/target properties from
+ per-thread/per-frame/per-objfile properties, accesses to
+ per-inferior/target properties should be made through this
+ gdbarch. */
+
+/* This is a convenience wrapper for 'current_inferior ()->gdbarch'. */
+#define target_gdbarch get_target_gdbarch ()
+extern struct gdbarch *get_target_gdbarch (void);
+
+/* The initial, default architecture. It uses host values (for want of a better
+ choice). */
+extern struct gdbarch startup_gdbarch;
+

/* Callback type for the 'iterate_over_objfiles_in_search_order'
gdbarch method. */
@@ -1559,7 +1568,6 @@ cat <<EOF
/* startup_gdbarch() */
};

-struct gdbarch *target_gdbarch = &startup_gdbarch;
EOF

# Create a new gdbarch struct
@@ -2278,11 +2286,19 @@ deprecated_target_gdbarch_select_hack (struct gdbarch *new_gdbarch)
{
gdb_assert (new_gdbarch != NULL);
gdb_assert (new_gdbarch->initialized_p);
- target_gdbarch = new_gdbarch;
+ current_inferior ()->gdbarch = new_gdbarch;
observer_notify_architecture_changed (new_gdbarch);
registers_changed ();
}

+/* Helper for 'target_gdbarch'. */
+
+struct gdbarch *
+get_target_gdbarch (void)
+{
+ return current_inferior ()->gdbarch;
+}
+
extern void _initialize_gdbarch (void);

void
diff --git a/gdb/inferior.c b/gdb/inferior.c
index f46e1e3..5bb1514 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -33,6 +33,8 @@
#include "environ.h"
#include "cli/cli-utils.h"
#include "continuations.h"
+#include "arch-utils.h"
+#include "target-descriptions.h"

void _initialize_inferiors (void);

@@ -98,6 +100,7 @@ free_inferior (struct inferior *inf)
xfree (inf->args);
xfree (inf->terminal);
free_environ (inf->environment);
+ target_desc_info_free (inf->tdesc_info);
xfree (inf->private);
xfree (inf);
}
@@ -794,6 +797,7 @@ add_inferior_with_spaces (void)
struct address_space *aspace;
struct program_space *pspace;
struct inferior *inf;
+ struct gdbarch_info info;

/* If all inferiors share an address space on this system, this
doesn't really return a new address space; otherwise, it
@@ -804,6 +808,14 @@ add_inferior_with_spaces (void)
inf->pspace = pspace;
inf->aspace = pspace->aspace;

+ /* Setup the inferior's initial arch, based on information obtained
+ from the global "set ..." options. */
+ gdbarch_info_init (&info);
+ inf->gdbarch = gdbarch_find_by_info (info);
+ /* The "set ..." options reject invalid settings, so we should
+ always have a valid arch by now. */
+ gdb_assert (inf->gdbarch != NULL);
+
return inf;
}

@@ -942,6 +954,12 @@ clone_inferior_command (char *args, int from_tty)
inf = add_inferior (0);
inf->pspace = pspace;
inf->aspace = pspace->aspace;
+ inf->gdbarch = orginf->gdbarch;
+
+ /* If the original inferior had a user specified target
+ description, make the clone use it too. */
+ if (target_desc_info_from_user_p (inf->tdesc_info))
+ copy_inferior_target_desc_info (inf, orginf);

printf_filtered (_("Added inferior %d.\n"), inf->num);

@@ -977,6 +995,8 @@ initialize_inferiors (void)
current_inferior_ = add_inferior (0);
current_inferior_->pspace = current_program_space;
current_inferior_->aspace = current_program_space->aspace;
+ /* The architecture will be initialized shortly, by
+ initialize_current_architecture. */

add_info ("inferiors", info_inferiors_command,
_("IDs of specified inferiors (all inferiors if no argument)."));
diff --git a/gdb/inferior.h b/gdb/inferior.h
index b2607c3..e1e7d29 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -30,6 +30,7 @@ struct gdbarch;
struct regcache;
struct ui_out;
struct terminal_info;
+struct target_desc_info;

#include "ptid.h"

@@ -513,6 +514,23 @@ struct inferior
from enum symfile_add_flags. */
int symfile_flags;

+ /* Info about an inferior's target description (if it's fetched; the
+ user supplied description's filename, if any; etc.). */
+ struct target_desc_info *tdesc_info;
+
+ /* The architecture associated with the inferior through the
+ connection to the target.
+
+ The architecture vector provides some information that is really
+ a property of the inferior, accessed through a particular target:
+ ptrace operations; the layout of certain RSP packets; the
+ solib_ops vector; etc. To differentiate architecture accesses to
+ per-inferior/target properties from
+ per-thread/per-frame/per-objfile properties, accesses to
+ per-inferior/target properties should be made through
+ this gdbarch. */
+ struct gdbarch *gdbarch;
+
/* Per inferior data-pointers required by other GDB modules. */
REGISTRY_FIELDS;
};
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 3516565..c282e5d 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -66,6 +66,7 @@
#include "exceptions.h"
#include "linux-ptrace.h"
#include "buffer.h"
+#include "target-descriptions.h"

#ifndef SPUFS_MAGIC
#define SPUFS_MAGIC 0x23c9b64e
@@ -723,6 +724,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
parent_inf = current_inferior ();
child_inf->attach_flag = parent_inf->attach_flag;
copy_terminal_info (child_inf, parent_inf);
+ child_inf->gdbarch = parent_inf->gdbarch;
+ copy_inferior_target_desc_info (child_inf, parent_inf);

old_chain = save_inferior_ptid ();
save_current_program_space ();
@@ -887,6 +890,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
parent_inf = current_inferior ();
child_inf->attach_flag = parent_inf->attach_flag;
copy_terminal_info (child_inf, parent_inf);
+ child_inf->gdbarch = parent_inf->gdbarch;
+ copy_inferior_target_desc_info (child_inf, parent_inf);

parent_pspace = parent_inf->pspace;

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 8b81730..468fe42 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -34,6 +34,7 @@
#include "gdb_assert.h"
#include "gdb_obstack.h"
#include "hashtab.h"
+#include "inferior.h"

/* Types. */

@@ -231,34 +232,92 @@ struct tdesc_arch_data
gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p;
};

-/* Global state. These variables are associated with the current
- target; if GDB adds support for multiple simultaneous targets, then
- these variables should become target-specific data. */
+/* Info about an inferior's target description. There's one of these
+ for each inferior. */

-/* A flag indicating that a description has already been fetched from
- the current target, so it should not be queried again. */
+struct target_desc_info
+{
+ /* A flag indicating that a description has already been fetched
+ from the target, so it should not be queried again. */
+
+ int fetched;

-static int target_desc_fetched;
+ /* The description fetched from the target, or NULL if the target
+ did not supply any description. Only valid when
+ target_desc_fetched is set. Only the description initialization
+ code should access this; normally, the description should be
+ accessed through the gdbarch object. */

-/* The description fetched from the current target, or NULL if the
- current target did not supply any description. Only valid when
- target_desc_fetched is set. Only the description initialization
- code should access this; normally, the description should be
- accessed through the gdbarch object. */
+ const struct target_desc *tdesc;

-static const struct target_desc *current_target_desc;
+ /* The filename to read a target description from, as set by "set
+ tdesc filename ..." */

-/* Other global variables. */
+ char *filename;
+};

-/* The filename to read a target description from. */
+/* Get the inferior INF's target description info, allocating one on
+ the stop if necessary. */

-static char *target_description_filename;
+static struct target_desc_info *
+get_tdesc_info (struct inferior *inf)
+{
+ if (inf->tdesc_info == NULL)
+ inf->tdesc_info = XCNEW (struct target_desc_info);
+ return inf->tdesc_info;
+}

/* A handle for architecture-specific data associated with the
target description (see struct tdesc_arch_data). */

static struct gdbarch_data *tdesc_data;

+/* See target-descriptions.h. */
+
+int
+target_desc_info_from_user_p (struct target_desc_info *info)
+{
+ return info != NULL && info->filename != NULL;
+}
+
+/* See target-descriptions.h. */
+
+void
+copy_inferior_target_desc_info (struct inferior *destinf, struct inferior *srcinf)
+{
+ struct target_desc_info *src = get_tdesc_info (srcinf);
+ struct target_desc_info *dest = get_tdesc_info (destinf);
+
+ dest->fetched = src->fetched;
+ dest->tdesc = src->tdesc;
+ dest->filename = src->filename != NULL ? xstrdup (src->filename) : NULL;
+}
+
+/* See target-descriptions.h. */
+
+void
+target_desc_info_free (struct target_desc_info *tdesc_info)
+{
+ if (tdesc_info != NULL)
+ {
+ xfree (tdesc_info->filename);
+ xfree (tdesc_info);
+ }
+}
+
+/* Convenience helper macros. */
+
+#define target_desc_fetched \
+ get_tdesc_info (current_inferior ())->fetched
+#define current_target_desc \
+ get_tdesc_info (current_inferior ())->tdesc
+#define target_description_filename \
+ get_tdesc_info (current_inferior ())->filename
+
+/* The string manipulated by the "set tdesc filename ..." command. */
+
+static char *tdesc_filename_cmd_string;
+
/* Fetch the current target's description, and switch the current
architecture to one which incorporates that description. */

@@ -1510,6 +1569,9 @@ static void
set_tdesc_filename_cmd (char *args, int from_tty,
struct cmd_list_element *c)
{
+ xfree (target_description_filename);
+ target_description_filename = xstrdup (tdesc_filename_cmd_string);
+
target_clear_description ();
target_find_description ();
}
@@ -1519,6 +1581,8 @@ show_tdesc_filename_cmd (struct ui_file *file, int from_tty,
struct cmd_list_element *c,
const char *value)
{
+ value = target_description_filename;
+
if (value != NULL && *value != '\0')
printf_filtered (_("The target description will be read from \"%s\".\n"),
value);
@@ -1758,7 +1822,7 @@ Unset target description specific variables."),
0 /* allow-unknown */, &unsetlist);

add_setshow_filename_cmd ("filename", class_obscure,
- &target_description_filename,
+ &tdesc_filename_cmd_string,
_("\
Set the file to read for an XML target description"), _("\
Show the file to read for an XML target description"), _("\
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 6a09a90..f0841d1 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -28,23 +28,45 @@ struct tdesc_type;
struct tdesc_reg;
struct target_desc;
struct target_ops;
+struct target_desc;
+/* An inferior's target description info is stored in this opaque
+ object. There's one such object per inferior. */
+struct target_desc_info;
+struct inferior;

-/* Fetch the current target's description, and switch the current
+/* Fetch the current inferior's description, and switch its current
architecture to one which incorporates that description. */

void target_find_description (void);

-/* Discard any description fetched from the current target, and switch
- the current architecture to one with no target description. */
+/* Discard any description fetched from the target for the current
+ inferior, and switch the current architecture to one with no target
+ description. */

void target_clear_description (void);

-/* Return the global current target description. This should only be
- used by gdbarch initialization code; most access should be through
- an existing gdbarch. */
+/* Return the current inferior's target description. This should only
+ be used by gdbarch initialization code; most access should be
+ through an existing gdbarch. */

const struct target_desc *target_current_description (void);

+/* Copy inferior target description data. Used for example when
+ handling (v)forks, where child's description is the same as the
+ parent's, since the child really is a copy of the parent. */
+
+void copy_inferior_target_desc_info (struct inferior *destinf,
+ struct inferior *srcinf);
+
+/* Free a target_desc_info object. */
+
+void target_desc_info_free (struct target_desc_info *tdesc_info);
+
+/* Returns true if INFO indicates the target description had been
+ supplied by the user. */
+
+int target_desc_info_from_user_p (struct target_desc_info *info);
+
/* Record architecture-specific functions to call for pseudo-register
support. If tdesc_use_registers is called and gdbarch_num_pseudo_regs
is greater than zero, then these should be called as well.
diff --git a/gdb/testsuite/gdb.multi/multi-arch.exp b/gdb/testsuite/gdb.multi/multi-arch.exp
new file mode 100644
index 0000000..0adc127
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/multi-arch.exp
@@ -0,0 +1,76 @@
+# Copyright 2009-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/>.
+
+# Test multi-exec / multi-process features that work for all configurations,
+# even ones that cannot run multiple processes simultaneously.
+
+set testfile "multi-arch"
+
+# The plain remote target can't do multiple inferiors.
+if [target_info exists use_gdb_stub] {
+ return
+}
+
+# Can't use standard_testfile, we want executables with specialized
+# names.
+set exec1 "ma-hello"
+set srcfile1 hello.c
+set binfile1 [standard_output_file ${exec1}]
+
+set exec2 "ma-hangout"
+set srcfile2 hangout.c
+set binfile2 [standard_output_file ${exec2}]
+
+# Build two executables, one for each arch.
+
+if [istarget "s390*-*-*"] {
+ set march1 "-m64"
+ set march2 "-m31"
+} else {
+ set march1 "-m64"
+ set march2 "-m32"
+}
+
+if { [prepare_for_testing ${testfile}.exp ${exec1} "${srcfile1}" \
+ [list debug nowarnings additional_flags=${march1}]] } {
+ return -1
+}
+
+if { [prepare_for_testing ${testfile}.exp ${exec2} "${srcfile2}" \
+ [list debug nowarnings additional_flags=${march2}]] } {
+ return -1
+}
+
+# Start inferior 1
+
+clean_restart ${exec1}
+if ![runto_main] then {
+ fail "starting inferior 1"
+}
+
+# Add and start inferior 2
+
+gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
+gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2"
+gdb_load ${binfile2}
+
+if ![runto_main] then {
+ fail "starting inferior 2"
+}
+
+# Check we do have two inferiors loaded.
+
+gdb_test "info inferiors" \
+ "Executable.*${exec2}.*${exec1}.*"