[PATCH] use gdbarch_addr_bits_remove for entry point address

[PATCH] use gdbarch_addr_bits_remove for entry point address

[Yao Qi]

Hi,
This patch attempts to clear the lsb of the entry address, which might
be set by compiler for thumb code.

This patch does something similar to this one,

  RFC: Handle ISA bits for the entry point
  http://sourceware.org/ml/gdb-patches/2009-07/msg00682.html

Regression tested on arm-none-linux-gnueabi.  Is it OK?

gdb:

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

	* objfiles.c (init_entry_point_info): Use gdbarch_addr_bits_remove.
	* solib-svr4.c (exec_entry_point): Likewise.
	* symfile.c (generic_load): Call gdbarch_addr_bits_remove on
	the entry address.
---
 gdb/objfiles.c   |    5 +++++
 gdb/solib-svr4.c |    5 ++++-
 gdb/symfile.c    |    1 +
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index a1db8c6..3374c68 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -353,6 +353,11 @@ 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)
+    objfile->ei.entry_point
+      = gdbarch_addr_bits_remove (objfile->gdbarch,
+				  objfile->ei.entry_point);
 }
 
 /* If there is a valid and known entry point, function fills *ENTRY_P with it
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

Re: [PATCH] use gdbarch_addr_bits_remove for entry point address

[Pedro Alves]

On 11/20/2012 09:36 AM, Yao Qi wrote:

> This patch attempts to clear the lsb of the entry address, which might
> be set by compiler for thumb code.
> 
> This patch does something similar to this one,
> 
>   RFC: Handle ISA bits for the entry point
>   http://sourceware.org/ml/gdb-patches/2009-07/msg00682.html
> 
> Regression tested on arm-none-linux-gnueabi.  Is it OK?
> 
> gdb:
> 
> 2012-11-20  Daniel Jacobowitz  <dan@codesourcery.com>
> 	    Kazu Hirata  <kazu@codesourcery.com>
> 	    Yao Qi  <yao@codesourcery.com>
> 
> 	* objfiles.c (init_entry_point_info): Use gdbarch_addr_bits_remove.
> 	* solib-svr4.c (exec_entry_point): Likewise.
> 	* symfile.c (generic_load): Call gdbarch_addr_bits_remove on
> 	the entry address.
> ---
>  gdb/objfiles.c   |    5 +++++
>  gdb/solib-svr4.c |    5 ++++-
>  gdb/symfile.c    |    1 +
>  3 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index a1db8c6..3374c68 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -353,6 +353,11 @@ 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)
> +    objfile->ei.entry_point
> +      = gdbarch_addr_bits_remove (objfile->gdbarch,
> +				  objfile->ei.entry_point);
>  }

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).
Maybe there are yet other callers that could have gdbarch_addr_bits_remove calls
removed as redundant too, I haven't checked.

-- 
Pedro Alves

Re: [PATCH] use gdbarch_addr_bits_remove for entry point address

[Yao Qi]

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

Re: [PATCH] use gdbarch_addr_bits_remove for entry point address

[Pedro Alves]

This version is OK, but please fix up the ChangeLog entry.

> 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.

Please be more specific:

 	* objfiles.c (entry_point_address_query): Call
	gdbarch_convert_from_func_ptr_addr and gdbarch_addr_bits_remove
	here ...
 	(init_entry_point_info): ... instead of here.

That's more useful for archaeology.

> 	* solib-svr4.c (exec_entry_point): Likewise.

"Likewise" here doesn't make sense.  Perhaps:

 	* solib-svr4.c (exec_entry_point): Call gdbarch_addr_bits_remove.

> 	* symfile.c (generic_load): Call gdbarch_addr_bits_remove on
> 	the entry address.

-- 
Pedro Alves

Re: [PATCH] use gdbarch_addr_bits_remove for entry point address

[Yao Qi]

On 11/26/2012 07:57 PM, Pedro Alves wrote:
> Please be more specific:
>
>   	* objfiles.c (entry_point_address_query): Call
> 	gdbarch_convert_from_func_ptr_addr and gdbarch_addr_bits_remove
> 	here ...
>   	(init_entry_point_info): ... instead of here.
>
> That's more useful for archaeology.
>
>> >	* solib-svr4.c (exec_entry_point): Likewise.
> "Likewise" here doesn't make sense.  Perhaps:
>
>   	* solib-svr4.c (exec_entry_point): Call gdbarch_addr_bits_remove.
>

The changelog entry is updated, and patch is committed.  Thanks for the 
review.

-- 
Yao (齐尧)

crash/regression with ia64 targets (was: "Re: [PATCH] use gdbarch_addr_bits_remove for entry point address")

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]

> 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.

Unfortunately, this patch breaks GDB on ia64 (linux and hpux) :-(.
Take any program, and try loading it in GDB:

    (gdb) file hello
    Reading symbols from /[...]/hello...
    zsh: 9462 segmentation fault  /[...]/gdb

What happens is that we've now added a call to
gdbarch_convert_from_func_ptr_addr inside init_entry_point_info,
which is called from syms_from_objfile *before* the objfile's
section_offsets array is allocated.

On ia64, gdbarch_convert_from_func_ptr_addr resolves to
ia64_convert_from_func_ptr_addr, which calls find_pc_section.
This, in turns calls update_section_map, which does a sort
on all sections, where qsort_cmp uses the obj_section_addr
macro:

#define obj_section_addr(s)                                             \
  (bfd_get_section_vma ((s)->objfile->obfd, s->the_bfd_section)         \
   + obj_section_offset (s))

... and obj_section_offset is defined as:

#define obj_section_offset(s)                                           \
  (((s)->objfile->section_offsets)->offsets[(s)->the_bfd_section->index])
                  ^^^^^^^^^^^^^^^^^^

Basically, to resolve whether our pointer is pointing to a descriptor
or not, we need to find its associated section. But to find the section,
we need the section_offsets to be defined.

So, to me, it looks like the attempt at resolving the entry point
is performed too early. And to make things even more fun, there
are cases where we do not allocated section_offsets at all:

  if (objfile->sf == NULL)
    return;     /* No symbols.  */

