Re: [PATCH] [gdb/testsuite] Fix gdb.base/inline-frame-cycle-unwind.exp for s390x

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

On 12/9/25 8:53 PM, Simon Marchi wrote:
> On 12/8/25 2:53 PM, Kevin Buettner wrote:
>> Ping.
>>
>> On Sat,  1 Nov 2025 21:46:51 -0700
>> Kevin Buettner <kevinb@redhat.com> wrote:
>>
>>> This commit fixes six failures for s390x due to a fundamental
>>> difference in unwinding behavior between s390x and other
>>> architectures:
>>>
>>> FAIL: gdb.base/inline-frame-cycle-unwind.exp: bt: cycle at level 5:
>>>    backtrace when the unwind is broken at frame 5
>>> FAIL: gdb.base/inline-frame-cycle-unwind.exp: bt: cycle at level 3:
>>>    backtrace when the unwind is broken at frame 3
>>> FAIL: gdb.base/inline-frame-cycle-unwind.exp: bt: cycle at level 1:
>>>    backtrace when the unwind is broken at frame 1
>>> FAIL: gdb.base/inline-frame-cycle-unwind.exp: bt -no-filters: cycle at level 5:
>>>    backtrace when the unwind is broken at frame 5
>>> FAIL: gdb.base/inline-frame-cycle-unwind.exp: bt -no-filters: cycle at level 3:
>>>    backtrace when the unwind is broken at frame 3
>>> FAIL: gdb.base/inline-frame-cycle-unwind.exp: bt -no-filters: cycle at level 1:
>>>    backtrace when the unwind is broken at frame 1
>>>
>>> The core issue is that on s390x, the Canonical Frame Address (CFA) for
>>> a function points *into the caller's stack frame*, whereas on x86_64
>>> or aarch64 the CFA points *within the current function's frame*.  This
>>> architectural difference causes cycle detection to occur later on
>>> s390x.
>>>
>>> The patch resolves this by:
>>> - Making expected backtrace output architecture-specific.
>>> - For non-s390x targets: expecting the full set of frames up to the
>>>    specified level.
>>> - For s390x: expecting fewer frames before detecting the cycle
>>>    (e.g., level 5 shows 3 frames instead of 5).
>>> - Skipping the cycle at level 1 test entirely on s390x since it cannot
>>>    be detected at that frame.
>>>
>>> Tested using recent Fedora releases on s390x, x86_64, and aarch64.
> 
> I can't comment on the s390x-specific details, but code-wise the patch
> LGTM.  Tom de Vries might have an opinion, he was doing some s390x fixes
> recently.
> 

After playing around with printf debugging the test-case on s390x-linux 
and x86_64-linux side by side, my understanding is that the test-case 
fails because it doesn't manage to correctly calculate frame-ids for 
s390x, which can easily be fixed by using "maint print frame-id".

I've got a proposal for an alternative patch ( 
https://sourceware.org/pipermail/gdb-patches/2025-December/223355.html ).

Thanks,
- Tom

> Some nits below.
> 
>>> ---
>>>   .../gdb.base/inline-frame-cycle-unwind.exp    | 81 +++++++++++++------
>>>   1 file changed, 57 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp b/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
>>> index 7fc47af624f..ccd86eb79ab 100644
>>> --- a/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
>>> +++ b/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.exp
>>> @@ -92,15 +92,31 @@ foreach bt_cmd { "bt" "bt -no-filters" } {
>>>   	    gdb_test_no_output "python stop_at_level=5"
>>>   	    gdb_test "maint flush register-cache" \
>>>   		"Register cache flushed\\."
>>> +
>>> +	    # Frames expected on all targets
>>> +	    set exp [list \
>>> +		"#0 \[^\r\n\]* inline_func \\(\\) at \[^\r\n\]*" \
>>> +		"#1 \[^\r\n\]* normal_func \\(\\) at \[^\r\n\]*" \
>>> +		"#2 \[^\r\n\]* inline_func \\(\\) at \[^\r\n\]*" \
>>> +		"#3 \[^\r\n\]* normal_func \\(\\) at \[^\r\n\]*"]
>>> +
>>> +	    # Additional frames required on non-s390x targets
>>> +	    if {![istarget "s390x*-*-*"]} {
>>> +		lappend exp \
>>> +		    "#4 \[^\r\n\]* inline_func \\(\\) at \[^\r\n\]*" \
>>> +		    "#5 \[^\r\n\]* normal_func \\(\\) at \[^\r\n\]*"
>>> +	    }
>>> +	    #
>>> +	    # The final line that should appear for every target
> 
> Unexpected `#` by itself above.
> 
>>> @@ -108,25 +124,42 @@ foreach bt_cmd { "bt" "bt -no-filters" } {
>>>   	    gdb_test_no_output "python stop_at_level=3"
>>>   	    gdb_test "maint flush register-cache" \
>>>   		"Register cache flushed\\."
>>> +	    # Frames expected on all targets
>>> +	    set exp [list \
>>> +		"#0 \[^\r\n\]* inline_func \\(\\) at \[^\r\n\]*" \
>>> +		"#1 \[^\r\n\]* normal_func \\(\\) at \[^\r\n\]*"]
>>> +
>>> +	    # Additional frames required on non-s390x targets
>>> +	    if {![istarget "s390x*-*-*"]} {
>>> +		lappend exp \
>>> +		    "#2 \[^\r\n\]* inline_func \\(\\) at \[^\r\n\]*" \
>>> +		    "#3 \[^\r\n\]* normal_func \\(\\) at \[^\r\n\]*"
>>> +	    }
>>> +	    #
>>> +	    # The final line that should appear for every target
> 
> Likewise.
> 
> Simon