[PATCH, FT32] Proper support for address <-> pointer conversions

[PATCH, FT32] Proper support for address <-> pointer conversions

[James Bowman]

FT32 is a Harvard architecture with two address spaces -- RAM and flash.
This patch properly implements the address to pointer conversion method,
and its reverse. There are some other small fixes to handle address
spaces.

OK to apply?

2015-09-21  James Bowman  <james.bowman@ftdichip.com>

	* ft32-tdep.c: Properly support address <-> pointer conversions,
	so that FT32's address spaces (RAM and flash) work correctly
        when dereferencing pointers in gdb.
	* ft32-tdep.h: Add a type for an address space 1 pointer.


diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 2e5deca..d6e8c6d 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -117,7 +117,7 @@ static struct type *
 ft32_register_type (struct gdbarch *gdbarch, int reg_nr)
 {
   if (reg_nr == FT32_PC_REGNUM)
-    return builtin_type (gdbarch)->builtin_func_ptr;
+    return gdbarch_tdep (gdbarch)->pc_type;
   else if (reg_nr == FT32_SP_REGNUM || reg_nr == FT32_FP_REGNUM)
     return builtin_type (gdbarch)->builtin_data_ptr;
   else
@@ -270,6 +270,88 @@ ft32_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
   return pc;
 }
 
+/* Convert from address to pointer and vice-versa.  */
+
+static void
+ft32_address_to_pointer (struct gdbarch *gdbarch,
+			 struct type *type, gdb_byte *buf, CORE_ADDR addr)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  store_unsigned_integer (buf, TYPE_LENGTH (type), byte_order,
+			  addr);
+}
+
+static CORE_ADDR
+ft32_pointer_to_address (struct gdbarch *gdbarch,
+			 struct type *type, const gdb_byte *buf)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  CORE_ADDR addr
+    = extract_unsigned_integer (buf, TYPE_LENGTH (type), byte_order);
+  if (TYPE_ADDRESS_CLASS_1 (type))
+    return addr;
+  else
+    return addr | RAM_BIAS;
+}
+
+static CORE_ADDR
+ft32_integer_to_address (struct gdbarch *gdbarch,
+			 struct type *type, const gdb_byte *buf)
+{
+  ULONGEST addr = unpack_long (type, buf);
+
+  return addr;
+}
+
+/* Implementation of `address_class_type_flags' gdbarch method.
+
+   This method maps DW_AT_address_class attributes to a
+   type_instance_flag_value.  */
+
+static int
+ft32_address_class_type_flags (int byte_size, int dwarf2_addr_class)
+{
+  /* The value 1 of the DW_AT_address_class attribute corresponds to the
+     __flash__ qualifier, meaning pointer to data in FT32 program memory.
+   */
+  if (dwarf2_addr_class == 1)
+    return TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+  return 0;
+}
+
+/* Implementation of `address_class_type_flags_to_name' gdbarch method.
+
+   Convert a type_instance_flag_value to an address space qualifier.  */
+
+static const char*
+ft32_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
+{
+  if (type_flags & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1)
+    return "flash";
+  else
+    return NULL;
+}
+
+/* Implementation of `address_class_name_to_type_flags' gdbarch method.
+
+   Convert an address space qualifier to a type_instance_flag_value.  */
+
+static int
+ft32_address_class_name_to_type_flags (struct gdbarch *gdbarch,
+				       const char* name,
+				       int *type_flags_ptr)
+{
+  if (strcmp (name, "flash") == 0)
+    {
+      *type_flags_ptr = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+      return 1;
+    }
+  else
+    return 0;
+}
+
+
 /* Implement the "read_pc" gdbarch method.  */
 
 static CORE_ADDR
