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

Message ID CA9BBF0458F83C4F9051448B941B57D119614FCF@glaexch1
State New, archived
Headers

Commit Message

James Bowman Sept. 22, 2015, 6:37 p.m. UTC
  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.
  

Comments

Doug Evans Sept. 22, 2015, 7:53 p.m. UTC | #1
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
  

Patch

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 */