[RFC 2/3] Record function descriptor address instead of function address in value

[Yao Qi <qiyaoltc at gmail dot com>]

1. Problem statement

Nowadays, function address is recorded in value in GDB.  It is convenient
to use GDB on some targets function pointer address isn't function
address, like ppc64 and arm thumb mode.  For example, on ppc64,

(gdb) p incr
$4 = {int (int)} 0x1000069c <incr>
(gdb) disassemble incr
Dump of assembler code for function incr:
   0x000000001000069c <+0>:	mflr    r0

"incr" is the address of "incr" function, which differs from its meaning
in C language.  In .c file, "incr" is the address of "incr" function
descriptor, if we write

  printf ("incr %llx, &incr %llx\n", incr, &incr);

it prints the function descriptor address.  Then this brings confusion
in GDB when we do expression evaluation,

int incr (int i) {}
int (*calc) (int) = incr;

in C, the statement above means assign "incr" function descriptor
address to "calc", or "calc" pointers to "incr" function descriptor.
However, if we evaluation expressions in gdb with function involved,
we'll see something interesting,

(gdb) p if calc == incr

this prints 0 because calc pointers to "incr" function descriptor,
but "incr" is its function address, so they are different,

(gdb) set calc = incr

"incr" function address is assigned to "calc", but it is expected
to get function descriptor address.

2.  Solution

GCC vs GDB divergence on the meaning of "incr" brings the confusion.
We should reduce such divergence as much as we can.  However, this
divergence was added in https://sourceware.org/ml/gdb-patches/2001-11/msg00001.html
I agree with Jim, but I'd like use function descriptor address in value,
which makes the whole expression evaluation look more reasonable and
more close to compiler's behavior.

In this patch, I add a new gdbarch method convert_from_func_addr, which
converts function address back to function descriptor address or
function pointer address.  It is the reversed operation of
convert_from_func_ptr_addr.  We convert function address to function
descriptor address when,

 - we create value for a function,
 - we generate ax code for a function,

I don't change the meaning of return value of value_as_address, because
it is widely used, so value_as_address still return the function
address if type is TYPE_CODE_FUNC or TYPE_CODE_METHOD.

3. User visible changes

This patch brings several user visible changes, which look more
accurate, shown by this table below,

 COMMAND           BEFORE                       AFTER
 p main            main function address        main function descriptor
                                                address
 disass main       disassembly function main    not changed
 disass main+4,+4  disassembly 4 bytes from     disassembly 4 bytes from
                   function main address + 4    main's function descriptor + 4
 x/i main          show one instruction on      show one instruction on main's
                   function main address        function descriptor

Although the latter looks inconvenient, that is consistent to the
meaning on C language level.  Due to these changes, test cases are
adjusted accordingly.

gdb:

2016-10-14  Yao Qi  <yao.qi@linaro.org>

	* arch-utils.c (convert_from_func_addr_identity): New function.
	* arch-utils.h (convert_from_func_addr_identity): Declare.
	* arm-tdep.c (arm_convert_from_func_addr): New function.
	(arm_convert_from_func_ptr_addr): New function.
	(arm_gdbarch_init): Install gdbarch hook convert_from_func_addr
	and convert_from_func_ptr_addr.
	* ax-gdb.c (gen_var_ref): Call gdbarch_convert_from_func_addr.
	* findvar.c (default_read_var_value): Likewise.
	* gdbarch.sh (convert_from_func_addr): New.
	* gdbarch.h, gdbarch.c: Re-generated.
	* infcall.c (find_function_addr): Call value_as_address
	instead of value_address.
	* ppc64-tdep.c (ppc64_convert_from_func_addr): New function.
	* ppc64-tdep.h (ppc64_convert_from_func_addr): Declare.
	* ppc-linux-tdep.c (ppc_linux_init_abi): Install
	convert_from_func_addr.
	* ppcfbsd-tdep.c: Likewise.
	* value.c (value_as_address): Call gdbarch_convert_from_func_ptr_addr.