@@ -498,6 +580,15 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep = XNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
+  /* Create a type for PC.  We can't use builtin types here, as they may not
+     be defined.  */
+  tdep->void_type = arch_type (gdbarch, TYPE_CODE_VOID, 1, "void");
+  tdep->func_void_type = make_function_type (tdep->void_type, NULL);
+  tdep->pc_type = arch_type (gdbarch, TYPE_CODE_PTR, 4, NULL);
+  TYPE_TARGET_TYPE (tdep->pc_type) = tdep->func_void_type;
+  TYPE_UNSIGNED (tdep->pc_type) = 1;
+  TYPE_INSTANCE_FLAGS (tdep->pc_type) |= TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+
   set_gdbarch_read_pc (gdbarch, ft32_read_pc);
   set_gdbarch_write_pc (gdbarch, ft32_write_pc);
   set_gdbarch_unwind_sp (gdbarch, ft32_unwind_sp);
@@ -510,6 +601,10 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   set_gdbarch_return_value (gdbarch, ft32_return_value);
 
+  set_gdbarch_address_to_pointer (gdbarch, ft32_address_to_pointer);
+  set_gdbarch_pointer_to_address (gdbarch, ft32_pointer_to_address);
+  set_gdbarch_integer_to_address (gdbarch, ft32_integer_to_address);
+
   set_gdbarch_skip_prologue (gdbarch, ft32_skip_prologue);
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, ft32_breakpoint_from_pc);
@@ -535,6 +630,12 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Support simple overlay manager.  */
   set_gdbarch_overlay_update (gdbarch, simple_overlay_update);
 
+  set_gdbarch_address_class_type_flags (gdbarch, ft32_address_class_type_flags);
+  set_gdbarch_address_class_name_to_type_flags
+    (gdbarch, ft32_address_class_name_to_type_flags);
+  set_gdbarch_address_class_type_flags_to_name
+    (gdbarch, ft32_address_class_type_flags_to_name);
+
   return gdbarch;
 }
 
diff --git a/gdb/ft32-tdep.h b/gdb/ft32-tdep.h
index 5c52480..f236163 100644
--- a/gdb/ft32-tdep.h
+++ b/gdb/ft32-tdep.h
@@ -22,7 +22,12 @@
 
 struct gdbarch_tdep
 {
-  /* gdbarch target dependent data here.  Currently unused for FT32.  */
+  /* Type for void.  */
+  struct type *void_type;
+  /* Type for a function returning void.  */
+  struct type *func_void_type;
+  /* Type for a pointer to a function.  Used for the type of PC.  */
+  struct type *pc_type;
 };
 
 #endif /* FT32_TDEP_H */
From gdb-patches-return-126278-listarch-gdb-patches=sources.redhat.com@sourceware.org Mon Sep 21 23:17:04 2015
Return-Path: <gdb-patches-return-126278-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-patches@sources.redhat.com
Received: (qmail 118636 invoked by alias); 21 Sep 2015 23:17:04 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Delivered-To: mailing list gdb-patches@sourceware.org
Received: (qmail 118625 invoked by uid 89); 21 Sep 2015 23:17:03 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2
X-HELO: usevmg21.ericsson.net
Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 21 Sep 2015 23:17:02 +0000
Received: from EUSAAHC001.ericsson.se (Unknown_Domain [147.117.188.75])	by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 7D.B4.26730.9B420065; Mon, 21 Sep 2015 17:39:37 +0200 (CEST)
Received: from elxcz23q12-y4.dyn.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.75) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Sep 2015 19:16:59 -0400
From: Simon Marchi <simon.marchi@ericsson.com>
To: <gdb-patches@sourceware.org>
CC: Simon Marchi <simon.marchi@ericsson.com>
Subject: [PATCH] tui: Simplify tui_alloc_content
Date: Mon, 21 Sep 2015 23:17:00 -0000
Message-ID: <1442877416-16659-1-git-send-email-simon.marchi@ericsson.com>
MIME-Version: 1.0
Content-Type: text/plain
X-IsSubscribed: yes
X-SW-Source: 2015-09/txt/msg00518.txt.bz2
Content-length: 2383

