[PATCH RFA/RFC] Short pointer support

[PATCH RFA/RFC] Short pointer support

[Kevin Buettner]

The patch below implements the generic portions for "short" pointer
support.

To understand why this support might be needed, consider a 16-bit
architecture that's been extended to 32-bits.  The toolchain for the
32-bit architecture uses 32-bit pointers, but it's occassionally
useful to be able to link against and use the data types of legacy
16-bit libraries.  In this case, the 32-bit pointers are "normal" and
the 16-bit pointers are "short".

I'm a little bit uneasy about calling them "short" pointers, but I'm
even more uneasy about what might happen if these differently sized
pointers should end up being larger than normal pointers.  I.e, would
such pointers still be guaranteed to fit in a CORE_ADDR?  Plus, for
the work I'm doing, the pointers in question actually do end up
occupying fewer bits than normal pointers, so that's how I've come
to think of them.  I'd be happy to consider suggestions for a better
name.

The patch below introduces a new gdbarch method (variable) called
SHORTPTR_QUALIFIER_NAME.  A platform that needs to support an
alternate sized pointer might set this variable as follows in
its tdep.c file:

    set_gdbarch_shortptr_qualifier_name (gdbarch, "short");

Then, assuming that the compiler has generated appropriate debug
information, you could do something similar to the following in gdb:

    (gdb) ptype short_ptr_var
    type = int * @short

Note that this new support leverages the existing address space
modifier support that was introduced late last year.  One of the
concerns that I have about it is that it only handles the case
where there are at most two sizes of pointers.  It would be possible
to handle the more general case, but the patch would be larger and
certain interfaces would have to change.  At this point, I'm not
convinced it's worth it.  After all, how many environments need even
two different pointer sizes?

I need approval from either Jim or Elena for the dwarf2read.c
modifications, and Eli for the doc addition.

In gdb:

	* dwarf2read.c (read_tag_pointer_type): Add short pointer support.
	* gdbarch.sh (SHORTPTR_QUALIFIER_NAME): New method.
	* gdbarch.h, gdbarch.c: Regenerate.
	* gdbtypes.c (address_space_name_to_int, address_space_int_to_name)
	(recursive_dump_type): Add short pointer support.
	* gdbtypes.h (TYPE_FLAG_SHORTPTR, TYPE_SHORTPTR): New defines.

In gdb/doc:

	* gdbint.texinfo (Target Conditionals): Document
	SHORTPTR_QUALIFIER_NAME.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.68
