On 1/4/21 5:27 PM, Bernd Edlinger wrote:
> On 1/4/21 2:59 PM, Tom de Vries wrote:
>> Hi,
>>
>> While working on PR26935 I noticed that when running test-case
>> gdb.base/morestack.exp with target board unix/-m32/-fPIE/-pie and ld linker,
>> I get this linetable fragment for morestack.S using readelf -wL:
>> ...
>> CU: ../../../../libgcc/config/i386/morestack.S:
>> Line number    Starting address    View    Stmt
>> 109               0xc9c               x
>>   ...
>> 838               0xe03               x
>>   -               0xe04
>>
>> 636                   0               x
>> 637                 0x3               x
>>   -                 0x4
>> ...
>> but with "maint info line-table" I get:
>> ...
>> INDEX  LINE   ADDRESS            IS-STMT
>> 0      END    0x00000004         Y
>> 1      109    0x00000c9c         Y
>>   ...
>> 110    838    0x00000e03         Y
>> 111    END    0x00000e04         Y
>> ...
>>
>> So, apparently the entries with addresses 0x0 and 0x3 are filtered out
>> because the addresses are out of range, but the same doesn't happen with the
>> end-of-seq terminator.
>>
>> Fix this by filtering out end-of-seq terminators that do not actually
>> terminate anything.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
>>
>> [gdb/symtab] Remove superfluous end-of-sequence marker
>>
>> gdb/ChangeLog:
>>
>> 2021-01-04  Tom de Vries  <tdevries@suse.de>
>>
>> 	* buildsym.c (buildsym_compunit::record_line): Filter out end-of-seq
>> 	terminators that do not terminate anything.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2021-01-04  Tom de Vries  <tdevries@suse.de>
>>
>> 	* gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: New file.
>>
>> ---
>>  gdb/buildsym.c                                     |  8 ++
>>  .../gdb.dwarf2/dw2-out-of-range-end-of-seq.exp     | 94 ++++++++++++++++++++++
>>  2 files changed, 102 insertions(+)
>>
>> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
>> index 1d8c579b7b8..245aecf0c38 100644
>> --- a/gdb/buildsym.c
>> +++ b/gdb/buildsym.c
>> @@ -714,6 +714,14 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
>>  	    break;
>>  	  subfile->line_vector->nitems--;
>>  	}
>> +
>> +      /* Ignore an end-of-sequence marker marking an empty sequence.  */
>> +      struct linetable_entry *last
>> +	= (subfile->line_vector->nitems == 0
>> +	   ? nullptr
>> +	   : &subfile->line_vector->item[subfile->line_vector->nitems - 1]);
>> +      if (last == nullptr || last->line == 0)
>> +	return;
>>      }
>>  
> 
> What you want to achieve here can probably be done more easily this way:
> 
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 1d8c579..dda83f7 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -710,6 +710,8 @@ struct blockvector *
>        while (subfile->line_vector->nitems > 0)
>         {
>           e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
> +         if (e->line == 0)
> +           return;
>           if (e->pc != pc)
>             break;
>           subfile->line_vector->nitems--;
> 
> 

Hi Bernd,

Thanks for the review.

Indeed, that's less verbose than the current patch, but to me it feels
more convoluted.  I'd rather keep the additional test out of the loop.

I did realize because of your comment that in both cases we're dealing
with the last entry in the table, so I've rewritten the patch to use a
last variable in the loop, and to reuse it afterwards.

> I wonder however where the out-of-range line entries are filtered away.
> Would it be better to filter the end of sequence markers there?

I'm also curious about that, but I don't think it matters much.

Committed to trunk as attached.

Thanks,
- Tom