Re: [PATCH v4 1/2] Change calculation of frame_id by amd64 epilogue unwinder

[Simon Marchi <simark@simark.ca>]

On 10/25/22 10:13, Bruno Larsen via Gdb-patches wrote:
> On 25/10/2022 15:59, Simon Marchi wrote:
>> On 10/25/22 09:44, Simon Marchi wrote:
>>> On 10/5/22 06:38, Bruno Larsen via Gdb-patches wrote:
>>>> When GDB is stopped at a ret instruction and no debug information is
>>>> available for unwinding, GDB defaults to the amd64 epilogue unwinder, to
>>>> be able to generate a decent backtrace. However, when calculating the
>>>> frame id, the epilogue unwinder generates information as if the return
>>>> instruction was the whole frame.
>>>>
>>>> This was an issue especially when attempting to reverse debug, as GDB
>>>> would place a step_resume_breakpoint from the epilogue of a function if
>>>> we were to attempt to skip that function, and this breakpoint should
>>>> ideally have the current function's frame_id to avoid other problems
>>>> such as PR record/16678.
>>>>
>>>> This commit changes the frame_id calculation for the amd64 epilogue,
>>>> so that it is always the same as the dwarf2 unwinder's frame_id.
>>>>
>>>> It also adds a test to confirm that the frame_id will be the same,
>>>> regardless of using the epilogue unwinder or not, thanks to Andrew
>>>> Burgess.
>>>>
>>>> Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
>>>> ---
>>>>   gdb/amd64-tdep.c                              |  10 +-
>>>>   .../gdb.base/unwind-on-each-insn-foo.c        |  22 +++
>>>>   gdb/testsuite/gdb.base/unwind-on-each-insn.c  |  25 +++
>>>>   .../gdb.base/unwind-on-each-insn.exp          | 154 ++++++++++++++++++/usr/lib/x86_64-linux-gnu/libasan.so.6
>>>>   4 files changed, 206 insertions(+), 5 deletions(-)
>>>>   create mode 100644 gdb/testsuite/gdb.base/unwind-on-each-insn-foo.c
>>>>   create mode 100644 gdb/testsuite/gdb.base/unwind-on-each-insn.c
>>>>   create mode 100644 gdb/testsuite/gdb.base/unwind-on-each-insn.exp
>>> Hi Bruno,
>>>
>>> On Ubuntu 22.04, I can get this new test to fail quite reliably with:
>>>
>>>      $ taskset -c 1 make check TESTS="gdb.base/unwind-on-each-insn.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
>>>
>>> Can you give it a try?
>>>
>>> Simon
>> Actually, I took the time to look into it, it turns out the problem is
>> simple.  Here's a patch below.
> Ah great! I didn't even have time to finish making an Ubuntu VM lol. I just noticed one typo:

99% of the time, when needing to test on a specific version of a
specific distro, it is sufficient to use Docker (or whatever)
containers.  It's much, much faster than installing a VM from scratch.

>>  From 7090bf701b2f1cca89985ea1b45b0a2e3859e19e Mon Sep 17 00:00:00 2001
>> From: Simon Marchi <simon.marchi@efficios.com>
>> Date: Tue, 25 Oct 2022 09:50:56 -0400
>> Subject: [PATCH] gdb/testsuite: make sure to consume the prompt in
>>   gdb.base/unwind-on-each-insn.exp
>>
>> This test fails quite reliably for me when ran as:
>>
>>      $ taskset -c 1 make check TESTS="gdb.base/unwind-on-each-insn.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
>>
>> or more simply:
>>
>>      $ make check-read1 TESTS="gdb.base/unwind-on-each-insn.exp"
>>
>> The problem is that the that grabs the frame id from "maint print
> 
> Missing word here. "The problem is that the __proc__" ?

I think I meant "the gdb_test_multiple", thanks for pointing it out.

> With this fixed, LGTM.
> 
> Reviewed-By: Bruno Larsen <blarsen@redhat.com>

Thanks, will push.

Simon