Fix setting-breakpoints regression on PPC64 (function descriptors)

  Pedro Alves <palves@redhat.com> writes:

> @@ -4309,22 +4309,16 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
>  	      struct minimal_symbol *msymbol,
>  	      std::vector<symtab_and_line> *result)
>  {
> -  struct gdbarch *gdbarch = get_objfile_arch (objfile);
> -  CORE_ADDR pc;
>    struct symtab_and_line sal;
>  
> -  if (msymbol_is_text (msymbol))
> -    {
> -      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
> -			       (struct obj_section *) 0, 0);
> -      sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);

This line is removed, so that sal.section can be null.  It causes
PR 22567.  How is the patch below?

> +  /* The minimal symbol might point to a function descriptor, which is
> +     not a text symbol; try resolving it to the actual code
> +     address.  */
>  
> -      /* The minimal symbol might point to a function descriptor;
> -	 resolve it to the actual code address instead.  */
> -      pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
> -					       &current_target);
> -      if (pc != sal.pc)
> -	sal = find_pc_sect_line (pc, NULL, 0);
> +  CORE_ADDR func_addr;
> +  if (msymbol_is_function (objfile, msymbol, &func_addr))
> +    {
> +      sal = find_pc_sect_line (func_addr, NULL, 0);
>  
>        if (self->funfirstline)
>  	{
  

On 12/08/2017 09:44 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> @@ -4309,22 +4309,16 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
>>  	      struct minimal_symbol *msymbol,
>>  	      std::vector<symtab_and_line> *result)
>>  {
>> -  struct gdbarch *gdbarch = get_objfile_arch (objfile);
>> -  CORE_ADDR pc;
>>    struct symtab_and_line sal;
>>  
>> -  if (msymbol_is_text (msymbol))
>> -    {
>> -      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
>> -			       (struct obj_section *) 0, 0);
>> -      sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
> 
> This line is removed, so that sal.section can be null.  It causes
> PR 22567.  How is the patch below?

That's fine with me.  I guess we end up with the wrong section
in the function descriptor / PPC64 case (".opd" instead of some kind
of  ".text" where the resolved function lives), but it shouldn't
matter, since the old code did that as well, AFAICT.

(I noticed that get_sal_arch doesn't consider sal.objfile, probably
because it predates addition of the 'obfile' field.  We could probably
fill in / use that field more, but we don't need to do that now.)

Pedro Alves
  

On Fri, Dec 8, 2017 at 11:34 AM, Pedro Alves <palves@redhat.com> wrote:
> That's fine with me.  I guess we end up with the wrong section
> in the function descriptor / PPC64 case (".opd" instead of some kind
> of  ".text" where the resolved function lives), but it shouldn't
> matter, since the old code did that as well, AFAICT.

I tested the patch on gcc110, there is no regression.  I pushed it in.

>
> (I noticed that get_sal_arch doesn't consider sal.objfile, probably
> because it predates addition of the 'obfile' field.  We could probably
> fill in / use that field more, but we don't need to do that now.)

I noticed that too, but one thing I am not sure is that sal.objfile is
*only* used for probe,

  /* The probe associated with this symtab_and_line.  */
  probe *prob = NULL;
  /* If PROBE is not NULL, then this is the objfile in which the probe
     originated.  */
  struct objfile *objfile = NULL;

so can we use it in other cases (when probe is not used)?  I don't
know.  If so, are sal.objfile and sal.section->objfile different or same?
I need to look at the code there.