---
 gdb/arch-utils.c     |  6 ++++++
 gdb/arch-utils.h     |  1 +
 gdb/arm-tdep.c       | 23 +++++++++++++++++++++++
 gdb/ax-gdb.c         | 12 +++++++++---
 gdb/findvar.c        |  5 +++++
 gdb/gdbarch.c        | 23 +++++++++++++++++++++++
 gdb/gdbarch.h        |  6 ++++++
 gdb/gdbarch.sh       |  4 ++++
 gdb/infcall.c        |  2 +-
 gdb/ppc-linux-tdep.c |  2 ++
 gdb/ppc64-tdep.c     | 30 ++++++++++++++++++++++++++++++
 gdb/ppc64-tdep.h     |  3 ++-
 gdb/ppcfbsd-tdep.c   |  2 ++
 gdb/value.c          |  4 +++-
 14 files changed, 117 insertions(+), 6 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 776dabc..b5ec892 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -168,6 +168,12 @@ convert_from_func_ptr_addr_identity (struct gdbarch *gdbarch, CORE_ADDR addr,
   return addr;
 }
 
+CORE_ADDR
+convert_from_func_addr_identity (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  return addr;
+}
+
 int
 no_op_reg_to_regnum (struct gdbarch *gdbarch, int reg)
 {
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index bbb0878..d86e2a7 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -63,6 +63,7 @@ extern int core_addr_greaterthan (CORE_ADDR lhs, CORE_ADDR rhs);
 
 extern CORE_ADDR core_addr_identity (struct gdbarch *gdbarch, CORE_ADDR addr);
 extern gdbarch_convert_from_func_ptr_addr_ftype convert_from_func_ptr_addr_identity;
+extern gdbarch_convert_from_func_addr_ftype convert_from_func_addr_identity;
 
 /* No-op conversion of reg to regnum.  */
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 27a3ebe..cf4a5e8 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -537,6 +537,26 @@ arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val)
     return (val & 0x03fffffc);
 }
 
+/* Implement the convert_from_func_addr gdbarch method.  */
+
+static CORE_ADDR
+arm_convert_from_func_addr (struct gdbarch *gdbarch, CORE_ADDR val)
+{
+  if (arm_pc_is_thumb (gdbarch, val))
+    return MAKE_THUMB_ADDR (val);
+  else
+    return val;
+}
+
+/* Implement the convert_from_func_ptr_addr gdbarch method.  */
+
+static CORE_ADDR
+arm_convert_from_func_ptr_addr (struct gdbarch *gdbarch, CORE_ADDR addr,
+				struct target_ops *targ)
+{
+  return IS_THUMB_ADDR (addr) ? UNMAKE_THUMB_ADDR (addr) : addr;
+}
+
 /* Return 1 if PC is the start of a compiler helper function which
    can be safely ignored during prologue skipping.  IS_THUMB is true
    if the function is known to be a Thumb function due to the way it
@@ -9396,6 +9416,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* Address manipulation.  */
   set_gdbarch_addr_bits_remove (gdbarch, arm_addr_bits_remove);
+  set_gdbarch_convert_from_func_addr (gdbarch, arm_convert_from_func_addr);
+  set_gdbarch_convert_from_func_ptr_addr (gdbarch,
+					  arm_convert_from_func_ptr_addr);
 
   /* Advance PC across function entry code.  */
   set_gdbarch_skip_prologue (gdbarch, arm_skip_prologue);
diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 7c6cb64..a698741 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -689,10 +689,16 @@ gen_var_ref (struct gdbarch *gdbarch, struct agent_expr *ax,
       break;
 
     case LOC_BLOCK:
-      ax_const_l (ax, BLOCK_START (SYMBOL_BLOCK_VALUE (var)));
-      value->kind = axs_rvalue;
-      break;
+      {
+	CORE_ADDR addr = BLOCK_START (SYMBOL_BLOCK_VALUE (var));
 
+	/* Get address of function pointer rather than function
+	   address.  */
+	addr = gdbarch_convert_from_func_addr (gdbarch, addr);
+	ax_const_l (ax, addr);
+	value->kind = axs_rvalue;
+	break;
+      }
     case LOC_REGISTER:
       /* Don't generate any code at all; in the process of treating
          this as an lvalue or rvalue, the caller will generate the
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 6e28a29..3b4bd15 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -698,6 +698,11 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
 	   SYMBOL_OBJ_SECTION (symbol_objfile (var), var));
       else
 	addr = BLOCK_START (SYMBOL_BLOCK_VALUE (var));
+
+      /* Get address of function pointer rather than function
+	 address.  */
+      addr = gdbarch_convert_from_func_addr (get_type_arch (type),
+					     addr);
       break;
 
     case LOC_REGISTER:
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 4d8ef18..a213d9b 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -246,6 +246,7 @@ struct gdbarch
   gdbarch_stabs_argument_has_addr_ftype *stabs_argument_has_addr;
   int frame_red_zone_size;
   gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr;
+  gdbarch_convert_from_func_addr_ftype *convert_from_func_addr;
   gdbarch_addr_bits_remove_ftype *addr_bits_remove;
   gdbarch_software_single_step_ftype *software_single_step;
   gdbarch_single_step_through_delay_ftype *single_step_through_delay;
@@ -409,6 +410,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->remote_register_number = default_remote_register_number;
   gdbarch->stabs_argument_has_addr = default_stabs_argument_has_addr;
   gdbarch->convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity;
+  gdbarch->convert_from_func_addr = convert_from_func_addr_identity;
   gdbarch->addr_bits_remove = core_addr_identity;
   gdbarch->skip_trampoline_code = generic_skip_trampoline_code;
   gdbarch->skip_solib_resolver = generic_skip_solib_resolver;
@@ -598,6 +600,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of frame_align, has predicate.  */
   /* Skip verify of stabs_argument_has_addr, invalid_p == 0 */
   /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0 */
+  /* Skip verify of convert_from_func_addr, invalid_p == 0 */
   /* Skip verify of addr_bits_remove, invalid_p == 0 */
   /* Skip verify of software_single_step, has predicate.  */
   /* Skip verify of single_step_through_delay, has predicate.  */
@@ -820,6 +823,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: coff_make_msymbol_special = <%s>\n",
                       host_address_to_string (gdbarch->coff_make_msymbol_special));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: convert_from_func_addr = <%s>\n",
+                      host_address_to_string (gdbarch->convert_from_func_addr));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: convert_from_func_ptr_addr = <%s>\n",
                       host_address_to_string (gdbarch->convert_from_func_ptr_addr));
   fprintf_unfiltered (file,
@@ -3096,6 +3102,23 @@ set_gdbarch_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
 }
 
 CORE_ADDR
