public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* GDB 13 release -- 2023-01-21 Update
@ 2023-01-21  6:08 Joel Brobecker
  2023-01-25 20:18 ` Torbjorn SVENSSON
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2023-01-21  6:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker

Hi everyone,

Quick update for the past week: We've inching closer, but we still
have one leftover regression which I think we should wait for before
we create the first pre-release. Kudos to Tom for his persistence
in hardening the new parallel DWARF reader.

Here is an updated status since last week:

Fixed Since the Previous Update:
--------------------------------

  < none >

Added Since the Last Update:
----------------------------

  < none >

Other Ongoing Items:
--------------------

  * [TomT] c++/29896
    GDB git doesn't recognize template function name
    https://sourceware.org/bugzilla/show_bug.cgi?id=29896

    Patches pushed, to both master and branch, but unfortunately
    a regression was found - Tom confirmed and is working on a fix.
    https://sourceware.org/pipermail/gdb-patches/2023-January/195828.html

  * [Torbjorn] tdep/29738
    Arm M-profile dwarf2 unwinder performance suffers from exponential growth
    https://sourceware.org/bugzilla/show_bug.cgi?id=29738

    patch v3, 2023-01-19, reviewed 2023-01-20:
    https://sourceware.org/pipermail/gdb-patches/2023-January/195915.html

Not Blocking, But Keep An Eye On:
---------------------------------

  * [TomT] build/29966
    Cannot build anymore for win32 thread model of gdb
    https://sourceware.org/bugzilla/show_bug.cgi?id=29966

-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-21  6:08 GDB 13 release -- 2023-01-21 Update Joel Brobecker
@ 2023-01-25 20:18 ` Torbjorn SVENSSON
  2023-01-27  6:30   ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Torbjorn SVENSSON @ 2023-01-25 20:18 UTC (permalink / raw)
  To: Joel Brobecker, gdb-patches



On 2023-01-21 07:08, Joel Brobecker via Gdb-patches wrote:
> Hi everyone,
> 
> Quick update for the past week: We've inching closer, but we still
> have one leftover regression which I think we should wait for before
> we create the first pre-release. Kudos to Tom for his persistence
> in hardening the new parallel DWARF reader.
> 
> Here is an updated status since last week:
> 
> Fixed Since the Previous Update:
> --------------------------------
> 
>    < none >
> 
> Added Since the Last Update:
> ----------------------------
> 
>    < none >
> 
> Other Ongoing Items:
> --------------------
> 
>    * [TomT] c++/29896
>      GDB git doesn't recognize template function name
>      https://sourceware.org/bugzilla/show_bug.cgi?id=29896
> 
>      Patches pushed, to both master and branch, but unfortunately
>      a regression was found - Tom confirmed and is working on a fix.
>      https://sourceware.org/pipermail/gdb-patches/2023-January/195828.html
> 
>    * [Torbjorn] tdep/29738
>      Arm M-profile dwarf2 unwinder performance suffers from exponential growth
>      https://sourceware.org/bugzilla/show_bug.cgi?id=29738
> 
>      patch v3, 2023-01-19, reviewed 2023-01-20:
>      https://sourceware.org/pipermail/gdb-patches/2023-January/195915.html

I just pushed this for master.
Is it okay to also push the 2 patches to gdb-13-branch?

Kind regards,
Torbjörn

> 
> Not Blocking, But Keep An Eye On:
> ---------------------------------
> 
>    * [TomT] build/29966
>      Cannot build anymore for win32 thread model of gdb
>      https://sourceware.org/bugzilla/show_bug.cgi?id=29966
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-25 20:18 ` Torbjorn SVENSSON
@ 2023-01-27  6:30   ` Joel Brobecker
  2023-01-27  6:38     ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2023-01-27  6:30 UTC (permalink / raw)
  To: Torbjorn SVENSSON, simon.marchi, tom
  Cc: Joel Brobecker, gdb-patches, luis.machado

