Re: [patchv2] Do not skip prologue for asm (.S) files

[Doug Evans <dje at google dot com>]

Jan Kratochvil writes:
 > Hi Doug,
 >
 > Sergio has found a regression on ppc64 so I had to patch another  
function.
 > Now on ppc64:
 >
 > +Running  
/root/jkratoch/redhat/gdb-clean/gdb/testsuite/gdb.arch/amd64-prologue-skip.exp ...
 > -PASS: gdb.base/break.exp: run until function breakpoint, optimized file
 > +PASS: gdb.base/break.exp: run until function breakpoint, optimized file  
(code motion)
 > -PASS: gdb.threads/attach-many-short-lived-threads.exp: iter 3: attach
 > +XFAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 3: attach  
(EPERM)
 >
 > Jan
 > gdb/ChangeLog
 > 2015-06-25  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 for COMPUNIT_LOCATIONS_VALID.
 > 	* symtab.c (find_function_start_sal): Likewise.

Hmmm, so there's an undocumented requirement that minsym_found
and find_function_start_sal work compatibly?
[Which is actually not surprising if you know how linespecs and
breakpoints: we find all the matching minsyms and fullsyms
and then throw away the duplicates. But if these two functions
behave differently then the search for duplicates fails. At least
I'm guessing that's what happened here.]

Fortunately the check for SYMTAB_LANGUAGE should work here too.
Can you do that, plus add a comment to both minsym_found
and find_function_start_sal stating that we rely on them
working compatibly.

Thanks.

 > 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.
 >
 > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
 > index 496b74f..8bfd034 100644
 > --- a/gdb/dwarf2read.c
 > +++ b/gdb/dwarf2read.c
 > @@ -8094,7 +8094,7 @@ process_full_comp_unit (struct dwarf2_per_cu_data  
*per_cu,
 >  	 Still one can confuse GDB by using non-standard GCC compilation
 >  	 options - this waits on GCC PR other/32998 (-frecord-gcc-switches).
 >  	 */
 > -      if (cu->has_loclist && gcc_4_minor >= 5)
 > +      if ((cu->has_loclist && gcc_4_minor >= 5) || cu->language ==  
language_asm)
 >  	cust->locations_valid = 1;
 >
 >        if (gcc_4_minor >= 5)
 > diff --git a/gdb/linespec.c b/gdb/linespec.c
 > index d2089b5..e5b4c56 100644
 > --- a/gdb/linespec.c
 > +++ b/gdb/linespec.c
 > @@ -3454,7 +3454,22 @@ minsym_found (struct linespec_state *self, struct  
objfile *objfile,
 >      sal = find_pc_sect_line (pc, NULL, 0);
 >
 >    if (self->funfirstline)
 > -    skip_prologue_sal (&sal);
 > +    {
 > +      if (sal.symtab != NULL
 > +	  && COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab)))
 > +	{
 > +	  /* If gdbarch_convert_from_func_ptr_addr does not apply then
 > +	     sal.SECTION, sal.LINE&co. will stay correct from above.
 > +	     If gdbarch_convert_from_func_ptr_addr applies then
 > +	     sal.SECTION is cleared from above and sal.LINE&co. will
 > +	     stay correct from the last find_pc_sect_line above.  */
 > +	  sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
 > +	  sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
 > +						       &current_target);
 > +	}
 > +      else
 > +	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);
 > diff --git a/gdb/symtab.c b/gdb/symtab.c
 > index 6693930..43840ab 100644
 > --- a/gdb/symtab.c
 > +++ b/gdb/symtab.c
 > @@ -3617,6 +3617,13 @@ find_function_start_sal (struct symbol *sym, int  
funfirstline)
 >    section = SYMBOL_OBJ_SECTION (symbol_objfile (sym), sym);
 >    sal = find_pc_sect_line (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)),  
section, 0);
 >
 > +  if (funfirstline && sal.symtab != NULL
 > +      && COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab)))
 > +    {
 > +      sal.pc = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
 > +      return sal;
 > +    }
 > +
 >    /* We always should have a line for the function start address.
 >       If we don't, something is odd.  Create a plain SAL refering
 >       just the PC and hope that skip_prologue_sal (if requested)