On Mon, 22 Jun 2015 00:52:54 +0200, Doug Evans wrote:
> On Sat, Jun 20, 2015 at 10:44 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > without debuginfo - correct address:
> > (gdb) b select
> > Breakpoint 1 at 0xf4210
> 
> Hi.  I'm having trouble understanding the discussion.
> 
> f4210 is the correct address?

Yes.  It is also where .dynsym+.symtab point to:
00000000000f4210 T __select
00000000000f4210 W select


> It would help to have more data here to understand why.
> [e.g., disassembly of entire function?]

It is attached. The important part is that for -O2 -g code the very first
instruction can jump arbitrarily, no matter what .debug_line says.
So the disassembly does not matter much, I cannot read much s390 anyway.


> > with debuginfo, either with or without the fix:
> > (gdb) b select
> > Breakpoint 1 at 0xf41c8: file ../sysdeps/unix/syscall-template.S, line 81.
> 
> The fix doesn't change the breakpoint address?

This is a typo from copy-pasting, it should have said:

with debuginfo, without the fix:

Sure "with the fix" it now puts the breakpoint at the correct address 0xf4210.


> > One part is to make 'locations_valid' true even for asm files.
> >   /* Symtab has been compiled with both optimizations and debug info so that
> >      GDB may stop skipping prologues as variables locations are valid already
> >      at function entry points.  */
> >   unsigned int locations_valid : 1;
> >
> > The other part is to extend the 'locations_valid' functionality more - I have
> > chosen mostly randomly this place.
> 
> Can you elaborate on this?
> The original code is simple and intuitive:
> 
>   if (self->funfirstline)
>     skip_prologue_sal (&sal);
> 
> skip_prologue_sal is pretty complicated itself, but I can read those
> two lines without fretting too much about whether they're correct.

The complications in skip_prologue_sal IMO do not work anymore plus they are
irrelevant as they are for various *_deprecated_* arch features.  But I do not
want to discuss it more or touch that so I just try to disable that code for
new compilers with correct debug info.


> Now it's got this extra code, and it's not clear to this reader why
> it's correct.
> 
>  if (self->funfirstline)
>    {
>      if (sal.symtab != NULL
>         && COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab)))
>       {
>         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);
>    }
> 
> This may be the wrong way to look at it, but one question that comes to mind is:
> Why can't this extra code go into skip_prologue_sal?

Because to read COMPUNIT_LOCATIONS_VALID one needs to find the symtab.
So one needs to map PC->SAL by find_pc_sect_line().  But then SAL.PC is
already "corrupted" by some .debug_line matching.

Maybe one could map find the symtab a different way but when the symtab was
already fetched in this code.


> > --- a/gdb/dwarf2read.c
> > +++ b/gdb/dwarf2read.c
> > @@ -8096,7 +8096,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;
> 
> How come this isn't written as:
> 
>   if (cu->has_loclist && (gcc_4_minor >= 5 || cu->language == language_asm))
> 
> ?

This conditional would not work.  language_asm always has has_loclist==0.
has_loclist says "This CU references .debug_loc." - language_asm CUs never
generate/reference .debug_loc.

The check 'gcc_4_minor >= 5' is there also only for C/C++ (as described in the
large comment there), not for language_asm.  For language_asm 'gcc_4_minor' is
always -1.


Jan