Hello,

> >    * [Torbjorn] tdep/29738
> >      Arm M-profile dwarf2 unwinder performance suffers from exponential growth
> >      https://sourceware.org/bugzilla/show_bug.cgi?id=29738
> > 
> >      patch v3, 2023-01-19, reviewed 2023-01-20:
> >      https://sourceware.org/pipermail/gdb-patches/2023-January/195915.html
> 
> I just pushed this for master.
> Is it okay to also push the 2 patches to gdb-13-branch?

For the avoidance of doubt, my understanding is that we are talking
about the following two patches:

    commit d72ba177c85f2ad18d0dcabdd8844532c9acb819
    Author: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
    Date:   Thu Nov 17 12:17:53 2022 +0100
    Subject: gdb: dwarf2 generic implementation for caching function data

... and ...

    commit 5cf11483141a58314834653003e49709b47822d5
    Author: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
    Date:   Thu Nov 17 12:18:20 2022 +0100
    Subject: gdb/arm: Use new dwarf2 function cache

I hope I having missed any other patch!

The first one adds, as the subject indicates, a framework for
caching frame-related information, and the second patch takes
advantage of that framework,

Luis marked the corresponding PR as "important to fix", so
I'm assuming the impact if we do not backport is significant
(exponentional performance degradation). I was confused into
thinking that this would "only" impact Cortex-m without security
extensions, but maybe it's the opposite actually.

So the next question is, what is the potential impact if we
backport the patch and there is a bug in it:

  - Well, it touches the generic Arm unwinding code, so
    worse case scenario, DWARF-based unwinding is broken?

  - With that said, the patch appears to simply add a cache,
    so the logic of it all doesn't appear to be extremely
    complicated. So I would rate the risk to be low.

If Tom and/or Simon agree, my assessment is that it is fine
to backport those two patches onto the gdb-13 branch.

