[v4,0/2] Improving frame printing with recursive calls

Message ID 20231107124150.116140-2-blarsen@redhat.com
Headers
Series Improving frame printing with recursive calls |

Message

Guinevere Larsen Nov. 7, 2023, 12:41 p.m. UTC
  This started with the first patch, just fixing PR record/29178. However,
Kevin pointed out that I could reuse some of the local variables to make
the function as a whole simpler, which I did on patch 2.
My changes on v3 introduced a regression in a dwarf2 test. I gave some
time for people to comment on Kevin's solution (then forgot about this
patch), but since no one disagreed, this new version changes thetest,
rather than the code.

Changelog for v4:
* changed gdb.dwarf2/dw2-out-of-range... because it was reporting a
  failure, but it is just some symbols being read earlier than before.

Changelog for v3:
* Fix remaining style issues in the testsuite

Changelog for v2:
* Added second patch in the series
* Fixed some tyle issues

Guinevere Larsen (2):
  gdb/record: print frame information when exiting a recursive call
  gdb/infrun: simplify process_event_stop_test

 gdb/infrun.c                                  | 36 ++++++++++-----
 .../dw2-out-of-range-end-of-seq.exp           |  2 +-
 gdb/testsuite/gdb.reverse/recursion.c         | 44 ++++++++++++++++++
 gdb/testsuite/gdb.reverse/recursion.exp       | 45 +++++++++++++++++++
 4 files changed, 114 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.c
 create mode 100644 gdb/testsuite/gdb.reverse/recursion.exp
  

Comments

Tom Tromey Nov. 17, 2023, 2:38 p.m. UTC | #1
>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere> This started with the first patch, just fixing PR record/29178. However,
Guinevere> Kevin pointed out that I could reuse some of the local variables to make
Guinevere> the function as a whole simpler, which I did on patch 2.
Guinevere> My changes on v3 introduced a regression in a dwarf2 test. I gave some
Guinevere> time for people to comment on Kevin's solution (then forgot about this
Guinevere> patch), but since no one disagreed, this new version changes thetest,
Guinevere> rather than the code.

These look reasonable to me.  Thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
  
Guinevere Larsen Nov. 20, 2023, 9:54 a.m. UTC | #2
On 17/11/2023 15:38, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
> Guinevere> This started with the first patch, just fixing PR record/29178. However,
> Guinevere> Kevin pointed out that I could reuse some of the local variables to make
> Guinevere> the function as a whole simpler, which I did on patch 2.
> Guinevere> My changes on v3 introduced a regression in a dwarf2 test. I gave some
> Guinevere> time for people to comment on Kevin's solution (then forgot about this
> Guinevere> patch), but since no one disagreed, this new version changes thetest,
> Guinevere> rather than the code.
>
> These look reasonable to me.  Thank you.
> Approved-By: Tom Tromey <tom@tromey.com>
>
Thanks, I pushed these!
  
Luis Machado Nov. 29, 2023, 11:42 a.m. UTC | #3
On 11/20/23 09:54, Guinevere Larsen wrote:
> On 17/11/2023 15:38, Tom Tromey wrote:
>>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
>> Guinevere> This started with the first patch, just fixing PR record/29178. However,
>> Guinevere> Kevin pointed out that I could reuse some of the local variables to make
>> Guinevere> the function as a whole simpler, which I did on patch 2.
>> Guinevere> My changes on v3 introduced a regression in a dwarf2 test. I gave some
>> Guinevere> time for people to comment on Kevin's solution (then forgot about this
>> Guinevere> patch), but since no one disagreed, this new version changes thetest,
>> Guinevere> rather than the code.
>>
>> These look reasonable to me.  Thank you.
>> Approved-By: Tom Tromey <tom@tromey.com>
>>
> Thanks, I pushed these!
> 

Sorry I'm late to the party, but looks like this regresses reverse debugging. I was just
testing Carl's patches for step/next reverse fixes, and I notice a few new FAIL's that
were not related to the fixes by Carl.

I don't see a problem with aarch64-linux, but arm-linux seems to be affected.

It was caught by the Linaro CI here: https://linaro.atlassian.net/browse/GNU-1026

Do you have any ideas on what's up?
  
Guinevere Larsen Nov. 29, 2023, 12:09 p.m. UTC | #4
On 29/11/2023 12:42, Luis Machado wrote:
> On 11/20/23 09:54, Guinevere Larsen wrote:
>> On 17/11/2023 15:38, Tom Tromey wrote:
>>>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
>>> Guinevere> This started with the first patch, just fixing PR record/29178. However,
>>> Guinevere> Kevin pointed out that I could reuse some of the local variables to make
>>> Guinevere> the function as a whole simpler, which I did on patch 2.
>>> Guinevere> My changes on v3 introduced a regression in a dwarf2 test. I gave some
>>> Guinevere> time for people to comment on Kevin's solution (then forgot about this
>>> Guinevere> patch), but since no one disagreed, this new version changes thetest,
>>> Guinevere> rather than the code.
>>>
>>> These look reasonable to me.  Thank you.
>>> Approved-By: Tom Tromey <tom@tromey.com>
>>>
>> Thanks, I pushed these!
>>
> Sorry I'm late to the party, but looks like this regresses reverse debugging. I was just
> testing Carl's patches for step/next reverse fixes, and I notice a few new FAIL's that
> were not related to the fixes by Carl.
>
> I don't see a problem with aarch64-linux, but arm-linux seems to be affected.
>
> It was caught by the Linaro CI here: https://linaro.atlassian.net/browse/GNU-1026
>
> Do you have any ideas on what's up?
>
I have no idea what's up. I'll take a look at it, thanks for bringing it up