Re: [PATCH v2 3/4] [gdb/testsuite] Fix end_sequence addresses

[Carl Love <cel@linux.ibm.com>]

Tom:

Thanks for verifying the patch on X86.  I agree, including the entire commit message from your patch
is a little overkill.  I cut down your commit message and made the description of my fix a little better.
Here is what I now have for the commit message:

------------------------------------------------------
Fix end_sequence addresses for dw2-lines.exp

The patch:

  From f0d556d14b1d1c3f8e2f9c13b08adca22e1b8c9c Mon Sep 17 00:00:00 2001
  From: Tom de Vries <tdevries@suse.de>
  Date: Wed, 17 Apr 2024 12:55:00 +0200
  Subject: [PATCH] [gdb/testsuite] Fix end_sequence addresses

  I noticed in test-case gdb.reverse/map-to-same-line.exp, that the end of main:
  ...
  00000000004102c4 <end_of_sequence>:
    4102c4:       52800000        mov     w0, #0x0                        // #0
   4102c8:       9100c3ff        add     sp, sp, #0x30
    4102cc:       d65f03c0        ret
  ...
  is not described by the line table:
  ...

  <snip>


The regression failure on PowerPC is due to the change in file
dw2-lines.exp,

-               DW_LNE_set_address bar_label_5
+               DW_LNE_set_address "$main_start + $main_len"

The label bar_label_5 is in function bar, not function main.  The new
set address should have been $bar_start + $bar_len.

--------------------------------------------------------

Please let me know if that looks reasonable and I will then commit the fix.  Thanks!

                       Carl 


On 4/24/24 14:07, Tom de Vries wrote:
> On 4/24/24 20:28, Carl Love wrote:
>>
>> Tom:
>>
>> The following patch fixes the issue for me on PowerPC.  Please let me know if it looks ok and if the patch works on your systems as well.  Thanks.
>>
>>                          Carl
> 
> Hi Carl,
> 
> I don't see the FAILs you mentioned, but your patch is correct, and doesn't regress the test-case for me on x86_64-linux.
> 
> Thanks for finding, reporting and fixing this.
> 
> So, approved, but consider making the commit message shorter. Including the entire commit message of the patch you're fixing is a bit of an overkill.
> 
> Approved-By: Tom de Vries <tdevries@suse.de>
> 
> Thanks,
> - Tom
> 
>>
>> ----------------------------------------------------------------------
>>
>> Fix end_sequence addresses for dw2-lines.exp
>>
>> The patch:
>>
>>    From f0d556d14b1d1c3f8e2f9c13b08adca22e1b8c9c Mon Sep 17 00:00:00 2001
>>    From: Tom de Vries <tdevries@suse.de>
>>    Date: Wed, 17 Apr 2024 12:55:00 +0200
>>    Subject: [PATCH] [gdb/testsuite] Fix end_sequence addresses
>>
>>    I noticed in test-case gdb.reverse/map-to-same-line.exp, that the end of main:
>>    ...
>>    00000000004102c4 <end_of_sequence>:
>>      4102c4:       52800000        mov     w0, #0x0                        // #0
>>     4102c8:       9100c3ff        add     sp, sp, #0x30
>>      4102cc:       d65f03c0        ret
>>    ...
>>    is not described by the line table:
>>    ...
>>    File name                    Line number    Starting address    View    Stmt
>>      ...
>>    map-to-same-line.c                    54            0x4102ac               x
>>    map-to-same-line.c                     -            0x4102c4
>>    ...
>>
>>    Fix this by ending the line table at $main_end.
>>
>>    Likewise in a few other test-cases, found using:
>>    ...
>>    $ find gdb/testsuite/ -type f \
>>      | xargs grep -B1 DW_LNE_end_sequence \
>>      | grep set_address \
>>      | egrep -v "_end|_len"
>>    ...
>>
>>    Tested on aarch64-linux.
>>    ---
>>     gdb/testsuite/gdb.dwarf2/dw2-gas-workaround.exp         | 2 +-
>>     gdb/testsuite/gdb.dwarf2/dw2-inline-header-1.exp        | 2 +-
>>     gdb/testsuite/gdb.dwarf2/dw2-inline-header-2.exp        | 2 +-
>>     gdb/testsuite/gdb.dwarf2/dw2-inline-header-3.exp        | 2 +-
>>     gdb/testsuite/gdb.dwarf2/dw2-line-number-zero.exp       | 7 +++++--
>>     gdb/testsuite/gdb.dwarf2/dw2-lines.exp                  | 2 +-
>>     gdb/testsuite/gdb.dwarf2/dw2-vendor-extended-opcode.exp | 4 +++-
>>     gdb/testsuite/gdb.dwarf2/imported-unit-bp.exp.tcl       | 2 +-
>>     gdb/testsuite/gdb.reverse/map-to-same-line.c            | 1 -
>>     gdb/testsuite/gdb.reverse/map-to-same-line.exp          | 3 ++-
>>     10 files changed, 16 insertions(+), 11 deletions(-)
>>
>> The issue in file dw2-lines.exp is the change
>>
>>
>> -               DW_LNE_set_address bar_label_5
>> +               DW_LNE_set_address "$main_start + $main_len"
>>
>> The label bar_label_5 is in function bar, not function  main.  The new
>> set address should have been $bar_start + $bar_len.
>> ---
>>   gdb/testsuite/gdb.dwarf2/dw2-lines.exp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-lines.exp b/gdb/testsuite/gdb.dwarf2/dw2-lines.exp
>> index 85d5e9c8afb..af5b6b71768 100644
>> --- a/gdb/testsuite/gdb.dwarf2/dw2-lines.exp
>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-lines.exp
>> @@ -104,7 +104,7 @@ proc test_1 { _cv _cdw64 _lv _ldw64 {_string_form ""}} {
>>           line [line_for bar_label_4]
>>           DW_LNS_copy
>>   -        DW_LNE_set_address "$main_start + $main_len"
>> +        DW_LNE_set_address "$bar_start + $bar_len"
>>           DW_LNE_end_sequence
>>           }
>>       }
>