I stumbled upon this while doing some cxx-conversion work.  Since the
x-family alloc functions throw on failure, it is useless to test their
result for failure.  The else branch of != NULL is basically dead code.

I changed the type of element_block_ptr to struct tui_win_element, which
seems obvious (this is actually what raised the flag, casting the result
of xmalloc to struct tui_win_element* wouldn't work).

gdb/ChangeLog:

	* tui/tui-data.c (tui_alloc_content): Don't check xmalloc
	result.  Change type of element_block_ptr.  Change allocation to
	use XNEWVEC.
---
 gdb/tui/tui-data.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index 2fcd547..ca7502d 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -573,40 +573,27 @@ tui_win_content
 tui_alloc_content (int num_elements, enum tui_win_type type)
 {
   tui_win_content content;
-  char *element_block_ptr;
+  struct tui_win_element *element_block_ptr;
   int i;

   content = XNEWVEC (struct tui_win_element *, num_elements);
-  if (content != NULL)
+
+  /*
+   * All windows, except the data window, can allocate the
+   * elements in a chunk.  The data window cannot because items
+   * can be added/removed from the data display by the user at any
+   * time.
+   */
+  if (type != DATA_WIN)
     {
-      /*
-       * All windows, except the data window, can allocate the
-       * elements in a chunk.  The data window cannot because items
-       * can be added/removed from the data display by the user at any
-       * time.
-       */
-      if (type != DATA_WIN)
+      element_block_ptr = XNEWVEC (struct tui_win_element, num_elements);
+      for (i = 0; i < num_elements; i++)
 	{
-	  element_block_ptr -	    xmalloc (sizeof (struct tui_win_element) * num_elements);
-	  if (element_block_ptr != NULL)
-	    {
-	      for (i = 0; i < num_elements; i++)
-		{
-		  content[i] = (struct tui_win_element *) element_block_ptr;
-		  init_content_element (content[i], type);
-		  element_block_ptr += sizeof (struct tui_win_element);
-		}
-	    }
-	  else
-	    {
-	      xfree (content);
-	      content = (tui_win_content) NULL;
-	    }
+	  content[i] = element_block_ptr;
+	  init_content_element (content[i], type);
+	  element_block_ptr++;
 	}
     }
-
-  return content;
 }


--
2.5.1

Re: [PATCH, FT32] Proper support for address <-> pointer conversions

[Yao Qi]

James Bowman <james.bowman@ftdichip.com> writes:

> FT32 is a Harvard architecture with two address spaces -- RAM and flash.
> This patch properly implements the address to pointer conversion method,
> and its reverse. There are some other small fixes to handle address
> spaces.

Hi James,
Thanks for your patch.  A few comments below...

Please split your patch, in order to place each independent small fix in
one patch.

>
> OK to apply?
>
> 2015-09-21  James Bowman  <james.bowman@ftdichip.com>
>
> 	* ft32-tdep.c: Properly support address <-> pointer conversions,
> 	so that FT32's address spaces (RAM and flash) work correctly
>         when dereferencing pointers in gdb.
> 	* ft32-tdep.h: Add a type for an address space 1 pointer.

The changelog entry looks incorrect to me.  Please read
https://sourceware.org/gdb/wiki/ContributionChecklist

>
>
> diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
> index 2e5deca..d6e8c6d 100644
> --- a/gdb/ft32-tdep.c
> +++ b/gdb/ft32-tdep.c
> @@ -117,7 +117,7 @@ static struct type *
>  ft32_register_type (struct gdbarch *gdbarch, int reg_nr)
>  {
>    if (reg_nr == FT32_PC_REGNUM)
> -    return builtin_type (gdbarch)->builtin_func_ptr;
> +    return gdbarch_tdep (gdbarch)->pc_type;
>    else if (reg_nr == FT32_SP_REGNUM || reg_nr == FT32_FP_REGNUM)
>      return builtin_type (gdbarch)->builtin_data_ptr;
>    else
> @@ -270,6 +270,88 @@ ft32_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
>    return pc;
>  }
>  
> +/* Convert from address to pointer and vice-versa.  */
> +
> +static void
> +ft32_address_to_pointer (struct gdbarch *gdbarch,
> +			 struct type *type, gdb_byte *buf, CORE_ADDR addr)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
> +  store_unsigned_integer (buf, TYPE_LENGTH (type), byte_order,
> +			  addr);
> +}

