Re: crash/regression with ia64 targets

[Yao Qi <yao@codesourcery.com>]

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 (齐尧)