RE: H8300 Patch - Fix GDB crash problem when object file of different H8 cpu is loaded

Dhananjay R. Deshpande
2003-12-01T12:58:01Z

Hi,

I think there is no need to share the variables in GDB & SIM. The Simulator can keep using its variables and for GDB we can have the functions as suggested by Andrew.

We need to use different names to functions as the variable name and function name conflicts.

Please see if the patch below makes sense.

-Dhananjay

diff -upr --strip-trailing-cr gdb+dejagnu-20031114.orig/gdb/config/h8300/tm-h8300.h gdb+dejagnu-20031114/gdb/config/h8300/tm-h8300.h
--- gdb+dejagnu-20031114.orig/gdb/config/h8300/tm-h8300.h 2003-10-17 18:13:06.000000000 +0530
+++ gdb+dejagnu-20031114/gdb/config/h8300/tm-h8300.h 2003-12-01 16:55:44.000000000 +0530
@@ -20,20 +20,15 @@
Boston, MA 02111-1307, USA. */
/* Contributed by Steve Chamberlain sac@cygnus.com */

-/* NOTE: ezannoni 2000-07-18: these variables are part of sim, defined
- in sim/h8300/compile.c. They really should not be used this
- way. Because of this we cannot get rid of the macro
- GDB_TARGET_IS_H8300 in remote-e7000.c */
-extern int h8300hmode;
-extern int h8300smode;
-extern int h8300_normal_mode; /* 1 - Normal Mode , 0 - Advanced mode */
-extern int h8300sxmode;
-#define GDB_TARGET_IS_H8300
-
/* Needed for remote.c */
#define DEPRECATED_REMOTE_BREAKPOINT { 0x57, 0x30} /* trapa #3 */
/* Needed for remote-hms.c */
#define CCR_REGNUM 8
/* Needed for remote-e7000.c */
-#define NUM_REALREGS (h8300smode?11:10)
+#define NUM_REALREGS (is_h8300smode()?11:10)
+
+extern int is_h8300hmode (void);
+extern int is_h8300smode (void);
+extern int is_h8300sxmode (void);
+extern int is_h8300_normal_mode (void);

diff -upr --strip-trailing-cr gdb+dejagnu-20031114.orig/gdb/h8300-tdep.c gdb+dejagnu-20031114/gdb/h8300-tdep.c
--- gdb+dejagnu-20031114.orig/gdb/h8300-tdep.c 2003-10-17 18:13:04.000000000 +0530
+++ gdb+dejagnu-20031114/gdb/h8300-tdep.c 2003-12-01 16:55:33.000000000 +0530
@@ -49,7 +49,7 @@ enum
h8300h_reg_size = 4,
h8300_max_reg_size = 4,
};
-#define BINWORD (h8300hmode && !h8300_normal_mode ? h8300h_reg_size : h8300_reg_size)
+#define BINWORD (is_h8300hmode() && !is_h8300_normal_mode() ? h8300h_reg_size : h8300_reg_size)

