[RFA] rs6000-tdep.c: arch switching buglet

[RFA] rs6000-tdep.c: arch switching buglet

[Elena Zannoni]

I found an odd bug on the ppc.

For this target, the list of arches in bfd includes
several that are not rupported in gdb, for instance powerpc:630.

If the user says:

set architecture powerpc:630 

What should happen?

set_architecture() calls gdbarch_update_p, which should fail (right?),
and set_architecture should print an error message.

Instead, consider the rs6000 code in rs6000_gdbarch_init():

  /* Choose variant. */
  v = find_variant_by_arch (arch, mach);
  if (!v)
    v = find_variant_by_name (power ? "power" : "powerpc");

This code will pick a different architecture, in this case
powerpc:common, but gdb/multiarch doesn't know, and prints that the
architecture has been successfully set to powerpc:630.

This code was put in place before the multiarch framework, and it has
become obsolete. So, how about the following:


2002-02-12  Elena Zannoni  <ezannoni@redhat.com>

	* rs6000-tdep.c (rs6000_gdbarch_init): Don't call
	find_variant_by_name, because it confuses the multiarch
	framework. Return NULL if there isn't an architecture with the
	user supplied name, instead of forcing a different one without
	recording the change with the multiarch machinery.
	(find_variant_by_name): Delete.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/uberbaum/gdb/rs6000-tdep.c,v
retrieving revision 1.37
diff -u -p -r1.37 rs6000-tdep.c
--- rs6000-tdep.c	2002/01/29 03:08:25	1.37
+++ rs6000-tdep.c	2002/02/12 15:45:26
@@ -2262,21 +2282,6 @@ static const struct variant variants[] =
 
 #undef num_registers
 
-/* Look up the variant named NAME in the `variants' table.  Return a
-   pointer to the struct variant, or null if we couldn't find it.  */
-
-static const struct variant *
-find_variant_by_name (char *name)
-{
-  const struct variant *v;
-
-  for (v = variants; v->name; v++)
-    if (!strcmp (name, v->name))
-      return v;
-
-  return NULL;
-}
-
 /* Return the variant corresponding to architecture ARCH and machine number
    MACH.  If no such variant exists, return null. */
 
@@ -2470,8 +2475,10 @@ rs6000_gdbarch_init (struct gdbarch_info
 
   /* Choose variant. */
   v = find_variant_by_arch (arch, mach);
+
   if (!v)
-    v = find_variant_by_name (power ? "power" : "powerpc");
+    return NULL;
+
   tdep->regs = v->regs;
 
   tdep->ppc_gp0_regnum = 0;

Re: [RFA] rs6000-tdep.c: arch switching buglet

[Kevin Buettner]

On Feb 12, 10:47am, Elena Zannoni wrote:

> I found an odd bug on the ppc.
> 
> For this target, the list of arches in bfd includes
> several that are not rupported in gdb, for instance powerpc:630.
> 
> If the user says:
> 
> set architecture powerpc:630 
> 
> What should happen?
> 
> set_architecture() calls gdbarch_update_p, which should fail (right?),
> and set_architecture should print an error message.
> 
> Instead, consider the rs6000 code in rs6000_gdbarch_init():
> 
>   /* Choose variant. */
>   v = find_variant_by_arch (arch, mach);
>   if (!v)
>     v = find_variant_by_name (power ? "power" : "powerpc");
> 
> This code will pick a different architecture, in this case
> powerpc:common, but gdb/multiarch doesn't know, and prints that the
> architecture has been successfully set to powerpc:630.
> 
> This code was put in place before the multiarch framework, and it has
> become obsolete. So, how about the following:
> 
> 
> 2002-02-12  Elena Zannoni  <ezannoni@redhat.com>
> 
> 	* rs6000-tdep.c (rs6000_gdbarch_init): Don't call
> 	find_variant_by_name, because it confuses the multiarch
> 	framework. Return NULL if there isn't an architecture with the
> 	user supplied name, instead of forcing a different one without
> 	recording the change with the multiarch machinery.
> 	(find_variant_by_name): Delete.

It took me a while to convince myself that find_variant_by_name()
wouldn't be useful in the future.  But, you're right, this code should
go.  Please commit it.

Thanks,

Kevin

Re: [RFA] rs6000-tdep.c: arch switching buglet

[Elena Zannoni]

Committed, thanks

Elena


Kevin Buettner writes:
 > On Feb 12, 10:47am, Elena Zannoni wrote:
 > 
 > > I found an odd bug on the ppc.
 > > 
 > > For this target, the list of arches in bfd includes
 > > several that are not rupported in gdb, for instance powerpc:630.
 > > 
 > > If the user says:
 > > 
 > > set architecture powerpc:630 
 > > 
 > > What should happen?
 > > 
 > > set_architecture() calls gdbarch_update_p, which should fail (right?),
 > > and set_architecture should print an error message.
 > > 
 > > Instead, consider the rs6000 code in rs6000_gdbarch_init():
 > > 
 > >   /* Choose variant. */
 > >   v = find_variant_by_arch (arch, mach);
 > >   if (!v)
 > >     v = find_variant_by_name (power ? "power" : "powerpc");
 > > 
 > > This code will pick a different architecture, in this case
 > > powerpc:common, but gdb/multiarch doesn't know, and prints that the
 > > architecture has been successfully set to powerpc:630.
 > > 
 > > This code was put in place before the multiarch framework, and it has
 > > become obsolete. So, how about the following:
 > > 
 > > 
 > > 2002-02-12  Elena Zannoni  <ezannoni@redhat.com>
 > > 
 > > 	* rs6000-tdep.c (rs6000_gdbarch_init): Don't call
 > > 	find_variant_by_name, because it confuses the multiarch
 > > 	framework. Return NULL if there isn't an architecture with the
 > > 	user supplied name, instead of forcing a different one without
 > > 	recording the change with the multiarch machinery.
 > > 	(find_variant_by_name): Delete.
 > 
 > It took me a while to convince myself that find_variant_by_name()
 > wouldn't be useful in the future.  But, you're right, this code should
 > go.  Please commit it.
 > 
 > Thanks,
 > 
 > Kevin