But the other worrisome element is that most calls to
init_entry_point_info are made from routines used as
the "sym_init" struct sym_fns hook, and this hook is
in fact called, in syms_from_objfile, before the section_offsets
table is allocated.

Since syms_from_objfile is calling init_entry_point_info,
it seems to me that the call to init_entry_point_info in
the "sym_init" hooks are redundant, and could be removed,
clearing one hurdle.

The other hurdle is making sure that init_entry_point_info
is called *after* the section offsets have been allocated.
Which means we need to make sure that we always allocate
some, including in the case where no symbols are found.
This must also become a documented invariant.

Attached is a prototype that seems to work on ia64-linux.
I've only tested it against our testsuite for now, but it will
need to be tested with the official testsuite on GNU/Linux,
as well as on Darwin, AiX, and maybe Windows (although,
I think the changes removing the calls to init_entry_point_info
should be fine).

Note that there is a second call to init_entry_point_info,
this time inside reread_symbols, but this one should be fine.

This patch also begs the question whether we might want to
move init_entry_point_info to objfiles.c and make it static.

Thoughts?

-- 
Joel

[-- Attachment #2: ia64-entry-point.diff --]
[-- Type: text/x-diff, Size: 3227 bytes --]

diff --git a/gdb/coffread.c b/gdb/coffread.c
index 3789995..186c8d2 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -468,8 +468,6 @@ coff_symfile_init (struct objfile *objfile)
      find this causes a significant slowdown in gdb then we could
      set it in the debug symbol readers only when necessary.  */
   objfile->flags |= OBJF_REORDERED;
-
-  init_entry_point_info (objfile);
 }
 
 /* This function is called for every section; it finds the outer
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 6a6eaa1..c0e6d90 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -79,7 +79,6 @@ static void
 macho_symfile_init (struct objfile *objfile)
 {
   objfile->flags |= OBJF_REORDERED;
-  init_entry_point_info (objfile);
 }
 
 /*  Add a new OSO to the vector of OSO to load.  */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index bc4f40a..f182617 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -929,8 +929,8 @@ read_symbols (struct objfile *objfile, int add_flags)
    an extra symbol file such as dynamically loaded code, and wether
    breakpoint reset should be deferred.  */
 