-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-27  6:30   ` Joel Brobecker
@ 2023-01-27  6:38     ` Luis Machado
  2023-01-27 17:17       ` Simon Marchi
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2023-01-27  6:38 UTC (permalink / raw)
  To: Joel Brobecker, Torbjorn SVENSSON, simon.marchi, tom; +Cc: gdb-patches

On 1/27/23 06:30, Joel Brobecker wrote:
> Hello,
> 
>>>     * [Torbjorn] tdep/29738
>>>       Arm M-profile dwarf2 unwinder performance suffers from exponential growth
>>>       https://sourceware.org/bugzilla/show_bug.cgi?id=29738
>>>
>>>       patch v3, 2023-01-19, reviewed 2023-01-20:
>>>       https://sourceware.org/pipermail/gdb-patches/2023-January/195915.html
>>
>> I just pushed this for master.
>> Is it okay to also push the 2 patches to gdb-13-branch?
> 
> For the avoidance of doubt, my understanding is that we are talking
> about the following two patches:
> 
>      commit d72ba177c85f2ad18d0dcabdd8844532c9acb819
>      Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
>      Date:   Thu Nov 17 12:17:53 2022 +0100
>      Subject: gdb: dwarf2 generic implementation for caching function data
> 
> ... and ...
> 
>      commit 5cf11483141a58314834653003e49709b47822d5
>      Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
>      Date:   Thu Nov 17 12:18:20 2022 +0100
>      Subject: gdb/arm: Use new dwarf2 function cache
> 
> I hope I having missed any other patch!
> 
> The first one adds, as the subject indicates, a framework for
> caching frame-related information, and the second patch takes
> advantage of that framework,
> 
> Luis marked the corresponding PR as "important to fix", so
> I'm assuming the impact if we do not backport is significant
> (exponentional performance degradation). I was confused into
> thinking that this would "only" impact Cortex-m without security
> extensions, but maybe it's the opposite actually.

It affects m-profiles, but it affects the targets with the security extensions more, as they have more SP registers.

> 
> So the next question is, what is the potential impact if we
> backport the patch and there is a bug in it:
> 
>    - Well, it touches the generic Arm unwinding code, so
>      worse case scenario, DWARF-based unwinding is broken?

The change is restricted to m-profiles. Unless there is something really off with the caching code, it should break all Arm unwinding.

> 
>    - With that said, the patch appears to simply add a cache,
>      so the logic of it all doesn't appear to be extremely
>      complicated. So I would rate the risk to be low.

That's what it seems to me.

I think another round of testing would be a good step to make sure there are no hidden bugs.

> 
> If Tom and/or Simon agree, my assessment is that it is fine
> to backport those two patches onto the gdb-13 branch.
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-27  6:38     ` Luis Machado
@ 2023-01-27 17:17       ` Simon Marchi
  2023-01-28  8:39         ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2023-01-27 17:17 UTC (permalink / raw)
  To: Luis Machado, Joel Brobecker, Torbjorn SVENSSON, tom; +Cc: gdb-patches

>>    - With that said, the patch appears to simply add a cache,
>>      so the logic of it all doesn't appear to be extremely
>>      complicated. So I would rate the risk to be low.
> 
> That's what it seems to me.
> 
> I think another round of testing would be a good step to make sure there are no hidden bugs.
My understanding is that the problem was making debugging borderline
impossible.  Or at least, really, really unpleasant.  So I think it
qualifies as a bug fix, and that the benefit is worth the risk.

Simon



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-27 17:17       ` Simon Marchi
@ 2023-01-28  8:39         ` Luis Machado
  2023-01-29 11:52           ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2023-01-28  8:39 UTC (permalink / raw)
  To: Simon Marchi, Joel Brobecker, Torbjorn SVENSSON, tom; +Cc: gdb-patches

On 1/27/23 17:17, Simon Marchi wrote:
>>>     - With that said, the patch appears to simply add a cache,
>>>       so the logic of it all doesn't appear to be extremely
>>>       complicated. So I would rate the risk to be low.
>>
>> That's what it seems to me.
>>
>> I think another round of testing would be a good step to make sure there are no hidden bugs.
> My understanding is that the problem was making debugging borderline
> impossible.  Or at least, really, really unpleasant.  So I think it
> qualifies as a bug fix, and that the benefit is worth the risk.

Right. Past a short number of frames, unwinding got really really slow.

At least on my end, things look ok testsuite-wise.

> 
> Simon
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-28  8:39         ` Luis Machado
@ 2023-01-29 11:52           ` Joel Brobecker
  2023-01-30 11:23             ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2023-01-29 11:52 UTC (permalink / raw)
  To: Luis Machado
  Cc: Simon Marchi, Joel Brobecker, Torbjorn SVENSSON, tom, gdb-patches

> > > >     - With that said, the patch appears to simply add a cache,
> > > >       so the logic of it all doesn't appear to be extremely
> > > >       complicated. So I would rate the risk to be low.
> > > 
> > > That's what it seems to me.
> > > 
> > > I think another round of testing would be a good step to make sure there are no hidden bugs.
> > My understanding is that the problem was making debugging borderline
> > impossible.  Or at least, really, really unpleasant.  So I think it
> > qualifies as a bug fix, and that the benefit is worth the risk.
> 
> Right. Past a short number of frames, unwinding got really really slow.

The part that I haven't been clear on is whether this affected everyone
on Arm-32bit, or everyone on both Arm and AArch64, or just a subset
of the users?

Regardless of the above, I agree we can backport. To avoid confusion,
can I leave this to you, Luis, to confirm that the two patches I identified
previously are the only two patches that need to be backported, and
can you backport those for us, with a round of testing if you haven't
done so already, just to double-check?

For the record, those patches were:

    commit d72ba177c85f2ad18d0dcabdd8844532c9acb819
    Author: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
    Date:   Thu Nov 17 12:17:53 2022 +0100
    Subject: gdb: dwarf2 generic implementation for caching function data

... and ...

    commit 5cf11483141a58314834653003e49709b47822d5
    Author: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
    Date:   Thu Nov 17 12:18:20 2022 +0100
    Subject: gdb/arm: Use new dwarf2 function cache

Thank you!
-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-29 11:52           ` Joel Brobecker
@ 2023-01-30 11:23             ` Luis Machado
  2023-01-31  7:03               ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Machado @ 2023-01-30 11:23 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Simon Marchi, Torbjorn SVENSSON, tom, gdb-patches

On 1/29/23 11:52, Joel Brobecker wrote:
>>>>>      - With that said, the patch appears to simply add a cache,
>>>>>        so the logic of it all doesn't appear to be extremely
>>>>>        complicated. So I would rate the risk to be low.
>>>>
>>>> That's what it seems to me.
>>>>
>>>> I think another round of testing would be a good step to make sure there are no hidden bugs.
>>> My understanding is that the problem was making debugging borderline
>>> impossible.  Or at least, really, really unpleasant.  So I think it
>>> qualifies as a bug fix, and that the benefit is worth the risk.
>>
>> Right. Past a short number of frames, unwinding got really really slow.
> 
> The part that I haven't been clear on is whether this affected everyone
> on Arm-32bit, or everyone on both Arm and AArch64, or just a subset
> of the users?

Just a subset.

It affects 32-bit m-profile Arm targets that report the additional stack pointers.  This is either
the org.gnu.gdb.arm.m-system feature or the org.gnu.gdb.arm.secext feature.

I'm fairly sure these features are only reported by emulators and bare-metal targets.

Now, a 64-bit gdb can debug 32-bit Arm as well, so you could have a 64-bit gdb running into this too
if the target is a 32-bit m-profile Arm. But the target is still 32-bit Arm.

This doesn't affect AArch64 at all.

> 
> Regardless of the above, I agree we can backport. To avoid confusion,
> can I leave this to you, Luis, to confirm that the two patches I identified
> previously are the only two patches that need to be backported, and
> can you backport those for us, with a round of testing if you haven't
> done so already, just to double-check?

Sure. I gave it a try with master and the test results look the same with or without the patches.

Unfortunately the 32-bit Arm test results are not too clean as there is quite a bit of noise from
failing watchpoints tests.

I'll try with GDB 13 just to be sure.

> 
> For the record, those patches were:
> 
>      commit d72ba177c85f2ad18d0dcabdd8844532c9acb819
>      Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
>      Date:   Thu Nov 17 12:17:53 2022 +0100
>      Subject: gdb: dwarf2 generic implementation for caching function data
> 
> ... and ...
> 
>      commit 5cf11483141a58314834653003e49709b47822d5
>      Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
>      Date:   Thu Nov 17 12:18:20 2022 +0100
>      Subject: gdb/arm: Use new dwarf2 function cache
> 
> Thank you!


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-30 11:23             ` Luis Machado
@ 2023-01-31  7:03               ` Joel Brobecker
  2023-01-31 13:57                 ` Luis Machado
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2023-01-31  7:03 UTC (permalink / raw)
  To: Luis Machado
  Cc: Joel Brobecker, Simon Marchi, Torbjorn SVENSSON, tom, gdb-patches

