This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
[rfc/wip] More defensive gdbarch initialization
- To: gdb-patches at sources dot redhat dot com
- Subject: [rfc/wip] More defensive gdbarch initialization
- From: Andrew Cagney <ac131313 at cygnus dot com>
- Date: Fri, 27 Jul 2001 18:48:54 -0400
Hello,
This is fallout from the sh-tdep.c bug Elena recently fixed.
I got curious as to how many multi-arch targets might unintentionally be
refering to the old ``current_gdbarch'' instead of the new gdbarch being
created.
For instance, code like:
set_gdbarch_double_bit (gdbarch, 8*TARGET_CHAR_BIT);
set_gdbarch_long_double_bit (gdbarch, 2*TARGET_DOUBLE_BIT);
would be wrong. TARGET_DOUBLE_BIT refers to the old
``current_gdbarch'''s double-bit and not ``gdbarch''. Did anyone
mention ``Macro's are bad, M'kay''?
So anyway, my idea was to, for the duration of the XXX_gdbarch_init()
call, invalidate ``current_gdbarch''. That way the target code couldn't
use it. The patch below is work-in-progress on that line.
The thing that makes this funny is that the first bug I found wasn't in
any target code. Rather it was in gdbarch.* proper. gdbarch_alloc()
contains:
gdbarch->long_long_bit = 2*TARGET_LONG_BIT;
gdbarch->long_double_bit = 2*TARGET_DOUBLE_BIT;
gdbarch->ptr_bit = TARGET_INT_BIT;
gdbarch->bfd_vma_bit = TARGET_ARCHITECTURE->bits_per_address;
Ulgh ...
You can expect some ``obvious fixes'' related to this over the next ew
days. I'll delay any decision to commit something like the change below
until after 5.1 has branched, it is proving a little too effective in
finding bugs :-)
Oh for the day when current_gdbarch isn't a global.
Andrew
2001-07-27 Andrew Cagney <ac131313@redhat.com>
* gdbarch.sh (bzero_gdbarch_swap): New function.
(swapin_gdbarch_swap): Update ``current_gdbarch''.
(gdbarch_update_p): Always swap out the old architecture before
trying to initialize a new one. Stops the init() function using
the wrong architectures information.
Index: gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.69
diff -p -r1.69 gdbarch.c
*** gdbarch.c 2001/07/10 21:24:48 1.69
--- gdbarch.c 2001/07/27 22:25:57
*************** swapout_gdbarch_swap (struct gdbarch *gd
*** 4479,4484 ****
--- 4479,4498 ----
}
static void
+ bzero_gdbarch_swap (void)
+ {
+ struct gdbarch_swap_registration *rego;
+ for (rego = gdbarch_swap_registry.registrations;
+ rego != NULL;
+ rego = rego->next)
+ {
+ if (rego->data != NULL)
+ memset (rego->data, 0, rego->sizeof_data);
+ }
+ current_gdbarch = NULL;
+ }
+
+ static void
swapin_gdbarch_swap (struct gdbarch *gdbarch)
{
struct gdbarch_swap *curr;
*************** swapin_gdbarch_swap (struct gdbarch *gdb
*** 4486,4491 ****
--- 4500,4506 ----
curr != NULL;
curr = curr->next)
memcpy (curr->source->data, curr->swap, curr->source->sizeof_data);
+ current_gdbarch = gdbarch;
}
*************** int
*** 4626,4631 ****
--- 4641,4647 ----
gdbarch_update_p (struct gdbarch_info info)
{
struct gdbarch *new_gdbarch;
+ struct gdbarch *old_gdbarch;
struct gdbarch_list **list;
struct gdbarch_registration *rego;
*************** gdbarch_update_p (struct gdbarch_info in
*** 4695,4701 ****
return 0;
}
! /* Ask the target for a replacement architecture. */
new_gdbarch = rego->init (info, rego->arches);
/* Did the target like it? No. Reject the change. */
--- 4711,4727 ----
return 0;
}
! /* Swap/save all data belonging to the old target out. */
! old_gdbarch = current_gdbarch;
! swapout_gdbarch_swap (old_gdbarch);
!
! /* Clear CURRENT_GDBARCH and all the swap space so that the ->init()
! call below can't accidently refer to anything from the previous
! architecture - this hopefully stops people using macros such as
! TARGET_PTR_BIT. */
! bzero_gdbarch_swap ();
!
! /* Ask the target for a replacement architecture. */
new_gdbarch = rego->init (info, rego->arches);
/* Did the target like it? No. Reject the change. */
*************** gdbarch_update_p (struct gdbarch_info in
*** 4703,4724 ****
{
if (gdbarch_debug)
fprintf_unfiltered (gdb_stdlog, "gdbarch_update: Target rejected architecture\n");
return 0;
}
/* Did the architecture change? No. Do nothing. */
! if (current_gdbarch == new_gdbarch)
{
if (gdbarch_debug)
fprintf_unfiltered (gdb_stdlog, "gdbarch_update: Architecture 0x%08lx (%s) unchanged\n",
(long) new_gdbarch,
new_gdbarch->bfd_arch_info->printable_name);
return 1;
}
- /* Swap all data belonging to the old target out */
- swapout_gdbarch_swap (current_gdbarch);
-
/* Is this a pre-existing architecture? Yes. Swap it in. */
for (list = ®o->arches;
(*list) != NULL;
--- 4729,4749 ----
{
if (gdbarch_debug)
fprintf_unfiltered (gdb_stdlog, "gdbarch_update: Target rejected architecture\n");
+ swapin_gdbarch_swap (old_gdbarch);
return 0;
}
/* Did the architecture change? No. Do nothing. */
! if (old_gdbarch == new_gdbarch)
{
if (gdbarch_debug)
fprintf_unfiltered (gdb_stdlog, "gdbarch_update: Architecture 0x%08lx (%s) unchanged\n",
(long) new_gdbarch,
new_gdbarch->bfd_arch_info->printable_name);
+ swapin_gdbarch_swap (old_gdbarch);
return 1;
}
/* Is this a pre-existing architecture? Yes. Swap it in. */
for (list = ®o->arches;
(*list) != NULL;
*************** gdbarch_update_p (struct gdbarch_info in
*** 4731,4737 ****
"gdbarch_update: Previous architecture 0x%08lx (%s) selected\n",
(long) new_gdbarch,
new_gdbarch->bfd_arch_info->printable_name);
- current_gdbarch = new_gdbarch;
swapin_gdbarch_swap (new_gdbarch);
return 1;
}
--- 4756,4761 ----