diff -u -p -r1.68 dwarf2read.c
--- dwarf2read.c	9 Oct 2002 04:43:49 -0000	1.68
+++ dwarf2read.c	11 Oct 2002 20:10:49 -0000
@@ -2936,7 +2936,21 @@ read_tag_pointer_type (struct die_info *
   attr = dwarf_attr (die, DW_AT_byte_size);
   if (attr)
     {
-      TYPE_LENGTH (type) = DW_UNSND (attr);
+      int size = DW_UNSND (attr);
+
+      /* If the pointer size is different than the default, create a type
+         variant marked as such and set the length accordingly.  */
+      if (TYPE_LENGTH (type) != size)
+	{
+	  type = make_type_with_address_space (type, TYPE_FLAG_SHORTPTR);
+	  /* Note: What if there's more than one size variant?  If that
+	     should happen, we'll end up just using the last size for
+	     all such variants.  To make this work right, we would need to
+	     pass the pointer size to make_type_with_address_space.
+	     However, if we do that, how should we represent the different
+	     sizes when printing out the types.  */
+	  TYPE_LENGTH (type) = DW_UNSND (attr);
+	}
     }
   else
     {
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.164
diff -u -p -r1.164 gdbarch.sh
--- gdbarch.sh	9 Oct 2002 11:59:54 -0000	1.164
+++ gdbarch.sh	11 Oct 2002 20:10:51 -0000
@@ -664,6 +664,7 @@ f:2:COFF_MAKE_MSYMBOL_SPECIAL:void:coff_
 v::NAME_OF_MALLOC:const char *:name_of_malloc::::"malloc":"malloc"::0
 v::CANNOT_STEP_BREAKPOINT:int:cannot_step_breakpoint::::0:0::0
 v::HAVE_NONSTEPPABLE_WATCHPOINT:int:have_nonsteppable_watchpoint::::0:0::0
+v::SHORTPTR_QUALIFIER_NAME:char *:shortptr_qualifier_name::::0:0::0
 EOF
 }
 
Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.59
diff -u -p -r1.59 gdbtypes.c
--- gdbtypes.c	2 Oct 2002 22:01:53 -0000	1.59
+++ gdbtypes.c	11 Oct 2002 20:10:51 -0000
@@ -402,6 +402,9 @@ address_space_name_to_int (char *space_i
     return TYPE_FLAG_CODE_SPACE;
   else if (!strcmp (space_identifier, "data"))
     return TYPE_FLAG_DATA_SPACE;
+  else if (SHORTPTR_QUALIFIER_NAME != NULL 
+           && strcmp (space_identifier, SHORTPTR_QUALIFIER_NAME) == 0)
+    return TYPE_FLAG_SHORTPTR;
   else
     error ("Unknown address space specifier: \"%s\"", space_identifier);
 }
@@ -416,6 +419,8 @@ address_space_int_to_name (int space_fla
     return "code";
   else if (space_flag & TYPE_FLAG_DATA_SPACE)
     return "data";
+  else if ((space_flag & TYPE_FLAG_SHORTPTR) && SHORTPTR_QUALIFIER_NAME != NULL)
+    return SHORTPTR_QUALIFIER_NAME;
   else
     return NULL;
 }
@@ -3139,6 +3144,10 @@ recursive_dump_type (struct type *type, 
   if (TYPE_DATA_SPACE (type))
     {
       puts_filtered (" TYPE_FLAG_DATA_SPACE");
+    }
+  if (TYPE_SHORTPTR (type))
+    {
+      puts_filtered (" TYPE_FLAG_SHORTPTR");
     }
   puts_filtered ("\n");
   printfi_filtered (spaces, "flags 0x%x", TYPE_FLAGS (type));
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.36
diff -u -p -r1.36 gdbtypes.h
--- gdbtypes.h	14 Sep 2002 02:09:39 -0000	1.36
+++ gdbtypes.h	11 Oct 2002 20:10:52 -0000
@@ -253,6 +253,12 @@ enum type_code
 #define TYPE_FLAG_VECTOR	(1 << 12)
 #define TYPE_VECTOR(t)		(TYPE_FLAGS (t) & TYPE_FLAG_VECTOR)
 
+/* Short pointers.  Some environments provide for pointers whose size
+   is smaller than that of a normal pointer.  These short pointer types
+   will reside on struct type's variant chain.  */
+#define TYPE_FLAG_SHORTPTR	(1 << 13)
+#define TYPE_SHORTPTR(t)	(TYPE_INSTANCE_FLAGS(t) & TYPE_FLAG_SHORTPTR)
+
 struct main_type
 {
   /* Code for kind of type */
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.103
diff -u -p -r1.103 gdbint.texinfo
--- doc/gdbint.texinfo	3 Oct 2002 22:30:01 -0000	1.103
+++ doc/gdbint.texinfo	11 Oct 2002 20:10:55 -0000
@@ -3677,6 +3677,12 @@ defined, no conversion will be done.
 @c OBSOLETE @findex SHIFT_INST_REGS
 @c OBSOLETE (Only used for m88k targets.)
 
+@item SHORTPTR_QUALIFIER_NAME
+@findex SHORTPTR_QUALIFIER_NAME
+Some environments have support for different sized pointers.  Define this
+to be the name of the qualifier used to indicate one of these different
+sized pointers in a type expression.
+
 @item SKIP_PERMANENT_BREAKPOINT
 @findex SKIP_PERMANENT_BREAKPOINT
 Advance the inferior's PC past a permanent breakpoint.  @value{GDBN} normally

Re: [PATCH RFA/RFC] Short pointer support

[Eli Zaretskii]

> Date: Fri, 11 Oct 2002 14:26:23 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> In gdb/doc:
> 
> 	* gdbint.texinfo (Target Conditionals): Document
> 	SHORTPTR_QUALIFIER_NAME.

This part is approved, but I wonder whether an example would clarify
the intent even more here.

Re: [PATCH RFA/RFC] Short pointer support

[Jim Blandy]

I approve of the the dwarf2read.c part of this change.

It would be nice if we actually checked the pointer size against the
correct size of short pointers on the target.  There's something that
feels a little flimsy about the way the patch just marks any pointer
with a non-standard size with TYPE_FLAG_SHORTPTR.  Wouldn't it be more
robust to have an arch method that takes a non-standard pointer size,
and returns the appropriate type flags for that, or zero if it's
something it doesn't recognize?  Or whatever.

Kevin Buettner <kevinb@redhat.com> writes:

> The patch below implements the generic portions for "short" pointer
> support.
> 
> To understand why this support might be needed, consider a 16-bit
> architecture that's been extended to 32-bits.  The toolchain for the
> 32-bit architecture uses 32-bit pointers, but it's occassionally
> useful to be able to link against and use the data types of legacy
> 16-bit libraries.  In this case, the 32-bit pointers are "normal" and
> the 16-bit pointers are "short".
> 
> I'm a little bit uneasy about calling them "short" pointers, but I'm
> even more uneasy about what might happen if these differently sized
> pointers should end up being larger than normal pointers.  I.e, would
> such pointers still be guaranteed to fit in a CORE_ADDR?  Plus, for
> the work I'm doing, the pointers in question actually do end up
> occupying fewer bits than normal pointers, so that's how I've come
> to think of them.  I'd be happy to consider suggestions for a better
> name.
> 
> The patch below introduces a new gdbarch method (variable) called
> SHORTPTR_QUALIFIER_NAME.  A platform that needs to support an
> alternate sized pointer might set this variable as follows in
> its tdep.c file:
> 
>     set_gdbarch_shortptr_qualifier_name (gdbarch, "short");
> 
> Then, assuming that the compiler has generated appropriate debug
> information, you could do something similar to the following in gdb:
> 
>     (gdb) ptype short_ptr_var
>     type = int * @short
> 
> Note that this new support leverages the existing address space
> modifier support that was introduced late last year.  One of the
> concerns that I have about it is that it only handles the case
> where there are at most two sizes of pointers.  It would be possible
> to handle the more general case, but the patch would be larger and
> certain interfaces would have to change.  At this point, I'm not
> convinced it's worth it.  After all, how many environments need even
> two different pointer sizes?
> 
> I need approval from either Jim or Elena for the dwarf2read.c
> modifications, and Eli for the doc addition.
> 
> In gdb:
> 
> 	* dwarf2read.c (read_tag_pointer_type): Add short pointer support.
> 	* gdbarch.sh (SHORTPTR_QUALIFIER_NAME): New method.
> 	* gdbarch.h, gdbarch.c: Regenerate.
> 	* gdbtypes.c (address_space_name_to_int, address_space_int_to_name)
> 	(recursive_dump_type): Add short pointer support.
> 	* gdbtypes.h (TYPE_FLAG_SHORTPTR, TYPE_SHORTPTR): New defines.
> 
> In gdb/doc:
> 
> 	* gdbint.texinfo (Target Conditionals): Document
> 	SHORTPTR_QUALIFIER_NAME.
> 
> Index: dwarf2read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 dwarf2read.c
> --- dwarf2read.c	9 Oct 2002 04:43:49 -0000	1.68
> +++ dwarf2read.c	11 Oct 2002 20:10:49 -0000
> @@ -2936,7 +2936,21 @@ read_tag_pointer_type (struct die_info *
>    attr = dwarf_attr (die, DW_AT_byte_size);
>    if (attr)
>      {
> -      TYPE_LENGTH (type) = DW_UNSND (attr);
> +      int size = DW_UNSND (attr);
> +
> +      /* If the pointer size is different than the default, create a type
> +         variant marked as such and set the length accordingly.  */
> +      if (TYPE_LENGTH (type) != size)
> +	{
> +	  type = make_type_with_address_space (type, TYPE_FLAG_SHORTPTR);
> +	  /* Note: What if there's more than one size variant?  If that
> +	     should happen, we'll end up just using the last size for
> +	     all such variants.  To make this work right, we would need to
> +	     pass the pointer size to make_type_with_address_space.
> +	     However, if we do that, how should we represent the different
> +	     sizes when printing out the types.  */
> +	  TYPE_LENGTH (type) = DW_UNSND (attr);
> +	}
>      }
>    else
>      {
> Index: gdbarch.sh
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbarch.sh,v
> retrieving revision 1.164
> diff -u -p -r1.164 gdbarch.sh
> --- gdbarch.sh	9 Oct 2002 11:59:54 -0000	1.164
> +++ gdbarch.sh	11 Oct 2002 20:10:51 -0000
> @@ -664,6 +664,7 @@ f:2:COFF_MAKE_MSYMBOL_SPECIAL:void:coff_
>  v::NAME_OF_MALLOC:const char *:name_of_malloc::::"malloc":"malloc"::0
>  v::CANNOT_STEP_BREAKPOINT:int:cannot_step_breakpoint::::0:0::0
>  v::HAVE_NONSTEPPABLE_WATCHPOINT:int:have_nonsteppable_watchpoint::::0:0::0
> +v::SHORTPTR_QUALIFIER_NAME:char *:shortptr_qualifier_name::::0:0::0
>  EOF
>  }
>  
> Index: gdbtypes.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtypes.c,v
> retrieving revision 1.59
> diff -u -p -r1.59 gdbtypes.c
> --- gdbtypes.c	2 Oct 2002 22:01:53 -0000	1.59
> +++ gdbtypes.c	11 Oct 2002 20:10:51 -0000
> @@ -402,6 +402,9 @@ address_space_name_to_int (char *space_i
>      return TYPE_FLAG_CODE_SPACE;
>    else if (!strcmp (space_identifier, "data"))
>      return TYPE_FLAG_DATA_SPACE;
> +  else if (SHORTPTR_QUALIFIER_NAME != NULL 
> +           && strcmp (space_identifier, SHORTPTR_QUALIFIER_NAME) == 0)
> +    return TYPE_FLAG_SHORTPTR;
>    else
>      error ("Unknown address space specifier: \"%s\"", space_identifier);
>  }
> @@ -416,6 +419,8 @@ address_space_int_to_name (int space_fla
>      return "code";
>    else if (space_flag & TYPE_FLAG_DATA_SPACE)
>      return "data";
> +  else if ((space_flag & TYPE_FLAG_SHORTPTR) && SHORTPTR_QUALIFIER_NAME != NULL)
> +    return SHORTPTR_QUALIFIER_NAME;
>    else
>      return NULL;
>  }
> @@ -3139,6 +3144,10 @@ recursive_dump_type (struct type *type, 
>    if (TYPE_DATA_SPACE (type))
>      {
>        puts_filtered (" TYPE_FLAG_DATA_SPACE");
> +    }
> +  if (TYPE_SHORTPTR (type))
> +    {
> +      puts_filtered (" TYPE_FLAG_SHORTPTR");
>      }
>    puts_filtered ("\n");
>    printfi_filtered (spaces, "flags 0x%x", TYPE_FLAGS (type));
> Index: gdbtypes.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtypes.h,v
> retrieving revision 1.36
> diff -u -p -r1.36 gdbtypes.h
> --- gdbtypes.h	14 Sep 2002 02:09:39 -0000	1.36
> +++ gdbtypes.h	11 Oct 2002 20:10:52 -0000
> @@ -253,6 +253,12 @@ enum type_code
>  #define TYPE_FLAG_VECTOR	(1 << 12)
>  #define TYPE_VECTOR(t)		(TYPE_FLAGS (t) & TYPE_FLAG_VECTOR)
>  
> +/* Short pointers.  Some environments provide for pointers whose size
> +   is smaller than that of a normal pointer.  These short pointer types
> +   will reside on struct type's variant chain.  */
> +#define TYPE_FLAG_SHORTPTR	(1 << 13)
> +#define TYPE_SHORTPTR(t)	(TYPE_INSTANCE_FLAGS(t) & TYPE_FLAG_SHORTPTR)
> +
>  struct main_type
>  {
>    /* Code for kind of type */
> Index: doc/gdbint.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
> retrieving revision 1.103
> diff -u -p -r1.103 gdbint.texinfo
> --- doc/gdbint.texinfo	3 Oct 2002 22:30:01 -0000	1.103
> +++ doc/gdbint.texinfo	11 Oct 2002 20:10:55 -0000
> @@ -3677,6 +3677,12 @@ defined, no conversion will be done.
>  @c OBSOLETE @findex SHIFT_INST_REGS
>  @c OBSOLETE (Only used for m88k targets.)
>  
> +@item SHORTPTR_QUALIFIER_NAME
> +@findex SHORTPTR_QUALIFIER_NAME
> +Some environments have support for different sized pointers.  Define this
> +to be the name of the qualifier used to indicate one of these different
> +sized pointers in a type expression.
> +
>  @item SKIP_PERMANENT_BREAKPOINT
>  @findex SKIP_PERMANENT_BREAKPOINT
>  Advance the inferior's PC past a permanent breakpoint.  @value{GDBN} normally

Re: [PATCH RFA/RFC] Short pointer support

[Kevin Buettner]

On Oct 14,  3:45pm, Jim Blandy wrote:

> It would be nice if we actually checked the pointer size against the
> correct size of short pointers on the target.  There's something that
> feels a little flimsy about the way the patch just marks any pointer
> with a non-standard size with TYPE_FLAG_SHORTPTR.  Wouldn't it be more
> robust to have an arch method that takes a non-standard pointer size,
> and returns the appropriate type flags for that, or zero if it's
> something it doesn't recognize?  Or whatever.

Okay, this patch is withdrawn.  I'm in the process of redoing it so
that it's more general and more robust.

Kevin