[rfc/wip] More defensive gdbarch initialization

[Andrew Cagney <ac131313 at cygnus dot com>]

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 = &rego->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 = &rego->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 ----