Re: [RFC] [gdb/testsuite] Add xfail in gdb.base/hbreak.exp

[Luis Machado <luis.machado@arm.com>]

On 7/24/24 06:25, Tom de Vries wrote:
> On 7/23/24 12:02, Luis Machado wrote:
>> On 7/17/24 16:14, Luis Machado wrote:
>>> On 7/17/24 16:10, Tom de Vries wrote:
>>>> On an aarch64-linux system with 32-bit userland running in a chroot, and using
>>>> target board unix/mthumb I get:
>>>> ...
>>>> (gdb) hbreak hbreak.c:27^M
>>>> Hardware assisted breakpoint 2 at 0x4004e2: file hbreak.c, line 27.^M
>>>
>>> That is a bit odd, but it goes through the compat layer, which is not exercised
>>> as often as the 32-bit code.
>>>
>>> Let me see if I can reproduce this one on my end.
>>
>> I managed to reproduce this. I checked with the kernel folks and this should
>> work, but I'm not sure where the error is coming from.
>>
> 
> Hi Luis,
> 
> thanks for looking into this, and the approval, committed.
> 
> Are you or the kernel folks following up on this, in terms of a linux kernel PR or some such?  It would be nice to add some sort of reference to the xfail.

It's in my TODO. I'm still investigating to understand where the error is coming from. Once located, I plan to check with them for their thoughts and a possible
fix. I don't think the kernel folks use the PR process much. We could probably ammend this commit later on once we have more information though.

> 
> Thanks,
> - Tom
> 
>>>
>>>
>>>> (gdb) PASS: gdb.base/hbreak.exp: hbreak
>>>> continue^M
>>>> Continuing.^M
>>>> Unexpected error setting breakpoint: Invalid argument.^M
>>>> (gdb) FAIL: gdb.base/hbreak.exp: continue to break-at-exit after hbreak
>>>> ...
>>>> due to this call in arm_linux_nat_target::low_prepare_to_resume:
>>>> ...
>>>>            if (ptrace (PTRACE_SETHBPREGS, pid,
>>>>                (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
>>>>              perror_with_name (_("Unexpected error setting breakpoint"));
>>>> ...
>>>>
>>>> This problem does not happen if instead we use a 4-byte aligned address.
>>>>
>>>> I'm not sure if this is simply unsupported or if there's a kernel bug of some
>>>> sort, but I don't see what gdb can do about this.
>>>>
>>>> Tentatively mark this as xfail.
>>>>
>>>> Tested on aarch64-linux.
>>>> ---
>>>>   gdb/testsuite/gdb.base/hbreak.exp | 40 ++++++++++++++++++++++++++-----
>>>>   1 file changed, 34 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/gdb/testsuite/gdb.base/hbreak.exp b/gdb/testsuite/gdb.base/hbreak.exp
>>>> index 73a3e2afb67..b140257a23e 100644
>>>> --- a/gdb/testsuite/gdb.base/hbreak.exp
>>>> +++ b/gdb/testsuite/gdb.base/hbreak.exp
>>>> @@ -27,10 +27,38 @@ if ![runto_main] {
>>>>     set breakline [gdb_get_line_number "break-at-exit"]
>>>>   -gdb_test "hbreak ${srcfile}:${breakline}" \
>>>> -     "Hardware assisted breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*${srcfile}, line ${breakline}\\." \
>>>> -     "hbreak"
>>>> +set re_loc "file \[^\r\n\]*$srcfile, line $breakline"
>>>> +set re_dot [string_to_regexp .]
>>>>   -gdb_test "continue" \
>>>> -     "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \
>>>> -     "continue to break-at-exit after hbreak"
>>>> +set addr 0x0
>>>> +gdb_test_multiple "hbreak ${srcfile}:${breakline}" "hbreak" {
>>>> +    -re -wrap "Hardware assisted breakpoint $decimal at ($hex): $re_loc$re_dot" {
>>>> +    set addr $expect_out(1,string)
>>>> +    pass $gdb_test_name
>>>> +    }
>>>> +}
>>>> +
>>>> +set have_xfail 0
>>>> +if { [istarget arm*-*-*] } {
>>>> +    # When running 32-bit userland on aarch64 kernel, thumb instructions that
>>>> +    # are not 4-byte aligned may not be supported for setting a hardware
>>>> +    # breakpoint on.
>>>> +    set have_xfail [expr ($addr & 0x2) == 2]
>>>> +}
>>>> +
>>>> +set re_xfail \
>>>> +    [string_to_regexp \
>>>> +     "Unexpected error setting breakpoint: Invalid argument."]
>>>> +
>>>> +gdb_test_multiple "continue" "continue to break-at-exit after hbreak" {
>>>> +    -re -wrap "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" {
>>>> +    pass $gdb_test_name
>>>> +    }
>>>> +    -re -wrap $re_xfail {
>>>> +    if { $have_xfail } {
>>>> +        xfail $gdb_test_name
>>>> +    } else {
>>>> +        fail $gdb_test_name
>>>> +    }
>>>> +    }
>>>> +}
>>>>
>>>> base-commit: 0ed152c5c6b3c72fc505b331ed77e08b438d643a
>>>
>>
>> In any case, I agree gdb doesn't have a better way to deal with it.
>>
>> Approved-By: Luis Machado <luis.machado@arm.com>
>