ft32_address_to_pointer is exactly the same as
unsigned_address_to_pointer, which is the default implementation of
gdbarch method address_to_pointer.  IOW, we don't need this function.

> +
> +static CORE_ADDR
> +ft32_pointer_to_address (struct gdbarch *gdbarch,
> +			 struct type *type, const gdb_byte *buf)

We need comments to this function, like,

"Implementation of ....."

> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  CORE_ADDR addr
> +    = extract_unsigned_integer (buf, TYPE_LENGTH (type), byte_order);

A blank line is needed here.

> +  if (TYPE_ADDRESS_CLASS_1 (type))
> +    return addr;
> +  else
> +    return addr | RAM_BIAS;
> +}



> +
> +static CORE_ADDR
> +ft32_integer_to_address (struct gdbarch *gdbarch,
> +			 struct type *type, const gdb_byte *buf)
> +{
> +  ULONGEST addr = unpack_long (type, buf);
> +
> +  return addr;
> +}
> +

Do we really need this function?  Looks it makes no differences after
this hook is installed to gdbarch.

>  
>  static CORE_ADDR
> @@ -498,6 +580,15 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    tdep = XNEW (struct gdbarch_tdep);
>    gdbarch = gdbarch_alloc (&info, tdep);
>  
> +  /* Create a type for PC.  We can't use builtin types here, as they may not
> +     be defined.  */
> +  tdep->void_type = arch_type (gdbarch, TYPE_CODE_VOID, 1, "void");
> +  tdep->func_void_type = make_function_type (tdep->void_type, NULL);
> +  tdep->pc_type = arch_type (gdbarch, TYPE_CODE_PTR, 4, NULL);
> +  TYPE_TARGET_TYPE (tdep->pc_type) = tdep->func_void_type;
> +  TYPE_UNSIGNED (tdep->pc_type) = 1;
> +  TYPE_INSTANCE_FLAGS (tdep->pc_type) |= TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
> +
>    set_gdbarch_read_pc (gdbarch, ft32_read_pc);
>    set_gdbarch_write_pc (gdbarch, ft32_write_pc);
>    set_gdbarch_unwind_sp (gdbarch, ft32_unwind_sp);
> @@ -510,6 +601,10 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>  
>    set_gdbarch_return_value (gdbarch, ft32_return_value);
>  
> +  set_gdbarch_address_to_pointer (gdbarch, ft32_address_to_pointer);
> +  set_gdbarch_pointer_to_address (gdbarch, ft32_pointer_to_address);
> +  set_gdbarch_integer_to_address (gdbarch, ft32_integer_to_address);
> +
>    set_gdbarch_skip_prologue (gdbarch, ft32_skip_prologue);
>    set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
>    set_gdbarch_breakpoint_from_pc (gdbarch, ft32_breakpoint_from_pc);
> @@ -535,6 +630,12 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    /* Support simple overlay manager.  */
>    set_gdbarch_overlay_update (gdbarch, simple_overlay_update);
>  
> +  set_gdbarch_address_class_type_flags (gdbarch, ft32_address_class_type_flags);
> +  set_gdbarch_address_class_name_to_type_flags
> +    (gdbarch, ft32_address_class_name_to_type_flags);
> +  set_gdbarch_address_class_type_flags_to_name
> +    (gdbarch, ft32_address_class_type_flags_to_name);
> +
>    return gdbarch;
>  }
>  
> diff --git a/gdb/ft32-tdep.h b/gdb/ft32-tdep.h
> index 5c52480..f236163 100644
> --- a/gdb/ft32-tdep.h
> +++ b/gdb/ft32-tdep.h
> @@ -22,7 +22,12 @@
>  
>  struct gdbarch_tdep
>  {
> -  /* gdbarch target dependent data here.  Currently unused for FT32.  */
> +  /* Type for void.  */
> +  struct type *void_type;
> +  /* Type for a function returning void.  */
> +  struct type *func_void_type;
> +  /* Type for a pointer to a function.  Used for the type of PC.  */
> +  struct type *pc_type;

Only pc_type is used (in ft32_register_type), so we don't need to add
fields void_type and func_void_type.

-- 
Yao (齐尧)

RE: [PATCH, FT32] Proper support for address <-> pointer conversions

[James Bowman]

Thanks for the review. Corrected Changelog/patch is below.

However I was not sure about splitting -- these changes all
seem to go together. That is, applying just some of the patch
results in a non-functioning gdb.

2015-09-22  James Bowman  <james.bowman@ftdichip.com>

	* ft32-tdep.c: Implement the pointer to address method so
	that FT32's address spaces (RAM and flash) work correctly when
	dereferencing pointers in gdb. Also make PC a flash pointer.
	* ft32-tdep.h: Add a type for an address space 1 pointer, for
	the PC.


diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 2e5deca..7c6efbb 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -117,7 +117,7 @@ static struct type *
 ft32_register_type (struct gdbarch *gdbarch, int reg_nr)
 {
   if (reg_nr == FT32_PC_REGNUM)
-    return builtin_type (gdbarch)->builtin_func_ptr;
+    return gdbarch_tdep (gdbarch)->pc_type;
   else if (reg_nr == FT32_SP_REGNUM || reg_nr == FT32_FP_REGNUM)
     return builtin_type (gdbarch)->builtin_data_ptr;
   else
@@ -270,6 +270,73 @@ ft32_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
   return pc;
 }
 
+/* Implementation of `pointer_to_address' gdbarch method.
+
+   On FT32 address space zero is RAM, address space 1 is flash.
+   RAM appears at address RAM_BIAS, flash at address 0.  */
+
+static CORE_ADDR
+ft32_pointer_to_address (struct gdbarch *gdbarch,
+			 struct type *type, const gdb_byte *buf)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  CORE_ADDR addr
+    = extract_unsigned_integer (buf, TYPE_LENGTH (type), byte_order);
+
+  if (TYPE_ADDRESS_CLASS_1 (type))
+    return addr;
+  else
+    return addr | RAM_BIAS;
+}
+
+/* Implementation of `address_class_type_flags' gdbarch method.
+
+   This method maps DW_AT_address_class attributes to a
+   type_instance_flag_value.  */
+
+static int
+ft32_address_class_type_flags (int byte_size, int dwarf2_addr_class)
+{
+  /* The value 1 of the DW_AT_address_class attribute corresponds to the
+     __flash__ qualifier, meaning pointer to data in FT32 program memory.
+   */
+  if (dwarf2_addr_class == 1)
+    return TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+  return 0;
+}
+
+/* Implementation of `address_class_type_flags_to_name' gdbarch method.
+
+   Convert a type_instance_flag_value to an address space qualifier.  */
+
+static const char*
+ft32_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
+{
+  if (type_flags & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1)
+    return "flash";
+  else
+    return NULL;
+}
+
+/* Implementation of `address_class_name_to_type_flags' gdbarch method.
+
+   Convert an address space qualifier to a type_instance_flag_value.  */
+
+static int
+ft32_address_class_name_to_type_flags (struct gdbarch *gdbarch,
+				       const char* name,
+				       int *type_flags_ptr)
+{
+  if (strcmp (name, "flash") == 0)
+    {
+      *type_flags_ptr = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+      return 1;
+    }
+  else
+    return 0;
+}
+
+
 /* Implement the "read_pc" gdbarch method.  */
 
 static CORE_ADDR