Hi Luis,

On Mon, Jan 30, 2023 at 11:23:06AM +0000, Luis Machado wrote:
> On 1/29/23 11:52, Joel Brobecker wrote:
> > > > > >      - With that said, the patch appears to simply add a cache,
> > > > > >        so the logic of it all doesn't appear to be extremely
> > > > > >        complicated. So I would rate the risk to be low.
> > > > > 
> > > > > That's what it seems to me.
> > > > > 
> > > > > I think another round of testing would be a good step to make sure there are no hidden bugs.
> > > > My understanding is that the problem was making debugging borderline
> > > > impossible.  Or at least, really, really unpleasant.  So I think it
> > > > qualifies as a bug fix, and that the benefit is worth the risk.
> > > 
> > > Right. Past a short number of frames, unwinding got really really slow.
> > 
> > The part that I haven't been clear on is whether this affected everyone
> > on Arm-32bit, or everyone on both Arm and AArch64, or just a subset
> > of the users?
> 
> Just a subset.
> 
> It affects 32-bit m-profile Arm targets that report the additional stack pointers.  This is either
> the org.gnu.gdb.arm.m-system feature or the org.gnu.gdb.arm.secext feature.
> 
> I'm fairly sure these features are only reported by emulators and bare-metal targets.
> 
> Now, a 64-bit gdb can debug 32-bit Arm as well, so you could have a 64-bit gdb running into this too
> if the target is a 32-bit m-profile Arm. But the target is still 32-bit Arm.
> 
> This doesn't affect AArch64 at all.

