Now I know why we used to swap builtin_type_void.

Now I know why we used to swap builtin_type_void.

[Daniel Jacobowitz]

(top-gdb) p *builtin_type_void_data_ptr
$24 = {pointer_type = 0x0, reference_type = 0x0, chain = 0x7b50d0,
  instance_flags = 0, length = 8, main_type = 0x7b5100}
(top-gdb) p current_gdbarch.ptr_bit
$25 = 32

Because we didn't swap out "void", we followed the cached pointer link
in builtin_type_void when we tried to create a pointer to void.  My
initial default gdbarch was 64-bit, because I built a 64-bit GDB
binary.  So the cached pointer type is 64-bit.

This is a mess.  I think we may need to revert the builtin_type_void
patch unless you have a better idea.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Now I know why we used to swap builtin_type_void.

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> (top-gdb) p *builtin_type_void_data_ptr
> $24 = {pointer_type = 0x0, reference_type = 0x0, chain = 0x7b50d0,
>   instance_flags = 0, length = 8, main_type = 0x7b5100}
> (top-gdb) p current_gdbarch.ptr_bit
> $25 = 32
>
> Because we didn't swap out "void", we followed the cached pointer link
> in builtin_type_void when we tried to create a pointer to void.  My
> initial default gdbarch was 64-bit, because I built a 64-bit GDB
> binary.  So the cached pointer type is 64-bit.
>
> This is a mess.  I think we may need to revert the builtin_type_void
> patch unless you have a better idea.

I see --- because types point to their pointer types, if we need to
swap pointer types, we must also swap target types.

Why doesn't the same logic apply to the other types created in
_initialize_gdbtypes?  I believe they're never referred to by debug
information, and the user has no way to refer to them from the command
line, so that keeps us out of trouble in those cases.  But if some
architecture-specific code ever calls lookup_pointer_type on them,
won't we have the same problem you've discovered above with void?

I'll revert the patch.  It was just a cleanup; I don't actually need
it for anything.

Re: Now I know why we used to swap builtin_type_void.

[Daniel Jacobowitz]

On Wed, Dec 06, 2006 at 12:05:11PM -0800, Jim Blandy wrote:
> Why doesn't the same logic apply to the other types created in
> _initialize_gdbtypes?  I believe they're never referred to by debug
> information, and the user has no way to refer to them from the command
> line, so that keeps us out of trouble in those cases.  But if some
> architecture-specific code ever calls lookup_pointer_type on them,
> won't we have the same problem you've discovered above with void?

Exactly.  I think we'd have to move TYPE_POINTER_TYPE out of the type
and into e.g. an architecture hash table to fix this.  Worth it? 
Don't know.

> I'll revert the patch.  It was just a cleanup; I don't actually need
> it for anything.

Thanks.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Now I know why we used to swap builtin_type_void.

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> (top-gdb) p *builtin_type_void_data_ptr
> $24 = {pointer_type = 0x0, reference_type = 0x0, chain = 0x7b50d0,
>   instance_flags = 0, length = 8, main_type = 0x7b5100}
> (top-gdb) p current_gdbarch.ptr_bit
> $25 = 32
>
> Because we didn't swap out "void", we followed the cached pointer link
> in builtin_type_void when we tried to create a pointer to void.  My
> initial default gdbarch was 64-bit, because I built a 64-bit GDB
> binary.  So the cached pointer type is 64-bit.
>
> This is a mess.  I think we may need to revert the builtin_type_void
> patch unless you have a better idea.

I've committed the following:

gdb/ChangeLog:
2006-12-06  Jim Blandy  <jimb@codesourcery.com>

	* gdbtypes.c: Revert 2006-12-05 change, and explain why.

Index: gdb/gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.107
diff -u -r1.107 gdbtypes.c
--- gdb/gdbtypes.c	6 Dec 2006 00:57:04 -0000	1.107
+++ gdb/gdbtypes.c	6 Dec 2006 20:14:57 -0000
@@ -3297,6 +3297,10 @@
 static void
 build_gdbtypes (void)
 {
+  builtin_type_void =
+    init_type (TYPE_CODE_VOID, 1,
+	       0,
+	       "void", (struct objfile *) NULL);
   builtin_type_char =
     init_type (TYPE_CODE_INT, TARGET_CHAR_BIT / TARGET_CHAR_BIT,
 	       (TYPE_FLAG_NOSIGN
@@ -3646,10 +3650,9 @@
 {
   struct cmd_list_element *c;
 
-  builtin_type_void =
-    init_type (TYPE_CODE_VOID, 1,
-	       0,
-	       "void", (struct objfile *) NULL);
+  /* FIXME: Why don't the following types need to be arch-swapped?
+     See the comment at the top of the calls to
+     DEPRECATED_REGISTER_GDBARCH_SWAP below.  */
   builtin_type_int0 =
     init_type (TYPE_CODE_INT, 0 / 8,
 	       0,
@@ -3701,7 +3704,14 @@
 
   /* FIXME - For the moment, handle types by swapping them in and out.
      Should be using the per-architecture data-pointer and a large
-     struct. */
+     struct. 
+
+     Note that any type T that we might create a 'T *' type for must
+     be arch-swapped: we cache a type's 'T *' type in the pointer_type
+     field, so if we change architectures but don't swap T, then
+     lookup_pointer_type will start handing out pointer types made for
+     a different architecture.  */
+  DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_void);
   DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_char);
   DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_short);
   DEPRECATED_REGISTER_GDBARCH_SWAP (builtin_type_int);
@@ -3744,7 +3754,8 @@
   deprecated_register_gdbarch_swap (NULL, 0, build_gdbtypes);
 
   /* Note: These types do not need to be swapped - they are target
-     neutral.  */
+     neutral.  FIXME: Are you sure?  See the comment above the calls
+     to DEPRECATED_REGISTER_GDBARCH_SWAP above.  */
   builtin_type_ieee_single_big =
     init_type (TYPE_CODE_FLT, floatformat_ieee_single_big.totalsize / 8,
 	       0, "builtin_type_ieee_single_big", NULL);