@@ -488,6 +555,8 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
   struct gdbarch *gdbarch;
   struct gdbarch_tdep *tdep;
+  struct type *void_type;
+  struct type *func_void_type;
 
   /* If there is already a candidate, use it.  */
   arches = gdbarch_list_lookup_by_info (arches, &info);
@@ -498,6 +567,15 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep = XNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
+  /* Create a type for PC.  We can't use builtin types here, as they may not
+     be defined.  */
+  void_type = arch_type (gdbarch, TYPE_CODE_VOID, 1, "void");
+  func_void_type = make_function_type (void_type, NULL);
+  tdep->pc_type = arch_type (gdbarch, TYPE_CODE_PTR, 4, NULL);
+  TYPE_TARGET_TYPE (tdep->pc_type) = func_void_type;
+  TYPE_UNSIGNED (tdep->pc_type) = 1;
+  TYPE_INSTANCE_FLAGS (tdep->pc_type) |= TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+
   set_gdbarch_read_pc (gdbarch, ft32_read_pc);
   set_gdbarch_write_pc (gdbarch, ft32_write_pc);
   set_gdbarch_unwind_sp (gdbarch, ft32_unwind_sp);
@@ -510,6 +588,8 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   set_gdbarch_return_value (gdbarch, ft32_return_value);
 