enum gdb_regnum
{
@@ -350,7 +350,7 @@ h8300_examine_prologue (CORE_ADDR ip, CO
}

/* If the PC isn't valid, quit now. */
- if (ip == 0 || ip & (h8300hmode && !h8300_normal_mode ? ~0xffffff : ~0xffff))
+ if (ip == 0 || ip & (is_h8300hmode() && !is_h8300_normal_mode() ? ~0xffffff : ~0xffff))
return 0;

next_ip = h8300_next_prologue_insn (ip, limit, &insn_word);
@@ -947,7 +947,7 @@ h8300_print_register (struct gdbarch *gd
rval = get_frame_register_signed (frame, regno);

fprintf_filtered (file, "%-14s ", name);
- if (regno == E_PSEUDO_CCR_REGNUM || (regno == E_PSEUDO_EXR_REGNUM && h8300smode))
+ if (regno == E_PSEUDO_CCR_REGNUM || (regno == E_PSEUDO_EXR_REGNUM && is_h8300smode()))
{
fprintf_filtered (file, "0x%02x ", (unsigned char)rval);
print_longest (file, 'u', 1, rval);
@@ -996,7 +996,7 @@ h8300_print_register (struct gdbarch *gd
if ((Z | (N ^ V)) == 1)
fprintf_filtered (file, "<= ");
}
- else if (regno == E_PSEUDO_EXR_REGNUM && h8300smode)
+ else if (regno == E_PSEUDO_EXR_REGNUM && is_h8300smode())
{
/* EXR register */
unsigned char l = rval & 0xff;
@@ -1019,10 +1019,10 @@ h8300_print_registers_info (struct gdbar
h8300_print_register (gdbarch, file, frame, regno);
h8300_print_register (gdbarch, file, frame, E_PSEUDO_CCR_REGNUM);
h8300_print_register (gdbarch, file, frame, E_PC_REGNUM);
- if (h8300smode)
+ if (is_h8300smode())
{
h8300_print_register (gdbarch, file, frame, E_PSEUDO_EXR_REGNUM);
- if (h8300sxmode)
+ if (is_h8300sxmode())
{
h8300_print_register (gdbarch, file, frame, E_SBR_REGNUM);
h8300_print_register (gdbarch, file, frame, E_VBR_REGNUM);
@@ -1044,7 +1044,7 @@ h8300_print_registers_info (struct gdbar
{
if (regno == E_CCR_REGNUM)
h8300_print_register (gdbarch, file, frame, E_PSEUDO_CCR_REGNUM);
- else if (regno == E_PSEUDO_EXR_REGNUM && h8300smode)
+ else if (regno == E_PSEUDO_EXR_REGNUM && is_h8300smode())
h8300_print_register (gdbarch, file, frame, E_PSEUDO_EXR_REGNUM);
else
h8300_print_register (gdbarch, file, frame, regno);
@@ -1078,7 +1078,7 @@ h8300_register_type (struct gdbarch *gdb
return builtin_type_uint8;
else if (regno == E_PSEUDO_EXR_REGNUM)
return builtin_type_uint8;
- else if (h8300hmode)
+ else if (is_h8300hmode())
return builtin_type_int32;
else
return builtin_type_int16;
@@ -1192,9 +1192,6 @@ h8300_gdbarch_init (struct gdbarch_info
switch (info.bfd_arch_info->mach)
{
case bfd_mach_h8300:
- h8300sxmode = 0;
- h8300smode = 0;
- h8300hmode = 0;
set_gdbarch_num_regs (gdbarch, 13);
set_gdbarch_num_pseudo_regs (gdbarch, 1);
set_gdbarch_ecoff_reg_to_regnum (gdbarch, h8300_dbg_reg_to_regnum);
@@ -1210,9 +1207,6 @@ h8300_gdbarch_init (struct gdbarch_info
break;
case bfd_mach_h8300h:
case bfd_mach_h8300hn:
- h8300sxmode = 0;
- h8300smode = 0;
- h8300hmode = 1;
set_gdbarch_num_regs (gdbarch, 13);
set_gdbarch_num_pseudo_regs (gdbarch, 1);
set_gdbarch_ecoff_reg_to_regnum (gdbarch, h8300_dbg_reg_to_regnum);
@@ -1222,13 +1216,11 @@ h8300_gdbarch_init (struct gdbarch_info
set_gdbarch_register_name (gdbarch, h8300_register_name);
if(info.bfd_arch_info->mach != bfd_mach_h8300hn)
{
- h8300_normal_mode = 0;
set_gdbarch_ptr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
set_gdbarch_addr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
}
else
{
- h8300_normal_mode = 1;
set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
set_gdbarch_addr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
}
@@ -1238,9 +1230,6 @@ h8300_gdbarch_init (struct gdbarch_info
break;
case bfd_mach_h8300s:
case bfd_mach_h8300sn:
- h8300sxmode = 0;
- h8300smode = 1;
- h8300hmode = 1;
set_gdbarch_num_regs (gdbarch, 16);
set_gdbarch_num_pseudo_regs (gdbarch, 2);
set_gdbarch_ecoff_reg_to_regnum (gdbarch, h8300s_dbg_reg_to_regnum);
@@ -1250,13 +1239,11 @@ h8300_gdbarch_init (struct gdbarch_info
set_gdbarch_register_name (gdbarch, h8300s_register_name);
if(info.bfd_arch_info->mach != bfd_mach_h8300sn)
{
- h8300_normal_mode = 0;
set_gdbarch_ptr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
set_gdbarch_addr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
}
else
{
- h8300_normal_mode = 1;
set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
set_gdbarch_addr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
}
@@ -1266,9 +1253,6 @@ h8300_gdbarch_init (struct gdbarch_info
break;
case bfd_mach_h8300sx:
case bfd_mach_h8300sxn:
- h8300sxmode = 1;
- h8300smode = 1;
- h8300hmode = 1;
set_gdbarch_num_regs (gdbarch, 18);
set_gdbarch_num_pseudo_regs (gdbarch, 2);
set_gdbarch_ecoff_reg_to_regnum (gdbarch, h8300s_dbg_reg_to_regnum);
@@ -1278,13 +1262,11 @@ h8300_gdbarch_init (struct gdbarch_info
set_gdbarch_register_name (gdbarch, h8300sx_register_name);
if(info.bfd_arch_info->mach != bfd_mach_h8300sxn)
{
- h8300_normal_mode = 0;
set_gdbarch_ptr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
set_gdbarch_addr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
}
else
{
- h8300_normal_mode = 1;
set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
set_gdbarch_addr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
}
@@ -1370,3 +1352,39 @@ _initialize_h8300_tdep (void)
{
register_gdbarch_init (bfd_arch_h8300, h8300_gdbarch_init);
}
+
+int
+is_h8300hmode (void)
+{
+ return gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sx
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sxn
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300s
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sn
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300h
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300hn;
+}
+
+int
+is_h8300smode (void)
+{
+ return gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sx
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sxn
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300s
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sn;
+}
+
+int
+is_h8300sxmode (void)
+{
+ return gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sx
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sxn;
+}
+
+int
+is_h8300_normal_mode (void)
+{
+ return gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sxn
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300sn
+ || gdbarch_bfd_arch_info (current_gdbarch)->mach == bfd_mach_h8300hn;
+}
+
diff -upr --strip-trailing-cr gdb+dejagnu-20031114.orig/gdb/remote-e7000.c gdb+dejagnu-20031114/gdb/remote-e7000.c
--- gdb+dejagnu-20031114.orig/gdb/remote-e7000.c 2003-10-10 12:43:10.000000000 +0530
+++ gdb+dejagnu-20031114/gdb/remote-e7000.c 2003-12-01 16:18:05.000000000 +0530
@@ -657,10 +657,6 @@ e7000_open (char *args, int from_tty)
}
serial_raw (e7000_desc);

-#ifdef GDB_TARGET_IS_H8300
- h8300hmode = 1;
-#endif
-
/* Start the remote connection; if error (0), discard this target.
In particular, if the user quits, be sure to discard it
(we'd be in an inconsistent state otherwise). */
@@ -909,15 +905,18 @@ e7000_fetch_registers (void)
wanted = want_sh3;
}
}
-#ifdef GDB_TARGET_IS_H8300
if (TARGET_ARCHITECTURE->arch == bfd_arch_h8300)
{
- if (h8300smode)
- wanted = want_h8300s;
- else
- wanted = want_h8300h;
+ wanted = want_h8300h;
+ switch (TARGET_ARCHITECTURE->mach)
+ {
+ case bfd_mach_h8300s:
+ case bfd_mach_h8300sn:
+ case bfd_mach_h8300sx:
+ case bfd_mach_h8300sxn:
+ wanted = want_h8300s;
+ }
}
-#endif

fetch_regs_from_dump (gch, wanted);

@@ -2046,15 +2045,18 @@ e7000_wait (ptid_t ptid, struct target_w
wanted_nopc = want_nopc_sh3;
}
}
-#ifdef GDB_TARGET_IS_H8300
if (TARGET_ARCHITECTURE->arch == bfd_arch_h8300)
{
- if (h8300smode)
- wanted_nopc = want_nopc_h8300s;
- else
- wanted_nopc = want_nopc_h8300h;
+ wanted_nopc = want_nopc_h8300h;
+ switch (TARGET_ARCHITECTURE->mach)
+ {
+ case bfd_mach_h8300s:
+ case bfd_mach_h8300sn:
+ case bfd_mach_h8300sx:
+ case bfd_mach_h8300sxn:
+ wanted_nopc = want_nopc_h8300s;
+ }
}
-#endif
fetch_regs_from_dump (gch, wanted_nopc);

/* And supply the extra ones the simulator uses */
diff -upr --strip-trailing-cr gdb+dejagnu-20031114.orig/sim/h8300/compile.c gdb+dejagnu-20031114/sim/h8300/compile.c
--- gdb+dejagnu-20031114.orig/sim/h8300/compile.c 2003-10-17 18:15:56.000000000 +0530
+++ gdb+dejagnu-20031114/sim/h8300/compile.c 2003-11-27 15:20:43.000000000 +0530
@@ -4930,6 +4930,8 @@ set_h8300h (unsigned long machine)
This function being replaced by a sim_open:ARGV configuration
option. */

+ h8300hmode = h8300smode = h8300sxmode = h8300_normal_mode = 0;
+
if (machine == bfd_mach_h8300sx || machine == bfd_mach_h8300sxn)
h8300sxmode = 1;

> -----Original Message-----
> From: Andrew Cagney [mailto:ac131313@redhat.com]
> Sent: Friday, November 14, 2003 8:19 PM
> To: Dhananjay R. Deshpande
> Cc: gdb-patches@sources.redhat.com
> Subject: Re: H8300 Patch - Fix GDB crash problem when object file of
> different H8 cpu is loaded
>
>
> > Hi,
> >
> > Currently if you are debugging one object and load another
> object of different
> > H8 CPU then the GDB crashes in h8300_register_name() saying
> "invalid register number". This is because in gdbarch_init we
> are returning existing gdbarch but
> > the globals h8300hmode, h8300smode, etc. are not set again
> corresponding to
> > newly loaded object.
> >
> > The patch below initializes these globals before returning
> existing gdbarch.
>
> Unfortunatly this isn't the right way to go. Because it is
> now possible
> for GDB can have more than one active architecture, code that rely on
> global state can no longer work. The globals:
>
> > + h8300_normal_mode = 0;
> > + h8300sxmode = 0;
> > + h8300smode = 0;
> > + h8300hmode = 0;
>
> will need to be eliminated. Looking at your patch it doesn't
> look too
> bad. Given h8300smode, I'd replace it with a function vis:
>
> int
> h8300smode (struct gdbarch *gdbarch)
> {
> return gdbarch_bfd_arch_info (current_gdbarch)->mach ==
> bfd_mach_h8300s
> }
>
> and called:
>
> if (h8300smode (current_gdbarch))
>
> Andrew
>
> > Changelog -
> >
> > 2003-11-14 Dhananjay Deshpande <dhananjayd@kpitcummins.com>
> > * gdb/h8300-tdep.c (h8300_gdbarch_init): Set
> globals before returning existing gdbarch
> >
> >
> ==============================================================
> ===================
> > --- gdb/h8300-tdep.c.old 2003-11-12 13:51:36.000000000 +0530
> > +++ gdb/h8300-tdep.c 2003-11-14 11:08:29.000000000 +0530
> > @@ -1176,6 +1176,53 @@ h8300_gdbarch_init (struct gdbarch_info
> > struct gdbarch_tdep *tdep = NULL;
> > struct gdbarch *gdbarch;
> >
> > + /* Set globals */
> > + switch (info.bfd_arch_info->mach)
> > + {
> > + case bfd_mach_h8300:
> > + h8300_normal_mode = 0;
> > + h8300sxmode = 0;
> > + h8300smode = 0;
> > + h8300hmode = 0;
> > + break;
> > + case bfd_mach_h8300h:
> > + h8300_normal_mode = 0;
> > + h8300sxmode = 0;
> > + h8300smode = 0;
> > + h8300hmode = 1;
> > + break;
> > + case bfd_mach_h8300hn:
> > + h8300_normal_mode = 1;
> > + h8300sxmode = 0;
> > + h8300smode = 0;
> > + h8300hmode = 1;
> > + break;
> > + case bfd_mach_h8300s:
> > + h8300_normal_mode = 0;
> > + h8300sxmode = 0;
> > + h8300smode = 1;
> > + h8300hmode = 1;
> > + break;
> > + case bfd_mach_h8300sn:
> > + h8300_normal_mode = 1;
> > + h8300sxmode = 0;
> > + h8300smode = 1;
> > + h8300hmode = 1;
> > + break;
> > + case bfd_mach_h8300sx:
> > + h8300_normal_mode = 0;
> > + h8300sxmode = 1;
> > + h8300smode = 1;
> > + h8300hmode = 1;
> > + break;
> > + case bfd_mach_h8300sxn:
> > + h8300_normal_mode = 1;
> > + h8300sxmode = 1;
> > + h8300smode = 1;
> > + h8300hmode = 1;
> > + break;
> > + }
> > +
> > arches = gdbarch_list_lookup_by_info (arches, &info);
> > if (arches != NULL)
> > return arches->gdbarch;
> > @@ -1192,9 +1239,6 @@ h8300_gdbarch_init (struct gdbarch_info
> > switch (info.bfd_arch_info->mach)
> > {
> > case bfd_mach_h8300:
> > - h8300sxmode = 0;
> > - h8300smode = 0;
> > - h8300hmode = 0;
> > set_gdbarch_num_regs (gdbarch, 13);
> > set_gdbarch_num_pseudo_regs (gdbarch, 1);
> > set_gdbarch_ecoff_reg_to_regnum (gdbarch,
> h8300_dbg_reg_to_regnum);
> > @@ -1210,9 +1254,6 @@ h8300_gdbarch_init (struct gdbarch_info
> > break;
> > case bfd_mach_h8300h:
> > case bfd_mach_h8300hn:
> > - h8300sxmode = 0;
> > - h8300smode = 0;
> > - h8300hmode = 1;
> > set_gdbarch_num_regs (gdbarch, 13);
> > set_gdbarch_num_pseudo_regs (gdbarch, 1);
> > set_gdbarch_ecoff_reg_to_regnum (gdbarch,
> h8300_dbg_reg_to_regnum);
> > @@ -1222,13 +1263,11 @@ h8300_gdbarch_init (struct gdbarch_info
> > set_gdbarch_register_name (gdbarch, h8300_register_name);
> > if(info.bfd_arch_info->mach != bfd_mach_h8300hn)
> > {
> > - h8300_normal_mode = 0;
> > set_gdbarch_ptr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > set_gdbarch_addr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > }
> > else
> > {
> > - h8300_normal_mode = 1;
> > set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> > set_gdbarch_addr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> > }
> > @@ -1238,9 +1277,6 @@ h8300_gdbarch_init (struct gdbarch_info
> > break;
> > case bfd_mach_h8300s:
> > case bfd_mach_h8300sn:
> > - h8300sxmode = 0;
> > - h8300smode = 1;
> > - h8300hmode = 1;
> > set_gdbarch_num_regs (gdbarch, 16);
> > set_gdbarch_num_pseudo_regs (gdbarch, 2);
> > set_gdbarch_ecoff_reg_to_regnum (gdbarch,
> h8300s_dbg_reg_to_regnum);
> > @@ -1250,13 +1286,11 @@ h8300_gdbarch_init (struct gdbarch_info
> > set_gdbarch_register_name (gdbarch, h8300s_register_name);
> > if(info.bfd_arch_info->mach != bfd_mach_h8300sn)
> > {
> > - h8300_normal_mode = 0;
> > set_gdbarch_ptr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > set_gdbarch_addr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > }
> > else
> > {
> > - h8300_normal_mode = 1;
> > set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> > set_gdbarch_addr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> > }
> > @@ -1266,9 +1300,6 @@ h8300_gdbarch_init (struct gdbarch_info
> > break;
> > case bfd_mach_h8300sx:
> > case bfd_mach_h8300sxn:
> > - h8300sxmode = 1;
> > - h8300smode = 1;
> > - h8300hmode = 1;
> > set_gdbarch_num_regs (gdbarch, 18);
> > set_gdbarch_num_pseudo_regs (gdbarch, 2);
> > set_gdbarch_ecoff_reg_to_regnum (gdbarch,
> h8300s_dbg_reg_to_regnum);
> > @@ -1278,13 +1309,11 @@ h8300_gdbarch_init (struct gdbarch_info
> > set_gdbarch_register_name (gdbarch, h8300sx_register_name);
> > if(info.bfd_arch_info->mach != bfd_mach_h8300sxn)
> > {
> > - h8300_normal_mode = 0;
> > set_gdbarch_ptr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > set_gdbarch_addr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
> > }
> > else
> > {
> > - h8300_normal_mode = 1;
> > set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> > set_gdbarch_addr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> > }
> >
> ==============================================================
> ===================
> >
>
>
>