-void
-syms_from_objfile (struct objfile *objfile,
+static void
+syms_from_objfile_1 (struct objfile *objfile,
                    struct section_addr_info *addrs,
                    struct section_offsets *offsets,
                    int num_offsets,
@@ -943,11 +943,19 @@ syms_from_objfile (struct objfile *objfile,
   gdb_assert (! (addrs && offsets));
 
   clear_ada_sym_cache ();
-  init_entry_point_info (objfile);
   objfile->sf = find_sym_fns (objfile->obfd);
 
   if (objfile->sf == NULL)
-    return;	/* No symbols.  */
+    {
+      int num_sections = bfd_count_sections (objfile->obfd);
+      size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets);
+
+      objfile->num_sections = num_sections;
+      objfile->section_offsets
+        = obstack_alloc (&objfile->objfile_obstack, size);
+      memset (objfile->section_offsets, 0, size);
+      return;	/* No symbols.  */
+    }
 
   /* Make sure that partially constructed symbol tables will be cleaned up
      if an error occurs during symbol reading.  */
@@ -1028,6 +1036,17 @@ syms_from_objfile (struct objfile *objfile,
   xfree (local_addr);
 }
 
+void
+syms_from_objfile (struct objfile *objfile,
+                   struct section_addr_info *addrs,
+                   struct section_offsets *offsets,
+                   int num_offsets,
+		   int add_flags)
+{
+  syms_from_objfile_1 (objfile, addrs, offsets, num_offsets, add_flags);
+  init_entry_point_info (objfile);
+}
+
 /* Perform required actions after either reading in the initial
    symbols for a new objfile, or mapping in the symbols from a reusable
    objfile.  ADD_FLAGS is a bitmask of enum symfile_add_flags.  */
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 2e562ed..e340b39 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -1927,8 +1927,6 @@ xcoff_symfile_init (struct objfile *objfile)
      find this causes a significant slowdown in gdb then we could
      set it in the debug symbol readers only when necessary.  */
   objfile->flags |= OBJF_REORDERED;
-
-  init_entry_point_info (objfile);
 }
 
 /* Perform any local cleanups required when we are done with a particular

Re: crash/regression with ia64 targets

[Yao Qi]

On 12/13/2012 08:05 PM, Joel Brobecker wrote:
> But the other worrisome element is that most calls to
> init_entry_point_info are made from routines used as
> the "sym_init" struct sym_fns hook, and this hook is
> in fact called, in syms_from_objfile, before the section_offsets
> table is allocated.
>
> Since syms_from_objfile is calling init_entry_point_info,
> it seems to me that the call to init_entry_point_info in
> the "sym_init" hooks are redundant, and could be removed,
> clearing one hurdle.

The change here looks straightforward to me, but I agree that we need 
test to double-check.

>
> The other hurdle is making sure that init_entry_point_info
> is called*after*  the section offsets have been allocated.
> Which means we need to make sure that we always allocate
> some, including in the case where no symbols are found.
> This must also become a documented invariant.

How about add an assert in init_entry_point_info?

   gdb_assert (objfile->section_offsets != NULL);

>
> Attached is a prototype that seems to work on ia64-linux.
> I've only tested it against our testsuite for now, but it will
> need to be tested with the official testsuite on GNU/Linux,
> as well as on Darwin, AiX, and maybe Windows (although,

I tested this patch (with conflict resolved, this patch can't be applied 
cleanly to FSF GDB trunk) on x86_64-linux with both board file unix and 
native-gdbserver respectively.  No regression.

> I think the changes removing the calls to init_entry_point_info
> should be fine).
>
> Note that there is a second call to init_entry_point_info,
> this time inside reread_symbols, but this one should be fine.

Yeah, looks 'objfile->section_offsets' has been already set when call 
init_entry_point_info in reread_symbols.

>
> This patch also begs the question whether we might want to
> move init_entry_point_info to objfiles.c and make it static.
>

I guess you meant "symfile.c" rather than "objfiles.c".  I am not sure, 
looks init_entry_point_info is not related to "symfile.c" very much, but 
I am not against this moving.

> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index bc4f40a..f182617 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -929,8 +929,8 @@ read_symbols (struct objfile *objfile, int add_flags)
>      an extra symbol file such as dynamically loaded code, and wether
>      breakpoint reset should be deferred.  */
>
> -void
> -syms_from_objfile (struct objfile *objfile,
> +static void
> +syms_from_objfile_1 (struct objfile *objfile,
>                      struct section_addr_info *addrs,
>                      struct section_offsets *offsets,
>                      int num_offsets,
> @@ -943,11 +943,19 @@ syms_from_objfile (struct objfile *objfile,
>     gdb_assert (! (addrs && offsets));
>
>     clear_ada_sym_cache ();
> -  init_entry_point_info (objfile);
>     objfile->sf = find_sym_fns (objfile->obfd);
>
>     if (objfile->sf == NULL)
> -    return;	/* No symbols.  */
> +    {
> +      int num_sections = bfd_count_sections (objfile->obfd);
> +      size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets);
> +
> +      objfile->num_sections = num_sections;

Can we use 'num_offsets' here, because I see these two lines in some 
lines below here,

       /* Just copy in the offset table directly as given to us.  */
       objfile->num_sections = num_offsets;

in this way, we don't call 'bfd_count_sections'.


> +      objfile->section_offsets
> +        = obstack_alloc (&objfile->objfile_obstack, size);
> +      memset (objfile->section_offsets, 0, size);
> +      return;	/* No symbols.  */
> +    }
>
>     /* Make sure that partially constructed symbol tables will be cleaned up
>        if an error occurs during symbol reading.  */
> @@ -1028,6 +1036,17 @@ syms_from_objfile (struct objfile *objfile,
>     xfree (local_addr);
>   }
>
> +void
> +syms_from_objfile (struct objfile *objfile,
> +                   struct section_addr_info *addrs,
> +                   struct section_offsets *offsets,
> +                   int num_offsets,
> +		   int add_flags)
> +{
> +  syms_from_objfile_1 (objfile, addrs, offsets, num_offsets, add_flags);
> +  init_entry_point_info (objfile);
> +}
> +
>   /* Perform required actions after either reading in the initial
>      symbols for a new objfile, or mapping in the symbols from a reusable
>      objfile.  ADD_FLAGS is a bitmask of enum symfile_add_flags.  */
-- 
Yao (齐尧)

Re: crash/regression with ia64 targets

[Joel Brobecker]

> How about add an assert in init_entry_point_info?
> 
>   gdb_assert (objfile->section_offsets != NULL);

I forgot about this one when I sent my patch. We could
add one separately. But it feels a little strange to put it
there, because it's not obvious why. If anything, I feel
it should be right next to where the access is made, which
might be a little difficult, given that it's in a macro and
not a function.

> >+      int num_sections = bfd_count_sections (objfile->obfd);
> >+      size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets);
> >+
> >+      objfile->num_sections = num_sections;
> 
> Can we use 'num_offsets' here, because I see these two lines in some
> lines below here,
> 
>       /* Just copy in the offset table directly as given to us.  */
>       objfile->num_sections = num_offsets;

I do not think so. The lines you quote take into the fact that
the function was passed a valid num_offsets.  But this is not
always the case: You can pass a section_addr_info insted, or
even nothing.

-- 
Joel

Re: crash/regression with ia64 targets

[Pedro Alves]

On 12/13/2012 12:05 PM, Joel Brobecker wrote:
>> 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.
> Unfortunately, this patch breaks GDB on ia64 (linux and hpux) :-(.
> Take any program, and try loading it in GDB:
> 
>     (gdb) file hello
>     Reading symbols from /[...]/hello...
>     zsh: 9462 segmentation fault  /[...]/gdb
> 
> What happens is that we've now added a call to
> gdbarch_convert_from_func_ptr_addr inside init_entry_point_info,
> which is called from syms_from_objfile *before* the objfile's
> section_offsets array is allocated.
> 
> On ia64, gdbarch_convert_from_func_ptr_addr resolves to
> ia64_convert_from_func_ptr_addr, which calls find_pc_section.
> This, in turns calls update_section_map, which does a sort
> on all sections, where qsort_cmp uses the obj_section_addr
> macro:
> 
> #define obj_section_addr(s)                                             \
>   (bfd_get_section_vma ((s)->objfile->obfd, s->the_bfd_section)         \
>    + obj_section_offset (s))
> 
> ... and obj_section_offset is defined as:
> 
> #define obj_section_offset(s)                                           \
>   (((s)->objfile->section_offsets)->offsets[(s)->the_bfd_section->index])
>                   ^^^^^^^^^^^^^^^^^^
> 
> Basically, to resolve whether our pointer is pointing to a descriptor
> or not, we need to find its associated section. But to find the section,
> we need the section_offsets to be defined.
> 
> So, to me, it looks like the attempt at resolving the entry point
> is performed too early. And to make things even more fun, there
> are cases where we do not allocated section_offsets at all:
> 
>   if (objfile->sf == NULL)
>     return;     /* No symbols.  */
> 
> But the other worrisome element is that most calls to
> init_entry_point_info are made from routines used as
> the "sym_init" struct sym_fns hook, and this hook is
> in fact called, in syms_from_objfile, before the section_offsets
> table is allocated.
> 
> Since syms_from_objfile is calling init_entry_point_info,
> it seems to me that the call to init_entry_point_info in
> the "sym_init" hooks are redundant, and could be removed,
> clearing one hurdle.
> 
> The other hurdle is making sure that init_entry_point_info
> is called *after* the section offsets have been allocated.
> Which means we need to make sure that we always allocate
> some, including in the case where no symbols are found.
> This must also become a documented invariant.

Seems fine to me.

I wonder why this crash wasn't visible before.  AFAICS from find_sym_fns,
the only kinds of objfiles that can be loaded without debugging symbols are
srec, ihex and tekhex (the latter can have symbols, but we don't read in those).

Ah, indeed, it was.  I can reproduce it:

...
(top-gdb) start
Temporary breakpoint 3 at 0x48f020: file ../../src/gdb/gdb.c, line 26.
Starting program: /home/pedro/gdb/mygit/build-all/gdb/gdb
Temporary breakpoint 3,
main (argc=1, argv=0x7fffffffdc38) at ../../src/gdb/gdb.c:26
26      {
(top-gdb) dump srec value dump.srec main
(top-gdb) shell cat dump.srec
S00C000064756D702E7372656362
S20548F020485A
S804000000FB
(top-gdb) file dump.srec
A program is being debugged already.
Are you sure you want to change the file? (y or n) y
Load new symbol table from "/home/pedro/gdb/mygit/build-all/gdb/dump.srec"? (y or n) y
Reading symbols from /home/pedro/gdb/mygit/build-all/gdb/dump.srec...(no debugging symbols found)...done.

Program received signal SIGSEGV, Segmentation fault.
0x00000000006c6566 in qsort_cmp (a=<optimized out>, b=<optimized out>) at ../../src/gdb/objfiles.c:1059
1059      const CORE_ADDR sect2_addr = obj_section_addr (sect2);
(top-gdb) bt
#0  0x00000000006c6566 in qsort_cmp (a=<optimized out>, b=<optimized out>) at ../../src/gdb/objfiles.c:1059
#1  0x0000003d25e37d72 in msort_with_tmp (p=0x7fffffffd5f0, b=0x35693a8, n=2) at msort.c:84
#2  0x0000003d25e37b5a in msort_with_tmp (n=2, b=0x35693a8, p=0x7fffffffd5f0) at msort.c:46
#3  msort_with_tmp (p=0x7fffffffd5f0, b=0x35693a0, n=3) at msort.c:55
#4  0x0000003d25e37b5a in msort_with_tmp (n=3, b=0x35693a0, p=0x7fffffffd5f0) at msort.c:46
#5  msort_with_tmp (p=0x7fffffffd5f0, b=0x3569390, n=5) at msort.c:55
#6  0x0000003d25e37b5a in msort_with_tmp (n=5, b=0x3569390, p=0x7fffffffd5f0) at msort.c:46
#7  msort_with_tmp (p=0x7fffffffd5f0, b=0x3569368, n=10) at msort.c:55
#8  0x0000003d25e37b5a in msort_with_tmp (n=10, b=0x3569368, p=0x7fffffffd5f0) at msort.c:46
#9  msort_with_tmp (p=0x7fffffffd5f0, b=0x3569318, n=20) at msort.c:55
#10 0x0000003d25e37b5a in msort_with_tmp (n=20, b=0x3569318, p=0x7fffffffd5f0) at msort.c:46
#11 msort_with_tmp (p=0x7fffffffd5f0, b=0x3569280, n=39) at msort.c:55
#12 0x0000003d25e37b5a in msort_with_tmp (n=39, b=0x3569280, p=0x7fffffffd5f0) at msort.c:46
#13 msort_with_tmp (p=0x7fffffffd5f0, b=0x3569150, n=77) at msort.c:55
#14 0x0000003d25e37b5a in msort_with_tmp (n=77, b=0x3569150, p=0x7fffffffd5f0) at msort.c:46
#15 msort_with_tmp (p=0x7fffffffd5f0, b=0x3568ee8, n=154) at msort.c:55
#16 0x0000003d25e37b5a in msort_with_tmp (n=154, b=0x3568ee8, p=0x7fffffffd5f0) at msort.c:46
#17 msort_with_tmp (p=0x7fffffffd5f0, b=0x3568a18, n=308) at msort.c:55
#18 0x0000003d25e37b5a in msort_with_tmp (n=308, b=0x3568a18, p=0x7fffffffd5f0) at msort.c:46
#19 msort_with_tmp (p=0x7fffffffd5f0, b=0x3568080, n=615) at msort.c:55
#20 0x0000003d25e380ac in msort_with_tmp (n=615, b=0x3568080, p=0x7fffffffd5f0) at msort.c:46
#21 __GI_qsort_r (b=b@entry=0x3568080, n=n@entry=615, s=s@entry=8, cmp=cmp@entry=0x6c6530 <qsort_cmp>, arg=arg@entry=0x0) at msort.c:298
#22 0x0000003d25e38168 in __GI_qsort (b=b@entry=0x3568080, n=n@entry=615, s=s@entry=8, cmp=cmp@entry=0x6c6530 <qsort_cmp>) at msort.c:308
#23 0x00000000006c7735 in update_section_map (pmap_size=0x1d11b30, pmap=0x1d11b28, pspace=0x1c96040) at ../../src/gdb/objfiles.c:1321
#24 find_pc_section (pc=4780064) at ../../src/gdb/objfiles.c:1366
#25 0x00000000006ca69d in lookup_minimal_symbol_by_pc_section (pc=pc@entry=4780064, section=section@entry=0x0) at ../../src/gdb/minsyms.c:708
#26 0x00000000006642a9 in find_pc_sect_symtab (pc=4780064, section=0x0) at ../../src/gdb/symtab.c:2078
#27 0x0000000000664495 in find_pc_symtab (pc=<optimized out>) at ../../src/gdb/symtab.c:2178
#28 0x0000000000758822 in select_frame (fi=0x26f8e80) at ../../src/gdb/frame.c:1448
#29 0x0000000000758979 in get_selected_frame (message=message@entry=0x0) at ../../src/gdb/frame.c:1386
#30 0x00000000007589e8 in deprecated_safe_get_selected_frame () at ../../src/gdb/frame.c:1410
#31 0x000000000074f189 in check_frame_language_change () at ../../src/gdb/top.c:376
#32 0x000000000074f35b in execute_command (p=0x1ab516d "c", p@entry=0x1ab5160 "", from_tty=1) at ../../src/gdb/top.c:504
...

We crash because bfd presents us a section:

$ objdump -h dump.srec

dump.srec:     file format srec

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .sec1         00000001  0048f020  0048f020  0000001e  2**0
                  CONTENTS, ALLOC, LOAD


and we allocate one entry for it in objfile->sections..sections_end but not
in the offsets table, and then walk over objfile->sections..sections_end:

#define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
  for (osect = objfile->sections; osect < objfile->sections_end; osect++)

> 
> Attached is a prototype that seems to work on ia64-linux.
> I've only tested it against our testsuite for now, but it will
> need to be tested with the official testsuite on GNU/Linux,
> as well as on Darwin, AiX, and maybe Windows (although,
> I think the changes removing the calls to init_entry_point_info
> should be fine).
> 
> Note that there is a second call to init_entry_point_info,
> this time inside reread_symbols, but this one should be fine.
> 
> This patch also begs the question whether we might want to
> move init_entry_point_info to objfiles.c and make it static.

Not clear to me which place would be best.  I suggest do nothing,
as its easiest  :-)

>    if (objfile->sf == NULL)
> -    return;	/* No symbols.  */
> +    {
> +      int num_sections = bfd_count_sections (objfile->obfd);
> +      size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets);
> +
> +      objfile->num_sections = num_sections;
> +      objfile->section_offsets
> +        = obstack_alloc (&objfile->objfile_obstack, size);
> +      memset (objfile->section_offsets, 0, size);
> +      return;	/* No symbols.  */

I'd put this "No symbols" comment right after the '{' after the if.

-- 
Pedro Alves

Re: crash/regression with ia64 targets

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

> I wonder why this crash wasn't visible before.  AFAICS from find_sym_fns,
> the only kinds of objfiles that can be loaded without debugging symbols are
> srec, ihex and tekhex (the latter can have symbols, but we don't read
> in those).
> 
> Ah, indeed, it was.  I can reproduce it:

Nice! (I think)

> Not clear to me which place would be best.  I suggest do nothing,
> as its easiest  :-)

Sold :)

Attached is the patch I am currently testing. It already passed
testing on ia64-linux (ie: I reverted the patch triggering the
crash, ran the testsuite, then un-reverted it, and applied this
patch), no regression. It passesd testing on x64_64-linux as well.

Testing on x86_64-darwin is under way, and I will add ppc-aix and
x86-windows using AdaCore's testsuite.

But given the nature of the changes triggering the cross-platform
testing, I remain fairly confident of the results.

gdb/ChangeLog:

        * symfile.c (syms_from_objfile_1): Renames syms_from_objfile.
        Remove call to init_entry_point_info.  Add OBJFILE's
        section_offsets and num_sections initialization.  Add note
        about entry info in the function documentation.
        (syms_from_objfile): New function.
        * coffread.c (coff_symfile_init): Remove call to
        init_entry_point_info.
        * machoread.c (macho_symfile_init): Likewise.
        * xcoffread.c(xcoff_symfile_init): Likewise.

Thanks,
-- 
Joel

[-- Attachment #2: 0001-Fix-NULL-objfile-s-section_offsets-dereference-durin.patch --]
[-- Type: text/x-diff, Size: 4903 bytes --]

From b1508860e512c1d9ee1c9826d1df5b1990af8230 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Fri, 14 Dec 2012 01:25:26 -0500
Subject: [PATCH] Fix NULL objfile's section_offsets dereference during symbol load.

gdb/ChangeLog:

        * symfile.c (syms_from_objfile_1): Renames syms_from_objfile.
        Remove call to init_entry_point_info.  Add OBJFILE's
        section_offsets and num_sections initialization.  Add note
        about entry info in the function documentation.
        (syms_from_objfile): New function.
        * coffread.c (coff_symfile_init): Remove call to
        init_entry_point_info.
        * machoread.c (macho_symfile_init): Likewise.
        * xcoffread.c(xcoff_symfile_init): Likewise.
---
 gdb/coffread.c  |    2 --
 gdb/machoread.c |    1 -
 gdb/symfile.c   |   42 ++++++++++++++++++++++++++++++++++--------
 gdb/xcoffread.c |    2 --
 4 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index 56ed5ae..398e61c 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -469,8 +469,6 @@ coff_symfile_init (struct objfile *objfile)
      find this causes a significant slowdown in gdb then we could
      set it in the debug symbol readers only when necessary.  */
   objfile->flags |= OBJF_REORDERED;
-
-  init_entry_point_info (objfile);
 }
 
 /* This function is called for every section; it finds the outer
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 6a6eaa1..c0e6d90 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -79,7 +79,6 @@ static void
 macho_symfile_init (struct objfile *objfile)
 {
   objfile->flags |= OBJF_REORDERED;
-  init_entry_point_info (objfile);
 }
 
 /*  Add a new OSO to the vector of OSO to load.  */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 6e09cbd..2a15293 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -896,6 +896,8 @@ read_symbols (struct objfile *objfile, int add_flags)
 /* Process a symbol file, as either the main file or as a dynamically
    loaded file.
 
+   This function does not set the OBJFILE's entry-point info.
+
    OBJFILE is where the symbols are to be read from.
 
    ADDRS is the list of section load addresses.  If the user has given
@@ -923,12 +925,12 @@ read_symbols (struct objfile *objfile, int add_flags)
    an extra symbol file such as dynamically loaded code, and wether
    breakpoint reset should be deferred.  */
 
-void
-syms_from_objfile (struct objfile *objfile,
-                   struct section_addr_info *addrs,
-                   struct section_offsets *offsets,
-                   int num_offsets,
-		   int add_flags)
+static void
+syms_from_objfile_1 (struct objfile *objfile,
+		     struct section_addr_info *addrs,
+		     struct section_offsets *offsets,
+		     int num_offsets,
+		     int add_flags)
 {
   struct section_addr_info *local_addr = NULL;
   struct cleanup *old_chain;
@@ -936,11 +938,21 @@ syms_from_objfile (struct objfile *objfile,
 
   gdb_assert (! (addrs && offsets));
 
-  init_entry_point_info (objfile);
   objfile->sf = find_sym_fns (objfile->obfd);
 
   if (objfile->sf == NULL)
-    return;	/* No symbols.  */
+    {
+      /* No symbols to load, but we still need to make sure
+	 that the section_offsets table is allocated.  */
+      int num_sections = bfd_count_sections (objfile->obfd);
+      size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets);
+
+      objfile->num_sections = num_sections;
+      objfile->section_offsets
+        = obstack_alloc (&objfile->objfile_obstack, size);
+      memset (objfile->section_offsets, 0, size);
+      return;
+    }
 
   /* Make sure that partially constructed symbol tables will be cleaned up
      if an error occurs during symbol reading.  */
@@ -1021,6 +1033,20 @@ syms_from_objfile (struct objfile *objfile,
   xfree (local_addr);
 }
 
+/* Same as syms_from_objfile_1, but also initializes the objfile
+   entry-point info.  */
+
+void
+syms_from_objfile (struct objfile *objfile,
+		   struct section_addr_info *addrs,
+		   struct section_offsets *offsets,
+		   int num_offsets,
+		   int add_flags)
+{
+  syms_from_objfile_1 (objfile, addrs, offsets, num_offsets, add_flags);
+  init_entry_point_info (objfile);
+}
+
 /* Perform required actions after either reading in the initial
    symbols for a new objfile, or mapping in the symbols from a reusable
    objfile.  ADD_FLAGS is a bitmask of enum symfile_add_flags.  */
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index ee47f6c..10c93cc 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -1922,8 +1922,6 @@ xcoff_symfile_init (struct objfile *objfile)
      find this causes a significant slowdown in gdb then we could
      set it in the debug symbol readers only when necessary.  */
   objfile->flags |= OBJF_REORDERED;
-
-  init_entry_point_info (objfile);
 }
 
 /* Perform any local cleanups required when we are done with a particular
-- 
1.7.0.4

checked in: Re: crash/regression with ia64 targets

[Joel Brobecker]

> Attached is the patch I am currently testing. It already passed
> testing on ia64-linux (ie: I reverted the patch triggering the
> crash, ran the testsuite, then un-reverted it, and applied this
> patch), no regression. It passesd testing on x64_64-linux as well.
> 
> Testing on x86_64-darwin is under way, and I will add ppc-aix and
> x86-windows using AdaCore's testsuite.
> 
> But given the nature of the changes triggering the cross-platform
> testing, I remain fairly confident of the results.

Tested was succesful on all platforms.

> gdb/ChangeLog:
> 
>         * symfile.c (syms_from_objfile_1): Renames syms_from_objfile.
>         Remove call to init_entry_point_info.  Add OBJFILE's
>         section_offsets and num_sections initialization.  Add note
>         about entry info in the function documentation.
>         (syms_from_objfile): New function.
>         * coffread.c (coff_symfile_init): Remove call to
>         init_entry_point_info.
>         * machoread.c (macho_symfile_init): Likewise.
>         * xcoffread.c(xcoff_symfile_init): Likewise.

So, I have committed this patch.

-- 
Joel

Re: crash/regression with ia64 targets

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This patch also begs the question whether we might want to
Joel> move init_entry_point_info to objfiles.c and make it static.

It seems like a good idea to me.
(I think you meant symfile.c here.)

Tom

Re: crash/regression with ia64 targets

[Joel Brobecker]

> It seems like a good idea to me.
> (I think you meant symfile.c here.)

OK - unless there are objections, I will see if I can move it,
but do it as a followup patch. Thanks, Tom.

-- 
Joel

Re: crash/regression with ia64 targets

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 439 bytes --]

> > It seems like a good idea to me.
> > (I think you meant symfile.c here.)

Here is a patch. I don't think it would be controversial, so
I checked it in right away. It's easy to adjust or revert if
requested.

gdb/ChangeLog:

        * objfiles.c (init_entry_point_info): Move function from here...
        * symfile.c (init_entry_point_info): ... to there.  Make static.
        * objfiles.h (objfiles.h): Delete declaration.

-- 
Joel

[-- Attachment #2: 0002-Move-init_entry_point_info-to-symfile.c-and-make-it-.patch --]
[-- Type: text/x-diff, Size: 5059 bytes --]

From aa833475f7e589719e2dd59f8241096f925dc69f Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Sat, 15 Dec 2012 16:07:26 +0400
Subject: [PATCH 2/2] Move init_entry_point_info to symfile.c and make it
 static.

gdb/ChangeLog:

        * objfiles.c (init_entry_point_info): Move function from here...
        * symfile.c (init_entry_point_info): ... to there.  Make static.
        * objfiles.h (objfiles.h): Delete declaration.
---
 gdb/objfiles.c |   48 ------------------------------------------------
 gdb/objfiles.h |    2 --
 gdb/symfile.c  |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index e5681fa..feb387b 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -324,54 +324,6 @@ get_objfile_arch (struct objfile *objfile)
   return objfile->gdbarch;
 }
 
-/* Initialize entry point information for this objfile.  */
-
-void
-init_entry_point_info (struct objfile *objfile)
-{
-  /* Save startup file's range of PC addresses to help blockframe.c
-     decide where the bottom of the stack is.  */
-
-  if (bfd_get_file_flags (objfile->obfd) & EXEC_P)
-    {
-      /* Executable file -- record its entry point so we'll recognize
-         the startup file because it contains the entry point.  */
-      objfile->ei.entry_point = bfd_get_start_address (objfile->obfd);
-      objfile->ei.entry_point_p = 1;
-    }
-  else if (bfd_get_file_flags (objfile->obfd) & DYNAMIC
-	   && bfd_get_start_address (objfile->obfd) != 0)
-    {
-      /* Some shared libraries may have entry points set and be
-	 runnable.  There's no clear way to indicate this, so just check
-	 for values other than zero.  */
-      objfile->ei.entry_point = bfd_get_start_address (objfile->obfd);
-      objfile->ei.entry_point_p = 1;
-    }
-  else
-    {
-      /* 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
    and returns non-zero; otherwise it returns zero.  */
 
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 877c9e0..c794598 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -443,8 +443,6 @@ extern struct objfile *allocate_objfile (bfd *, int);
 
 extern struct gdbarch *get_objfile_arch (struct objfile *);
 
-extern void init_entry_point_info (struct objfile *);
-
 extern int entry_point_address_query (CORE_ADDR *entry_p);
 
 extern CORE_ADDR entry_point_address (void);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 2a15293..fc908b3 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -893,6 +893,54 @@ read_symbols (struct objfile *objfile, int add_flags)
     require_partial_symbols (objfile, 0);
 }
 
+/* Initialize entry point information for this objfile.  */
+
+static void
+init_entry_point_info (struct objfile *objfile)
+{
+  /* Save startup file's range of PC addresses to help blockframe.c
+     decide where the bottom of the stack is.  */
+
+  if (bfd_get_file_flags (objfile->obfd) & EXEC_P)
+    {
+      /* Executable file -- record its entry point so we'll recognize
+         the startup file because it contains the entry point.  */
+      objfile->ei.entry_point = bfd_get_start_address (objfile->obfd);
+      objfile->ei.entry_point_p = 1;
+    }
+  else if (bfd_get_file_flags (objfile->obfd) & DYNAMIC
+	   && bfd_get_start_address (objfile->obfd) != 0)
+    {
+      /* Some shared libraries may have entry points set and be
+	 runnable.  There's no clear way to indicate this, so just check
+	 for values other than zero.  */
+      objfile->ei.entry_point = bfd_get_start_address (objfile->obfd);
+      objfile->ei.entry_point_p = 1;
+    }
+  else
+    {
+      /* 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);
+    }
+}
+
 /* Process a symbol file, as either the main file or as a dynamically
    loaded file.
 
-- 
1.7.10.4