Re: [PATCH] use gdbarch_addr_bits_remove for entry point address

[Yao Qi <yao at codesourcery dot com>]

On 11/24/2012 03:58 AM, Pedro Alves wrote:
> If this is needed here, then it would look to me that  gdbarch_convert_from_func_ptr_addr
> would be needed too.  See the function right below  init_entry_point_info:
> 
> /* If there is a valid and known entry point, function fills *ENTRY_P with it
>     and returns non-zero; otherwise it returns zero.  */
> 
> int
> entry_point_address_query (CORE_ADDR *entry_p)
> {
>    struct gdbarch *gdbarch;
>    CORE_ADDR entry_point;
> 
>    if (symfile_objfile == NULL || !symfile_objfile->ei.entry_point_p)
>      return 0;
> 
>    gdbarch = get_objfile_arch (symfile_objfile);
> 
>    entry_point = symfile_objfile->ei.entry_point;
> 
>    /* Make certain that the address points at real code, and not a
>       function descriptor.  */
>    entry_point = gdbarch_convert_from_func_ptr_addr (gdbarch, entry_point,
> 						    &current_target);
> 
>    /* Remove any ISA markers, so that this matches entries in the
>       symbol table.  */
>    entry_point = gdbarch_addr_bits_remove (gdbarch, entry_point);
> 
>    *entry_p = entry_point;
>    return 1;
> }
> 
> So you if put the gdbarch_addr_bits_remove call in init_entry_point_info,
> ISTM the same call in entry_point_address_query is no longer necessary.  And
> that it'd be better to move gdbarch_convert_from_func_ptr_addr too, I'd think (and I
> don't know if there's an order they should be called in; moving both preserves the order).

Pedro,
when wrote this patch, I thought of the order of
convert_from_func_ptr_addr and addr_bits_remove, is it required that
both of them should be called together (convert_from_func_ptr_addr
first and then addr_bits_remove)?  Examining the code shows that it is
not required (addr_bits_remove doesn't follow
convert_from_func_ptr_addr where it is called).  Further more, do we
need to call addr_bits_remove first to clear the bits of 'addr', pass
'addr' to 'convert_from_func_ptr_addr', and clear the return value of
'convert_from_func_ptr_addr'?  To be honest, I don't know.

> Maybe there are yet other callers that could have gdbarch_addr_bits_remove calls
> removed as redundant too, I haven't checked.

AFAICT, there isn't any.   Patch below is to move both
convert_from_func_ptr_addr and addr_bits_remove from
'entry_point_address_query' to 'init_entry_point_info', as I don't know
the right order, let us keep using the current one.

b.t.w, shall addr_bits_remove be called after every place where
convert_from_func_ptr_addr is called?  We don't do this, and it is
another issue, if it is.

-- 
Yao (éå)

gdb:

2012-11-24  Daniel Jacobowitz  <dan@codesourcery.com>
	    Kazu Hirata  <kazu@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* objfiles.c (entry_point_address_query): Move some code ...
	(init_entry_point_info): ... here.
	* solib-svr4.c (exec_entry_point): Likewise.
	* symfile.c (generic_load): Call gdbarch_addr_bits_remove on
	the entry address.
---
 gdb/objfiles.c   |   34 ++++++++++++++++++----------------
 gdb/solib-svr4.c |    5 ++++-
 gdb/symfile.c    |    1 +
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 4cc2fea..e5681fa 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -353,6 +353,23 @@ init_entry_point_info (struct objfile *objfile)
       /* Examination of non-executable.o files.  Short-circuit this stuff.  */
       objfile->ei.entry_point_p = 0;
     }
+
+  if (objfile->ei.entry_point_p)
+    {
+      CORE_ADDR entry_point =  objfile->ei.entry_point;
+
+      /* Make certain that the address points at real code, and not a
+	 function descriptor.  */
+      entry_point
+	= gdbarch_convert_from_func_ptr_addr (objfile->gdbarch,
+					      entry_point,
+					      &current_target);
+
+      /* Remove any ISA markers, so that this matches entries in the
+	 symbol table.  */
+      objfile->ei.entry_point
+	= gdbarch_addr_bits_remove (objfile->gdbarch, entry_point);
+    }
 }
 
 /* If there is a valid and known entry point, function fills *ENTRY_P with it
@@ -361,26 +378,11 @@ init_entry_point_info (struct objfile *objfile)
 int
 entry_point_address_query (CORE_ADDR *entry_p)
 {
-  struct gdbarch *gdbarch;
-  CORE_ADDR entry_point;
-
   if (symfile_objfile == NULL || !symfile_objfile->ei.entry_point_p)
     return 0;
 
-  gdbarch = get_objfile_arch (symfile_objfile);
-
-  entry_point = symfile_objfile->ei.entry_point;
-
-  /* Make certain that the address points at real code, and not a
-     function descriptor.  */
-  entry_point = gdbarch_convert_from_func_ptr_addr (gdbarch, entry_point,
-						    &current_target);
-
-  /* Remove any ISA markers, so that this matches entries in the
-     symbol table.  */
-  entry_point = gdbarch_addr_bits_remove (gdbarch, entry_point);
+  *entry_p = symfile_objfile->ei.entry_point;
 
-  *entry_p = entry_point;
   return 1;
 }
 
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 37cc654..02e45a3 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1388,6 +1388,8 @@ svr4_in_dynsym_resolve_code (CORE_ADDR pc)
 static CORE_ADDR
 exec_entry_point (struct bfd *abfd, struct target_ops *targ)
 {
+  CORE_ADDR addr;
+
   /* KevinB wrote ... for most targets, the address returned by
      bfd_get_start_address() is the entry point for the start
      function.  But, for some targets, bfd_get_start_address() returns
@@ -1396,9 +1398,10 @@ exec_entry_point (struct bfd *abfd, struct target_ops *targ)
      gdbarch_convert_from_func_ptr_addr().  The method
      gdbarch_convert_from_func_ptr_addr() is the merely the identify
      function for targets which don't use function descriptors.  */
-  return gdbarch_convert_from_func_ptr_addr (target_gdbarch (),
+  addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch (),
 					     bfd_get_start_address (abfd),
 					     targ);
+  return gdbarch_addr_bits_remove (target_gdbarch (), addr);
 }
 
 /* Helper function for gdb_bfd_lookup_symbol.  */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 55af541..70f631f 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2132,6 +2132,7 @@ generic_load (char *args, int from_tty)
   gettimeofday (&end_time, NULL);
 
   entry = bfd_get_start_address (loadfile_bfd);
+  entry = gdbarch_addr_bits_remove (target_gdbarch (), entry);
   ui_out_text (uiout, "Start address ");
   ui_out_field_fmt (uiout, "address", "%s", paddress (target_gdbarch (), entry));
   ui_out_text (uiout, ", load size ");
-- 
1.7.7.6