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

[Tom de Vries <tdevries@suse.de>]

On 4/25/24 19:05, Carl Love wrote:
> 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!


Hi Carl,

yes, this is fine, thanks.

- Tom

> 
>                         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
>>>            }
>>>        }
>>