+gdbarch_convert_from_func_addr (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->convert_from_func_addr != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_convert_from_func_addr called\n");
+  return gdbarch->convert_from_func_addr (gdbarch, addr);
+}
+
+void
+set_gdbarch_convert_from_func_addr (struct gdbarch *gdbarch,
+                                    gdbarch_convert_from_func_addr_ftype convert_from_func_addr)
+{
+  gdbarch->convert_from_func_addr = convert_from_func_addr;
+}
+
+CORE_ADDR
 gdbarch_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
   gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index cd01718..a93a603 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -642,6 +642,12 @@ typedef CORE_ADDR (gdbarch_convert_from_func_ptr_addr_ftype) (struct gdbarch *gd
 extern CORE_ADDR gdbarch_convert_from_func_ptr_addr (struct gdbarch *gdbarch, CORE_ADDR addr, struct target_ops *targ);
 extern void set_gdbarch_convert_from_func_ptr_addr (struct gdbarch *gdbarch, gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr);
 
+/* Convert function address to function pointer address. */
+
+typedef CORE_ADDR (gdbarch_convert_from_func_addr_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern CORE_ADDR gdbarch_convert_from_func_addr (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void set_gdbarch_convert_from_func_addr (struct gdbarch *gdbarch, gdbarch_convert_from_func_addr_ftype *convert_from_func_addr);
+
 /* On some machines there are bits in addresses which are not really
    part of the address, but are used by the kernel, the hardware, etc.
    for special purposes.  gdbarch_addr_bits_remove takes out any such bits so
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 1663156..57b541a 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -598,6 +598,10 @@ m:int:stabs_argument_has_addr:struct type *type:type::default_stabs_argument_has
 v:int:frame_red_zone_size
 #
 m:CORE_ADDR:convert_from_func_ptr_addr:CORE_ADDR addr, struct target_ops *targ:addr, targ::convert_from_func_ptr_addr_identity::0
+
+# Convert function address to function pointer address.
+m:CORE_ADDR:convert_from_func_addr:CORE_ADDR addr:addr::convert_from_func_addr_identity::0
+
 # On some machines there are bits in addresses which are not really
 # part of the address, but are used by the kernel, the hardware, etc.
 # for special purposes.  gdbarch_addr_bits_remove takes out any such bits so
diff --git a/gdb/infcall.c b/gdb/infcall.c
index ab7426d..718e479 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -260,7 +260,7 @@ find_function_addr (struct value *function, struct type **retval_type)
   /* Determine address to call.  */
   if (TYPE_CODE (ftype) == TYPE_CODE_FUNC
       || TYPE_CODE (ftype) == TYPE_CODE_METHOD)
-    funaddr = value_address (function);
+    funaddr = value_as_address (function);
   else if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
     {
       funaddr = value_as_address (function);
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index ee158e3..4626ab6 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1744,6 +1744,8 @@ ppc_linux_init_abi (struct gdbarch_info info,
 	     function descriptors).  */
 	  set_gdbarch_convert_from_func_ptr_addr
 	    (gdbarch, ppc64_convert_from_func_ptr_addr);
+	  set_gdbarch_convert_from_func_addr
+	    (gdbarch, ppc64_convert_from_func_entry);
 
 	  set_gdbarch_elf_make_msymbol_special
 	    (gdbarch, ppc64_elf_make_msymbol_special);
diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
index b7357e3..6b2be83 100644
--- a/gdb/ppc64-tdep.c
+++ b/gdb/ppc64-tdep.c
@@ -24,6 +24,7 @@
 #include "ppc-tdep.h"
 #include "ppc64-tdep.h"
 #include "elf-bfd.h"
+#include "objfiles.h"
 
 /* Macros for matching instructions.  Note that, since all the
    operands are masked off before they're or-ed into the instruction,
@@ -615,6 +616,35 @@ ppc64_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
   return addr;
 }
 
+/* Implement the convert_from_func_addr gdbarch method.  */
+
+CORE_ADDR
+ppc64_convert_from_func_addr (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  struct symbol *sym = find_pc_function (addr);
+
+  if (sym != NULL)
+    {
+      struct bound_minimal_symbol msym;
+      struct obj_section *dot_fn_section;
+
+      dot_fn_section = find_pc_section (addr);
+      if (dot_fn_section == NULL || dot_fn_section->objfile == NULL)
+	return addr;
+
+      msym = lookup_minimal_symbol (SYMBOL_LINKAGE_NAME (sym), NULL,
+				    dot_fn_section->objfile);
+
+      /* That is the address of function descriptor.  */
+      if (msym.minsym != NULL)
+	return BMSYMBOL_VALUE_ADDRESS (msym);
+      else
+	return addr;
+    }
+  else
+    return addr;
+}
+
 /* A synthetic 'dot' symbols on ppc64 has the udata.p entry pointing
    back to the original ELF symbol it was derived from.  Get the size
    from that symbol.  */
diff --git a/gdb/ppc64-tdep.h b/gdb/ppc64-tdep.h
index 202ffca..b9f315b 100644
--- a/gdb/ppc64-tdep.h
+++ b/gdb/ppc64-tdep.h
@@ -30,7 +30,8 @@ extern CORE_ADDR ppc64_skip_trampoline_code (struct frame_info *frame,
 extern CORE_ADDR ppc64_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
 						   CORE_ADDR addr,
 						   struct target_ops *targ);
-
+extern CORE_ADDR ppc64_convert_from_func_addr (struct gdbarch *gdbarch,
+					       CORE_ADDR addr);
 extern void ppc64_elf_make_msymbol_special (asymbol *,
 					    struct minimal_symbol *);
 #endif /* PPC64_TDEP_H  */
diff --git a/gdb/ppcfbsd-tdep.c b/gdb/ppcfbsd-tdep.c
index 10b41b0..3133a81 100644
--- a/gdb/ppcfbsd-tdep.c
+++ b/gdb/ppcfbsd-tdep.c
@@ -320,6 +320,8 @@ ppcfbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
     {
       set_gdbarch_convert_from_func_ptr_addr
 	(gdbarch, ppc64_convert_from_func_ptr_addr);
+      set_gdbarch_convert_from_func_addr
+	(gdbarch, ppc64_convert_from_func_entry);
       set_gdbarch_elf_make_msymbol_special (gdbarch,
 					    ppc64_elf_make_msymbol_special);
 
diff --git a/gdb/value.c b/gdb/value.c
index b825aec..039789d 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2853,7 +2853,9 @@ value_as_address (struct value *val)
      function, just return its address directly.  */
   if (TYPE_CODE (value_type (val)) == TYPE_CODE_FUNC
       || TYPE_CODE (value_type (val)) == TYPE_CODE_METHOD)
-    return value_address (val);
+    return gdbarch_convert_from_func_ptr_addr (gdbarch,
+					       value_address (val),
+					       &current_target);
 
   val = coerce_array (val);
 
-- 
1.9.1