+  set_gdbarch_pointer_to_address (gdbarch, ft32_pointer_to_address);
+
   set_gdbarch_skip_prologue (gdbarch, ft32_skip_prologue);
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, ft32_breakpoint_from_pc);
@@ -535,6 +615,12 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Support simple overlay manager.  */
   set_gdbarch_overlay_update (gdbarch, simple_overlay_update);
 
+  set_gdbarch_address_class_type_flags (gdbarch, ft32_address_class_type_flags);
+  set_gdbarch_address_class_name_to_type_flags
+    (gdbarch, ft32_address_class_name_to_type_flags);
+  set_gdbarch_address_class_type_flags_to_name
+    (gdbarch, ft32_address_class_type_flags_to_name);
+
   return gdbarch;
 }
 
diff --git a/gdb/ft32-tdep.h b/gdb/ft32-tdep.h
index 5c52480..ba747b0 100644
--- a/gdb/ft32-tdep.h
+++ b/gdb/ft32-tdep.h
@@ -22,7 +22,8 @@
 
 struct gdbarch_tdep
 {
-  /* gdbarch target dependent data here.  Currently unused for FT32.  */
+  /* Type for a pointer to a function.  Used for the type of PC.  */
+  struct type *pc_type;
 };
 
 #endif /* FT32_TDEP_H */

Re: [PATCH, FT32] Proper support for address <-> pointer conversions

[Doug Evans]

On Tue, Sep 22, 2015 at 11:37 AM, James Bowman
<james.bowman@ftdichip.com> wrote:
> Thanks for the review. Corrected Changelog/patch is below.
>
> However I was not sure about splitting -- these changes all
> seem to go together. That is, applying just some of the patch
> results in a non-functioning gdb.
>
> 2015-09-22  James Bowman  <james.bowman@ftdichip.com>
>
>         * ft32-tdep.c: Implement the pointer to address method so
>         that FT32's address spaces (RAM and flash) work correctly when
>         dereferencing pointers in gdb. Also make PC a flash pointer.
>         * ft32-tdep.h: Add a type for an address space 1 pointer, for
>         the PC.

Yeah, there'll always be some grey area as to when one should split up a patch.
This patch is small enough and is completely target specific.

The ChangeLog is still incorrect though. E.g. compare with just about
every existing changelog entry.
gdb's pretty good at following its own conventions, and that in turn
makes it easy to continue to follow them.
Just go with the flow and mimic (without being creative) what's already there.
There's also:
https://sourceware.org/gdb/wiki/ContributionChecklist