Thanks for the extra effort explaining the impact. This is much clear
for me, now. With your permission, I'd like to copy the text above
as a comment in the PR. Would that be OK?

> > Regardless of the above, I agree we can backport. To avoid confusion,
> > can I leave this to you, Luis, to confirm that the two patches I identified
> > previously are the only two patches that need to be backported, and
> > can you backport those for us, with a round of testing if you haven't
> > done so already, just to double-check?
> 
> Sure. I gave it a try with master and the test results look the same with or without the patches.
> 
> Unfortunately the 32-bit Arm test results are not too clean as there is quite a bit of noise from
> failing watchpoints tests.
> 
> I'll try with GDB 13 just to be sure.

Thank you. Once you've finished testing, whatever that might be,
you can go ahead and push the patches. As disussed earlier, we all
agree the risk of bad impact is very low.

Let me know when this is done. It's starting to look like we might
have a pre-release created this weekend! Maybe earlier, if I can
carve some time out of my work schedule.

> > For the record, those patches were:
> > 
> >      commit d72ba177c85f2ad18d0dcabdd8844532c9acb819
> >      Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
> >      Date:   Thu Nov 17 12:17:53 2022 +0100
> >      Subject: gdb: dwarf2 generic implementation for caching function data
> > 
> > ... and ...
> > 
> >      commit 5cf11483141a58314834653003e49709b47822d5
> >      Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
> >      Date:   Thu Nov 17 12:18:20 2022 +0100
> >      Subject: gdb/arm: Use new dwarf2 function cache
> > 
> > Thank you!
> 

-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-31  7:03               ` Joel Brobecker
@ 2023-01-31 13:57                 ` Luis Machado
  2023-01-31 14:33                   ` Luis Machado
  2023-02-02  3:44                   ` Joel Brobecker
  0 siblings, 2 replies; 12+ messages in thread
From: Luis Machado @ 2023-01-31 13:57 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Simon Marchi, Torbjorn SVENSSON, tom, gdb-patches

Hi Joel,

On 1/31/23 07:03, Joel Brobecker wrote:
> Hi Luis,
> 
> On Mon, Jan 30, 2023 at 11:23:06AM +0000, Luis Machado wrote:
>> On 1/29/23 11:52, Joel Brobecker wrote:
>>>>>>>       - With that said, the patch appears to simply add a cache,
>>>>>>>         so the logic of it all doesn't appear to be extremely
>>>>>>>         complicated. So I would rate the risk to be low.
>>>>>>
>>>>>> That's what it seems to me.
>>>>>>
>>>>>> I think another round of testing would be a good step to make sure there are no hidden bugs.
>>>>> My understanding is that the problem was making debugging borderline
>>>>> impossible.  Or at least, really, really unpleasant.  So I think it
>>>>> qualifies as a bug fix, and that the benefit is worth the risk.
>>>>
>>>> Right. Past a short number of frames, unwinding got really really slow.
>>>
>>> The part that I haven't been clear on is whether this affected everyone
>>> on Arm-32bit, or everyone on both Arm and AArch64, or just a subset
>>> of the users?
>>
>> Just a subset.
>>
>> It affects 32-bit m-profile Arm targets that report the additional stack pointers.  This is either
>> the org.gnu.gdb.arm.m-system feature or the org.gnu.gdb.arm.secext feature.
>>
>> I'm fairly sure these features are only reported by emulators and bare-metal targets.
>>
>> Now, a 64-bit gdb can debug 32-bit Arm as well, so you could have a 64-bit gdb running into this too
>> if the target is a 32-bit m-profile Arm. But the target is still 32-bit Arm.
>>
>> This doesn't affect AArch64 at all.
> 
> Thanks for the extra effort explaining the impact. This is much clear
> for me, now. With your permission, I'd like to copy the text above
> as a comment in the PR. Would that be OK?> 

