Re: [PATCHv2 2/2] gdb: Add support for tracking the DWARF line table is-stmt field

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 3/16/20 9:57 PM, Tom Tromey wrote:
> Hi.
> 
> This particular hunk causes a regression on an internal test case for a
> RISC-V target:
> 
> Andrew> +  /* If we have a duplicate for the previous entry then ignore the new
> Andrew> +     entry, except, if the new entry is setting the is_stmt flag, then
> Andrew> +     ensure the previous entry respects the new setting.  */
> Andrew> +  e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
> Andrew> +  if (e->line == line && e->pc == pc)
> Andrew> +    {
> Andrew> +      if (is_stmt && !e->is_stmt)
> Andrew> +	e->is_stmt = 1;
> Andrew> +      return;
> Andrew> +    }
> 

I don't think this whole if statement is really needed.
I looks more sane this way, but everything seems to work also without.


> Now, I'm not 100% certain that this is a "true" regression.  The test
> case in question already has a large number of XFAILs for various
> targets, because in practice gdb reports different stop locations for
> different compilers.
> 
> It's just 2 short files so I can share it here:
> 
>     /* r.c */
>     #include "r.h"
> 
>     int
>     main ()
>     {
>       callee ();
>     }
> 
>     /* r.h */
>     int counter = 42;
> 
>     inline void
>     callee () {
>       counter = 0;  /* break here */
>     }
> 
> When I look at this with readelf I see:
> 
>     $ readelf --debug-dump=decodedline ./r 
>     Contents of the .debug_line section:
> 
>     r.c:
>     File name                            Line number    Starting address    View    Stmt
>     r.c                                            6          0x80000042               x
>     r.c                                            7          0x80000042       1       x
> 
>     r.h:
>     r.h                                            6          0x80000042       2       x
>     r.h                                            6          0x80000042       3
> 
>     r.c:
>     r.c                                            8          0x8000004a       4
>     r.c                                            8          0x8000004e       5
> 
> I can send an executable if you want.
> 
> If I run gdb on the resulting program and "break r.h:6", before the
> is-statement patch it reports r.h:6, but afterward it says:
> 
>     (gdb) b r.h:6
>     Breakpoint 1 at 0x8000004a: file r.c, line 8.
> 
> Backing out the hunk in question up above is enough to fix the problem.
> 
> What is happening is that gdb is no longer recognizing the special
> prologue rule in skip_prologue_sal:
> 
>       /* For languages other than assembly, treat two consecutive line
> 	 entries at the same address as a zero-instruction prologue.
> 	 The GNU assembler emits separate line notes for each instruction
> 	 in a multi-instruction macro, but compilers generally will not
> 	 do this.  */
> 
> TBH this has always seemed like a big hack, at least in the DWARF era.
> But, I suppose it's a hack we continue to need, at least until GCC emits
> proper prologue end notes.
> 
> Just removing the "return" is enough to make it work for me.  I didn't
> try a real test run, though.
> 
> I'd appreciate any thoughts you have on this.
> 
> thanks,
> Tom
>