RE: [PATCH, FT32] Proper support for address <-> pointer conversions

[James Bowman]

Thanks, have attempted to match the style of gdb/Changelog.
How does this look?

2015-09-22  James Bowman  <james.bowman@ftdichip.com>

	* ft32-tdep.c: Add "pointer_to_address" gdbarch method.
	Add "address_class_type_flags" gdbarch method.
	Add "address_class_name_to_type" gdbarch method.
	Add "address_class_type_flags_to_name" gdbarch method.
	* ft32-tdep.h (struct gdbarch_tdep): Add pc_type.

+  set_gdbarch_address_class_type_flags (gdbarch, ft32_address_class_type_flags);
+  set_gdbarch_address_class_name_to_type_flags
+    (gdbarch, ft32_address_class_name_to_type_flags);
+  set_gdbarch_address_class_type_flags_to_name
+    (gdbarch, ft32_address_class_type_flags_to_name);
+
diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 2e5deca..7c6efbb 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -117,7 +117,7 @@ static struct type *
 ft32_register_type (struct gdbarch *gdbarch, int reg_nr)
 {
   if (reg_nr == FT32_PC_REGNUM)
-    return builtin_type (gdbarch)->builtin_func_ptr;
+    return gdbarch_tdep (gdbarch)->pc_type;
   else if (reg_nr == FT32_SP_REGNUM || reg_nr == FT32_FP_REGNUM)
     return builtin_type (gdbarch)->builtin_data_ptr;
   else
@@ -270,6 +270,73 @@ ft32_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
   return pc;
 }
 
+/* Implementation of `pointer_to_address' gdbarch method.
+
+   On FT32 address space zero is RAM, address space 1 is flash.
+   RAM appears at address RAM_BIAS, flash at address 0.  */
+
+static CORE_ADDR
+ft32_pointer_to_address (struct gdbarch *gdbarch,
+			 struct type *type, const gdb_byte *buf)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  CORE_ADDR addr
+    = extract_unsigned_integer (buf, TYPE_LENGTH (type), byte_order);
+
+  if (TYPE_ADDRESS_CLASS_1 (type))
+    return addr;
+  else
+    return addr | RAM_BIAS;
+}
+
+/* Implementation of `address_class_type_flags' gdbarch method.
+
+   This method maps DW_AT_address_class attributes to a
+   type_instance_flag_value.  */
+
+static int
+ft32_address_class_type_flags (int byte_size, int dwarf2_addr_class)
+{
+  /* The value 1 of the DW_AT_address_class attribute corresponds to the
+     __flash__ qualifier, meaning pointer to data in FT32 program memory.
+   */
+  if (dwarf2_addr_class == 1)
+    return TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+  return 0;
+}
+
+/* Implementation of `address_class_type_flags_to_name' gdbarch method.
+
+   Convert a type_instance_flag_value to an address space qualifier.  */
+
+static const char*
+ft32_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
+{
+  if (type_flags & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1)
+    return "flash";
+  else
+    return NULL;
+}
+
+/* Implementation of `address_class_name_to_type_flags' gdbarch method.
+
+   Convert an address space qualifier to a type_instance_flag_value.  */
+
+static int
+ft32_address_class_name_to_type_flags (struct gdbarch *gdbarch,
+				       const char* name,
+				       int *type_flags_ptr)
+{
+  if (strcmp (name, "flash") == 0)
+    {
+      *type_flags_ptr = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+      return 1;
+    }
+  else
+    return 0;
+}
+
+
 /* Implement the "read_pc" gdbarch method.  */
 
 static CORE_ADDR
@@ -488,6 +555,8 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
   struct gdbarch *gdbarch;
   struct gdbarch_tdep *tdep;
+  struct type *void_type;
+  struct type *func_void_type;
 
   /* If there is already a candidate, use it.  */
   arches = gdbarch_list_lookup_by_info (arches, &info);