Yes, that's fine. Let me know if you want me to do it.

>>> Regardless of the above, I agree we can backport. To avoid confusion,
>>> can I leave this to you, Luis, to confirm that the two patches I identified
>>> previously are the only two patches that need to be backported, and
>>> can you backport those for us, with a round of testing if you haven't
>>> done so already, just to double-check?
>>
>> Sure. I gave it a try with master and the test results look the same with or without the patches.
>>
>> Unfortunately the 32-bit Arm test results are not too clean as there is quite a bit of noise from
>> failing watchpoints tests.
>>
>> I'll try with GDB 13 just to be sure.
> 
> Thank you. Once you've finished testing, whatever that might be,
> you can go ahead and push the patches. As disussed earlier, we all
> agree the risk of bad impact is very low.
> 
> Let me know when this is done. It's starting to look like we might
> have a pre-release created this weekend! Maybe earlier, if I can
> carve some time out of my work schedule.
> 

I ran checks with the gdb 13 branch and the results look the same for a patched gdb compared to
an unpatched one.

I'll cherry-pick both commits to gdb 13 once sourceware is back up.

Thanks for the patience in getting this addressed.

>>> For the record, those patches were:
>>>
>>>       commit d72ba177c85f2ad18d0dcabdd8844532c9acb819
>>>       Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
>>>       Date:   Thu Nov 17 12:17:53 2022 +0100
>>>       Subject: gdb: dwarf2 generic implementation for caching function data
>>>
>>> ... and ...
>>>
>>>       commit 5cf11483141a58314834653003e49709b47822d5
>>>       Author: Torbj�rn SVENSSON <torbjorn.svensson@foss.st.com>
>>>       Date:   Thu Nov 17 12:18:20 2022 +0100
>>>       Subject: gdb/arm: Use new dwarf2 function cache
>>>
>>> Thank you!
>>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-31 13:57                 ` Luis Machado
@ 2023-01-31 14:33                   ` Luis Machado
  2023-02-02  3:44                   ` Joel Brobecker
  1 sibling, 0 replies; 12+ messages in thread
From: Luis Machado @ 2023-01-31 14:33 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Simon Marchi, Torbjorn SVENSSON, tom, gdb-patches

On 1/31/23 13:57, Luis Machado wrote:
> Hi Joel,
> 
> On 1/31/23 07:03, Joel Brobecker wrote:
>> Hi Luis,
>>
>> On Mon, Jan 30, 2023 at 11:23:06AM +0000, Luis Machado wrote:
>>> On 1/29/23 11:52, Joel Brobecker wrote:
>>>>>>>>       - With that said, the patch appears to simply add a cache,
>>>>>>>>         so the logic of it all doesn't appear to be extremely
>>>>>>>>         complicated. So I would rate the risk to be low.
>>>>>>>
>>>>>>> That's what it seems to me.
>>>>>>>
>>>>>>> I think another round of testing would be a good step to make sure there are no hidden bugs.
>>>>>> My understanding is that the problem was making debugging borderline
>>>>>> impossible.  Or at least, really, really unpleasant.  So I think it
>>>>>> qualifies as a bug fix, and that the benefit is worth the risk.
>>>>>
>>>>> Right. Past a short number of frames, unwinding got really really slow.
>>>>
>>>> The part that I haven't been clear on is whether this affected everyone
>>>> on Arm-32bit, or everyone on both Arm and AArch64, or just a subset
>>>> of the users?
>>>
>>> Just a subset.
>>>
>>> It affects 32-bit m-profile Arm targets that report the additional stack pointers.  This is either
>>> the org.gnu.gdb.arm.m-system feature or the org.gnu.gdb.arm.secext feature.
>>>
>>> I'm fairly sure these features are only reported by emulators and bare-metal targets.
>>>
>>> Now, a 64-bit gdb can debug 32-bit Arm as well, so you could have a 64-bit gdb running into this too
>>> if the target is a 32-bit m-profile Arm. But the target is still 32-bit Arm.
>>>
>>> This doesn't affect AArch64 at all.
>>
>> Thanks for the extra effort explaining the impact. This is much clear
>> for me, now. With your permission, I'd like to copy the text above
>> as a comment in the PR. Would that be OK?> 
> 
> Yes, that's fine. Let me know if you want me to do it.
> 
>>>> Regardless of the above, I agree we can backport. To avoid confusion,
>>>> can I leave this to you, Luis, to confirm that the two patches I identified
>>>> previously are the only two patches that need to be backported, and
>>>> can you backport those for us, with a round of testing if you haven't
>>>> done so already, just to double-check?
>>>
>>> Sure. I gave it a try with master and the test results look the same with or without the patches.
>>>
>>> Unfortunately the 32-bit Arm test results are not too clean as there is quite a bit of noise from
>>> failing watchpoints tests.
>>>
>>> I'll try with GDB 13 just to be sure.
>>
>> Thank you. Once you've finished testing, whatever that might be,
>> you can go ahead and push the patches. As disussed earlier, we all
>> agree the risk of bad impact is very low.
>>
>> Let me know when this is done. It's starting to look like we might
>> have a pre-release created this weekend! Maybe earlier, if I can
>> carve some time out of my work schedule.
>>
> 
> I ran checks with the gdb 13 branch and the results look the same for a patched gdb compared to
> an unpatched one.
> 
> I'll cherry-pick both commits to gdb 13 once sourceware is back up.

Done now.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: GDB 13 release -- 2023-01-21 Update
  2023-01-31 13:57                 ` Luis Machado
  2023-01-31 14:33                   ` Luis Machado
@ 2023-02-02  3:44                   ` Joel Brobecker
  1 sibling, 0 replies; 12+ messages in thread
From: Joel Brobecker @ 2023-02-02  3:44 UTC (permalink / raw)
  To: Luis Machado
  Cc: Joel Brobecker, Simon Marchi, Torbjorn SVENSSON, tom, gdb-patches

> > Thanks for the extra effort explaining the impact. This is much clear
> > for me, now. With your permission, I'd like to copy the text above
> > as a comment in the PR. Would that be OK?>
> 
> Yes, that's fine. Let me know if you want me to do it.

Thanks for the kind offer, Luis. I just took care of it as part
of my pass on pending PRs.

-- 
Joel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-02-02  3:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-21  6:08 GDB 13 release -- 2023-01-21 Update Joel Brobecker
2023-01-25 20:18 ` Torbjorn SVENSSON
2023-01-27  6:30   ` Joel Brobecker
2023-01-27  6:38     ` Luis Machado
2023-01-27 17:17       ` Simon Marchi
2023-01-28  8:39         ` Luis Machado
2023-01-29 11:52           ` Joel Brobecker
2023-01-30 11:23             ` Luis Machado
2023-01-31  7:03               ` Joel Brobecker
2023-01-31 13:57                 ` Luis Machado
2023-01-31 14:33                   ` Luis Machado
2023-02-02  3:44                   ` Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).