Do not skip prologue for asm (.S) files

  Jan Kratochvil writes:
  > On Tue, 23 Jun 2015 22:35:01 +0200, Jan Kratochvil wrote:
  > > On Tue, 23 Jun 2015 01:46:08 +0200, Doug Evans wrote:
  > > > static void
  > > > minsym_found (struct linespec_state *self, struct objfile *objfile,
  > > >               struct minimal_symbol *msymbol,
  > > >               struct symtabs_and_lines *result)
  > > > {
  > > >   struct gdbarch *gdbarch = get_objfile_arch (objfile);
  > > >   CORE_ADDR pc;
  > > >   struct symtab_and_line sal;
  > > >
  > > >   sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
  > > >                            (struct obj_section *) 0, 0);
  > > >   sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
  > > >
  > > >   /* 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);
  > > >
  > > >   if (self->funfirstline)
  > > >     skip_prologue_sal (&sal);
  > > >
  > > >   if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc))
  > > >     add_sal_to_sals (self, result, &sal, MSYMBOL_NATURAL_NAME  
(msymbol), 0);
  > > > }
  > > >
  > > > With the patch added, we're using the value of
  > > > MSYMBOL_VALUE_ADDRESS twice
  > > > and calling gdbarch_convert_from_func_ptr_addr twice.
  > > > [I'm not micro-optimizing here, my goal is code readability.]
  > > >
  > > > Plus the patch does:
  > > >
  > > >       sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
  > > >       sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
  > > >                                                   &current_target);
  > > >
  > > > but it doesn't update sal.section nor sal.line.
  > >
  > > OK, I agree that seems wrong.
  >
  > I do not agree, it seems correct to me.

I was wondering if things were correct, not stating they weren't.

  > I have only added a comment to the code.  Is it enough this way?

The comment helps, thanks.
I'm still uncomfortable with setting locations_valid for assembler.
The flag may get used for more things in the future, and this feels
like asking for trouble.
Fortunately, I think there's a better way: instead of setting  
locations_valid
check the language at the place where we care.

      add_sal_to_sals (self, result, &sal, MSYMBOL_NATURAL_NAME (msymbol),  
0);


[and leave dwarf2read.c as is]

Ok with this change.
Thanks!

  > I am sorry I cannot write it much cleanly as the data structures and  
functions
  > involved are not much clean.
  >
  >
  > Jan
  > gdb/ChangeLog
  > 2015-06-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
  >
  > 	* dwarf2read.c (process_full_comp_unit): Set LOCATIONS_VALID also for
  > 	language_asm.
  > 	* linespec.c (minsym_found): Reset sal.PC forCOMPUNIT_LOCATIONS_VALID.
  >
  > gdb/testsuite/ChangeLog
  > 2015-06-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
  >
  > 	* gdb.arch/amd64-prologue-skip.S: New file.
  > 	* gdb.arch/amd64-prologue-skip.exp: New file.
  >
  

On Thu, 25 Jun 2015 22:30:14 +0200, Doug Evans wrote:
> Jan Kratochvil writes:
>  > On Tue, 23 Jun 2015 22:35:01 +0200, Jan Kratochvil wrote:
>  > > On Tue, 23 Jun 2015 01:46:08 +0200, Doug Evans wrote:
>  > > > but it doesn't update sal.section nor sal.line.
>  > >
>  > > OK, I agree that seems wrong.
>  >
>  > I do not agree, it seems correct to me.
> 
> I was wondering if things were correct, not stating they weren't.

Fine with that, just you have convinced me initially there is really a bug.


> Fortunately, I think there's a better way: instead of setting
> locations_valid check the language at the place where we care.

I am sure fine with that.

> Ok with this change.

Just there is now also [patchv2].


Thanks,
Jan