@@ -498,6 +567,15 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep = XNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
 
+  /* Create a type for PC.  We can't use builtin types here, as they may not
+     be defined.  */
+  void_type = arch_type (gdbarch, TYPE_CODE_VOID, 1, "void");
+  func_void_type = make_function_type (void_type, NULL);
+  tdep->pc_type = arch_type (gdbarch, TYPE_CODE_PTR, 4, NULL);
+  TYPE_TARGET_TYPE (tdep->pc_type) = func_void_type;
+  TYPE_UNSIGNED (tdep->pc_type) = 1;
+  TYPE_INSTANCE_FLAGS (tdep->pc_type) |= TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+
   set_gdbarch_read_pc (gdbarch, ft32_read_pc);
   set_gdbarch_write_pc (gdbarch, ft32_write_pc);
   set_gdbarch_unwind_sp (gdbarch, ft32_unwind_sp);
@@ -510,6 +588,8 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   set_gdbarch_return_value (gdbarch, ft32_return_value);
 
+  set_gdbarch_pointer_to_address (gdbarch, ft32_pointer_to_address);
+
   set_gdbarch_skip_prologue (gdbarch, ft32_skip_prologue);
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, ft32_breakpoint_from_pc);
@@ -535,6 +615,12 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Support simple overlay manager.  */
   set_gdbarch_overlay_update (gdbarch, simple_overlay_update);
 
+  set_gdbarch_address_class_type_flags (gdbarch, ft32_address_class_type_flags);
+  set_gdbarch_address_class_name_to_type_flags
+    (gdbarch, ft32_address_class_name_to_type_flags);
+  set_gdbarch_address_class_type_flags_to_name
+    (gdbarch, ft32_address_class_type_flags_to_name);
+
   return gdbarch;
 }
 
diff --git a/gdb/ft32-tdep.h b/gdb/ft32-tdep.h
index 5c52480..ba747b0 100644
--- a/gdb/ft32-tdep.h
+++ b/gdb/ft32-tdep.h
@@ -22,7 +22,8 @@
 
 struct gdbarch_tdep
 {
-  /* gdbarch target dependent data here.  Currently unused for FT32.  */
+  /* Type for a pointer to a function.  Used for the type of PC.  */
+  struct type *pc_type;
 };
 
 #endif /* FT32_TDEP_H */

Re: [PATCH, FT32] Proper support for address <-> pointer conversions

[Yao Qi]

James Bowman <james.bowman@ftdichip.com> writes:

> 2015-09-22  James Bowman  <james.bowman@ftdichip.com>
>
> 	* ft32-tdep.c: Add "pointer_to_address" gdbarch method.
> 	Add "address_class_type_flags" gdbarch method.
> 	Add "address_class_name_to_type" gdbarch method.
> 	Add "address_class_type_flags_to_name" gdbarch method.
> 	* ft32-tdep.h (struct gdbarch_tdep): Add pc_type.
>

The ChangeLog is still wrong to me.  You need also read GNU coding
standard, https://www.gnu.org/prep/standards/standards.html

	* ft32-tdep.c (ft32_register_type): Return gdbarch_tdep (gdbarch)->pc_type
        instead of builtin_func_ptr.
        (ft32_pointer_to_address): New function.
        (ft32_address_class_type_flags): New function.
        (ft32_address_class_type_flags_to_name): New function.
        (ft32_address_class_name_to_type_flags): New function.
        (ft32_gdbarch_init): Set tdep->pc_type.  Call
        set_gdbarch_pointer_to_address,
        set_gdbarch_address_class_type_flags
        set_gdbarch_address_class_name_to_type_flags,
        and set_gdbarch_address_class_type_flags_to_name.
        * ft32-tdep.h (struct gdbarch_tdep) <pc_type>: New field.

Otherwise, the patch looks good to me.

-- 
Yao (齐尧)