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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom Tromey <tom@tromey.com> [2020-03-17 14:18:43 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> +      /* If the previous entry is for the same line, at the same pc, and
> Andrew> +        is marked as a statement, while we are trying to add a
> Andrew> +        non-statement, then lets just drop this new non-statement.  */
> 
> I didn't try it yet, but I suspect it won't work, because the readelf
> output up-thread showed that the 2nd r.h line was not a statement:
> 
>     File name                            Line number    Starting address    View    Stmt
> [...]
>     r.h                                            6          0x80000042       2       x
>     r.h                                            6          0x80000042       3

OK, but prior to the is-stmt patches GDB didn't track any
non-statement line table entries, so my thinking was that previously
we were spotting two sequential entries that were marked as
statements.  This led me to think that deleting duplicate
non-statement entries would be fine.

With current HEAD of GDB the line table looks like this:

  (gdb) maintenance info line-table 
  objfile: /home/andrew/tmp/r.x ((struct objfile *) 0x2ac3b90)
  compunit_symtab: ((struct compunit_symtab *) 0x2aec530)
  symtab: /home/andrew/tmp/r.c ((struct symtab *) 0x2aec5b0)
  linetable: ((struct linetable *) 0x2b861d0):
  INDEX    LINE ADDRESS IS-STMT
  0         END 0x0000000000010150 Y
  1           8 0x0000000000010154
  2         END 0x0000000000010158 Y
  objfile: /home/andrew/tmp/r.x ((struct objfile *) 0x2ac3b90)
  compunit_symtab: ((struct compunit_symtab *) 0x2aec530)
  symtab: /home/andrew/tmp/r.h ((struct symtab *) 0x2aec5e0)
  linetable: ((struct linetable *) 0x2b86190):
  INDEX    LINE ADDRESS IS-STMT
  0           6 0x0000000000010150 Y
  1         END 0x0000000000010154 Y

If we completely remove the hunk that you identified then the line
table becomes:

  (gdb) maintenance info line-table
  objfile: /home/andrew/tmp/r.x ((struct objfile *) 0x3040bd0)
  compunit_symtab: ((struct compunit_symtab *) 0x306d2e0)
  symtab: /home/andrew/tmp/r.c ((struct symtab *) 0x306d360)
  linetable: ((struct linetable *) 0x31012c0):
  INDEX    LINE ADDRESS IS-STMT
  0         END 0x0000000000010150 Y
  1           8 0x0000000000010154
  2         END 0x0000000000010158 Y
  objfile: /home/andrew/tmp/r.x ((struct objfile *) 0x3040bd0)
  compunit_symtab: ((struct compunit_symtab *) 0x306d2e0)
  symtab: /home/andrew/tmp/r.h ((struct symtab *) 0x306d390)
  linetable: ((struct linetable *) 0x3101270):
  INDEX    LINE ADDRESS IS-STMT
  0           6 0x0000000000010150 Y
  1           6 0x0000000000010150
  2         END 0x0000000000010154 Y

Interestingly, without any of the is-stmt patches the GDB's line table
looks like this:

  (gdb) maintenance info line-table
  objfile: /home/andrew/tmp/r.x ((struct objfile *) 0x15da130)
  compunit_symtab: ((struct compunit_symtab *) 0x15ca5c0)
  symtab: /home/andrew/tmp/r.c ((struct symtab *) 0x15ca640)
  linetable: ((struct linetable *) 0x1675df0):
  INDEX    LINE ADDRESS
  0           0 0x0000000000010150
  objfile: /home/andrew/tmp/r.x ((struct objfile *) 0x15da130)
  compunit_symtab: ((struct compunit_symtab *) 0x15ca5c0)
  symtab: /home/andrew/tmp/r.h ((struct symtab *) 0x15ca680)
  linetable: ((struct linetable *) 0x1675dc0):
  INDEX    LINE ADDRESS
  0           6 0x0000000000010150

So what happens is we run into this bit of code in
skip_prologue_using_sal:

      /* If there is only one sal that covers the entire function,
	 then it is probably a single line function, like
	 "foo(){}".  */
      if (prologue_sal.end >= end_pc)
	return 0;

So, it looks like we have two different parts of the is-stmt change
playing off against each other, the extra END marker is what changes
the function from a single entry line table to a multi-entry one, this
is what breaks thinks.  Then when you remove the hunk you identified
this causes the prologue skipping to spot the dual entry case and we
do the right thing again.

It feels like this is more luck than design.

I need to revisit the code that added the extra END marker and
convince myself again that this is the right thing to do I think.

Hopefully I'll have an update tomorrow.

Thanks,
Andrew