public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
       [not found]     ` <87wn3ocwqz.fsf@suse.de>
@ 2023-03-10 18:14       ` Alex Bennée
  2023-03-13 11:22         ` Peter Maydell
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Bennée @ 2023-03-10 18:14 UTC (permalink / raw)
  To: Fabiano Rosas
  Cc: Peter Maydell, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid


(adding some more gdb types to CC)

Fabiano Rosas <farosas@suse.de> writes:

> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On Fri, 10 Mar 2023 at 10:31, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>
>>> You need a very new gdb to be able to run with pauth support otherwise
>>> your likely to hit asserts and aborts. Disable pauth for now until we
>>> can properly probe support in gdb.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> If it makes gdb fall over, then shouldn't we be disabling
>> the pauth gdbstub stuff entirely ? Otherwise even if our
>> tests are fine our users will not be...
>>
>
> Have you seem my message on IRC about changing the feature name in the
> XML? I think the issue is that we're putting the .xml in a "namespace"
> where GDB expects to only find stuff which it has code to
> support. Changing from "org.gnu.gdb.aarch64.pauth" to
> "org.qemu.aarch64.pauth" made it stop crashing and I can read the
> registers just fine.

That would work, although I would prefer to probe support so we can use
the official namespace. We went through something similar with SVE
until:

  797920b952 (target/arm: use official org.gnu.gdb.aarch64.sve layout for registers)

which required:

  b1863ccc95 (configure: gate our use of GDB to 8.3.1 or above)

Since then we've introduced:

 ./scripts/probe-gdb-support.py

which given the runes to check for pauth support in gdb could expose a
symbol and we get the best of both worlds. Of course if this keeps
happening we could throw up our hands and just use custom XML for all
the extra register sets.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-10 18:14       ` [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests Alex Bennée
@ 2023-03-13 11:22         ` Peter Maydell
  2023-03-13 11:44           ` Luis Machado
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2023-03-13 11:22 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

On Fri, 10 Mar 2023 at 18:20, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> (adding some more gdb types to CC)
>
> Fabiano Rosas <farosas@suse.de> writes:
>
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >
> >> On Fri, 10 Mar 2023 at 10:31, Alex Bennée <alex.bennee@linaro.org> wrote:
> >>>
> >>> You need a very new gdb to be able to run with pauth support otherwise
> >>> your likely to hit asserts and aborts. Disable pauth for now until we
> >>> can properly probe support in gdb.
> >>>
> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >>
> >> If it makes gdb fall over, then shouldn't we be disabling
> >> the pauth gdbstub stuff entirely ? Otherwise even if our
> >> tests are fine our users will not be...
> >>
> >
> > Have you seem my message on IRC about changing the feature name in the
> > XML? I think the issue is that we're putting the .xml in a "namespace"
> > where GDB expects to only find stuff which it has code to
> > support. Changing from "org.gnu.gdb.aarch64.pauth" to
> > "org.qemu.aarch64.pauth" made it stop crashing and I can read the
> > registers just fine.
>
> That would work, although I would prefer to probe support so we can use
> the official namespace.

I don't think there's a way to probe for this problem. I spoke
to Luis about this, and apparently it's a bug in how gdb handles
the pauth XML description (fixed in gdb commit 1ba3a3222039eb25).
A gdb without any pauth support at all will be fine; a gdb with
the bug will report that it has pauth support but will crash
if you feed it the whole set of XML that QEMU has; a gdb
with the bug fixed will also report pauth support but won't crash.
(The bug only manifests if the full XML includes registers that GDB
doesn't care about, like the system registers; if the stub sends
only registers GDB knows about then it won't crash.)

Luis and I came up with two options:

(1) leave QEMU outputting the pauth xml as-is, and tell people
whose gdb 12 crashes that they should upgrade to a newer gdb

(2) make QEMU output the pauth info under a different XML namespace,
and tell people who need backtraces when pauth is enabled
that they should upgrade to a newer gdb

Neither of these feel great, but on balance I guess 2 is better?

Luis: I think that rather than doing (2) with a QEMU namespace,
we should define a gdb namespace for this. That makes it clear
that this is still a gdb-upstream-sanctioned way of exposing
the pauth registers.

thanks
-- PMM

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-13 11:22         ` Peter Maydell
@ 2023-03-13 11:44           ` Luis Machado
  2023-03-15  9:50             ` Luis Machado
  0 siblings, 1 reply; 9+ messages in thread
From: Luis Machado @ 2023-03-13 11:44 UTC (permalink / raw)
  To: Peter Maydell, Alex Bennée
  Cc: Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

On 3/13/23 11:22, Peter Maydell via Gdb wrote:
> On Fri, 10 Mar 2023 at 18:20, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> (adding some more gdb types to CC)
>>
>> Fabiano Rosas <farosas@suse.de> writes:
>>
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>
>>>> On Fri, 10 Mar 2023 at 10:31, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>>>
>>>>> You need a very new gdb to be able to run with pauth support otherwise
>>>>> your likely to hit asserts and aborts. Disable pauth for now until we
>>>>> can properly probe support in gdb.
>>>>>
>>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>>
>>>> If it makes gdb fall over, then shouldn't we be disabling
>>>> the pauth gdbstub stuff entirely ? Otherwise even if our
>>>> tests are fine our users will not be...
>>>>
>>>
>>> Have you seem my message on IRC about changing the feature name in the
>>> XML? I think the issue is that we're putting the .xml in a "namespace"
>>> where GDB expects to only find stuff which it has code to
>>> support. Changing from "org.gnu.gdb.aarch64.pauth" to
>>> "org.qemu.aarch64.pauth" made it stop crashing and I can read the
>>> registers just fine.
>>
>> That would work, although I would prefer to probe support so we can use
>> the official namespace.
>
> I don't think there's a way to probe for this problem. I spoke
> to Luis about this, and apparently it's a bug in how gdb handles
> the pauth XML description (fixed in gdb commit 1ba3a3222039eb25).
> A gdb without any pauth support at all will be fine; a gdb with
> the bug will report that it has pauth support but will crash
> if you feed it the whole set of XML that QEMU has; a gdb
> with the bug fixed will also report pauth support but won't crash.
> (The bug only manifests if the full XML includes registers that GDB
> doesn't care about, like the system registers; if the stub sends
> only registers GDB knows about then it won't crash.)
>
> Luis and I came up with two options:
>
> (1) leave QEMU outputting the pauth xml as-is, and tell people
> whose gdb 12 crashes that they should upgrade to a newer gdb
>
> (2) make QEMU output the pauth info under a different XML namespace,
> and tell people who need backtraces when pauth is enabled
> that they should upgrade to a newer gdb
>
> Neither of these feel great, but on balance I guess 2 is better?
>
> Luis: I think that rather than doing (2) with a QEMU namespace,
> we should define a gdb namespace for this. That makes it clear
> that this is still a gdb-upstream-sanctioned way of exposing
> the pauth registers.

That should be fine as well, and would work to side-step the gdb 12 bug so it doesn't crash.

We could name the feature "org.gnu.gdb.aarch64.pauth_v2" or somesuch, and slowly stop using the original
"org.gnu.gdb.aarch64.pauth" feature. I can document the requirements for a compliant pauth_v2.

FYI, I've pushed a better documentation for the arm/aarch64 xml descriptions here:

https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=d7001b29e9f256dfc60acb481d9df8f91f2ee623
https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=e0994165d1b8469dfc27b09b62ac74862d535812

>
> thanks
> -- PMM

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-13 11:44           ` Luis Machado
@ 2023-03-15  9:50             ` Luis Machado
  2023-03-17 16:37               ` Peter Maydell
  0 siblings, 1 reply; 9+ messages in thread
From: Luis Machado @ 2023-03-15  9:50 UTC (permalink / raw)
  To: Peter Maydell, Alex Bennée
  Cc: Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

Hi,

On 3/13/23 11:44, Luis Machado wrote:
> On 3/13/23 11:22, Peter Maydell via Gdb wrote:
>> On Fri, 10 Mar 2023 at 18:20, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>
>>>
>>> (adding some more gdb types to CC)
>>>
>>> Fabiano Rosas <farosas@suse.de> writes:
>>>
>>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>>
>>>>> On Fri, 10 Mar 2023 at 10:31, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>>>>
>>>>>> You need a very new gdb to be able to run with pauth support otherwise
>>>>>> your likely to hit asserts and aborts. Disable pauth for now until we
>>>>>> can properly probe support in gdb.
>>>>>>
>>>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>>>
>>>>> If it makes gdb fall over, then shouldn't we be disabling
>>>>> the pauth gdbstub stuff entirely ? Otherwise even if our
>>>>> tests are fine our users will not be...
>>>>>
>>>>
>>>> Have you seem my message on IRC about changing the feature name in the
>>>> XML? I think the issue is that we're putting the .xml in a "namespace"
>>>> where GDB expects to only find stuff which it has code to
>>>> support. Changing from "org.gnu.gdb.aarch64.pauth" to
>>>> "org.qemu.aarch64.pauth" made it stop crashing and I can read the
>>>> registers just fine.
>>>
>>> That would work, although I would prefer to probe support so we can use
>>> the official namespace.
>>
>> I don't think there's a way to probe for this problem. I spoke
>> to Luis about this, and apparently it's a bug in how gdb handles
>> the pauth XML description (fixed in gdb commit 1ba3a3222039eb25).
>> A gdb without any pauth support at all will be fine; a gdb with
>> the bug will report that it has pauth support but will crash
>> if you feed it the whole set of XML that QEMU has; a gdb
>> with the bug fixed will also report pauth support but won't crash.
>> (The bug only manifests if the full XML includes registers that GDB
>> doesn't care about, like the system registers; if the stub sends
>> only registers GDB knows about then it won't crash.)
>>
>> Luis and I came up with two options:
>>
>> (1) leave QEMU outputting the pauth xml as-is, and tell people
>> whose gdb 12 crashes that they should upgrade to a newer gdb
>>
>> (2) make QEMU output the pauth info under a different XML namespace,
>> and tell people who need backtraces when pauth is enabled
>> that they should upgrade to a newer gdb
>>
>> Neither of these feel great, but on balance I guess 2 is better?
>>
>> Luis: I think that rather than doing (2) with a QEMU namespace,
>> we should define a gdb namespace for this. That makes it clear
>> that this is still a gdb-upstream-sanctioned way of exposing
>> the pauth registers.
>
> That should be fine as well, and would work to side-step the gdb 12 bug so it doesn't crash.
>
> We could name the feature "org.gnu.gdb.aarch64.pauth_v2" or somesuch, and slowly stop using the original
> "org.gnu.gdb.aarch64.pauth" feature. I can document the requirements for a compliant pauth_v2.
>
> FYI, I've pushed a better documentation for the arm/aarch64 xml descriptions here:
>
> https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=d7001b29e9f256dfc60acb481d9df8f91f2ee623
> https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=e0994165d1b8469dfc27b09b62ac74862d535812
>
>>
>> thanks
>> -- PMM
>

Just an update on this. I had a chat with Richard Henderson yesterday, and it might actually be easier and more convenient to backport
fixes to older gdb versions (at least gdb-12 and gdb-11, but gdb-10 and gdb-9 are also affected). This will ensure those won't crash when
they connect to a qemu that advertises the pauth feature.

It also means we won't need qemu-side changes. My understanding is that we're close to the 8.0.0 release, and the code is already in place.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-15  9:50             ` Luis Machado
@ 2023-03-17 16:37               ` Peter Maydell
  2023-03-17 16:55                 ` Luis Machado
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2023-03-17 16:37 UTC (permalink / raw)
  To: Luis Machado
  Cc: Alex Bennée, Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

On Wed, 15 Mar 2023 at 09:51, Luis Machado <luis.machado@arm.com> wrote:
> On 3/13/23 11:44, Luis Machado wrote:
> > On 3/13/23 11:22, Peter Maydell via Gdb wrote:
> >> Luis and I came up with two options:
> >>
> >> (1) leave QEMU outputting the pauth xml as-is, and tell people
> >> whose gdb 12 crashes that they should upgrade to a newer gdb
> >>
> >> (2) make QEMU output the pauth info under a different XML namespace,
> >> and tell people who need backtraces when pauth is enabled
> >> that they should upgrade to a newer gdb
> >>
> >> Neither of these feel great, but on balance I guess 2 is better?
> >>
> >> Luis: I think that rather than doing (2) with a QEMU namespace,
> >> we should define a gdb namespace for this. That makes it clear
> >> that this is still a gdb-upstream-sanctioned way of exposing
> >> the pauth registers.
> >
> > That should be fine as well, and would work to side-step the gdb 12 bug so it doesn't crash.
> >
> > We could name the feature "org.gnu.gdb.aarch64.pauth_v2" or somesuch, and slowly stop using the original
> > "org.gnu.gdb.aarch64.pauth" feature. I can document the requirements for a compliant pauth_v2.
> >
> > FYI, I've pushed a better documentation for the arm/aarch64 xml descriptions here:
> >
> > https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=d7001b29e9f256dfc60acb481d9df8f91f2ee623
> > https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=e0994165d1b8469dfc27b09b62ac74862d535812

> Just an update on this. I had a chat with Richard Henderson yesterday, and it might actually be easier and more convenient to backport
> fixes to older gdb versions (at least gdb-12 and gdb-11, but gdb-10 and gdb-9 are also affected). This will ensure those won't crash when
> they connect to a qemu that advertises the pauth feature.
>
> It also means we won't need qemu-side changes. My understanding is that we're close to the 8.0.0 release, and the code is already in place.

Having run into this problem in another couple of situations, one of
which involved gdb 10, I think I'm increasingly favouring option
2 here. The affected gdbs seem to be quite widely deployed, and
the bug results in crashes even for users who didn't really
care about pauth. So I'd rather we didn't release a QEMU 8.0
which crashes these affected deployed gdbs.

So:
 (a) if on the gdb side you can define (within the next week) a
suitable new XML name you want QEMU to expose, we can commit a
change to switch to that before we do the 8.0 release
 (b) if that's too tight a timescale, we can commit a patch which
just stops QEMU from exposing the pauth.xml, and we can come up
with a better solution after 8.0 releases

In fact, I think I'm going to submit a patch to do (b) for
now and we can follow up with a patch for (a) if we want.

thanks
-- PMM

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-17 16:37               ` Peter Maydell
@ 2023-03-17 16:55                 ` Luis Machado
  2023-03-17 17:07                   ` Peter Maydell
  0 siblings, 1 reply; 9+ messages in thread
From: Luis Machado @ 2023-03-17 16:55 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Alex Bennée, Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

On 3/17/23 16:37, Peter Maydell wrote:
> On Wed, 15 Mar 2023 at 09:51, Luis Machado <luis.machado@arm.com> wrote:
>> On 3/13/23 11:44, Luis Machado wrote:
>>> On 3/13/23 11:22, Peter Maydell via Gdb wrote:
>>>> Luis and I came up with two options:
>>>>
>>>> (1) leave QEMU outputting the pauth xml as-is, and tell people
>>>> whose gdb 12 crashes that they should upgrade to a newer gdb
>>>>
>>>> (2) make QEMU output the pauth info under a different XML namespace,
>>>> and tell people who need backtraces when pauth is enabled
>>>> that they should upgrade to a newer gdb
>>>>
>>>> Neither of these feel great, but on balance I guess 2 is better?
>>>>
>>>> Luis: I think that rather than doing (2) with a QEMU namespace,
>>>> we should define a gdb namespace for this. That makes it clear
>>>> that this is still a gdb-upstream-sanctioned way of exposing
>>>> the pauth registers.
>>>
>>> That should be fine as well, and would work to side-step the gdb 12 bug so it doesn't crash.
>>>
>>> We could name the feature "org.gnu.gdb.aarch64.pauth_v2" or somesuch, and slowly stop using the original
>>> "org.gnu.gdb.aarch64.pauth" feature. I can document the requirements for a compliant pauth_v2.
>>>
>>> FYI, I've pushed a better documentation for the arm/aarch64 xml descriptions here:
>>>
>>> https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=d7001b29e9f256dfc60acb481d9df8f91f2ee623
>>> https://sourceware.org/git?p=binutils-gdb.git;a=commit;h=e0994165d1b8469dfc27b09b62ac74862d535812
>
>> Just an update on this. I had a chat with Richard Henderson yesterday, and it might actually be easier and more convenient to backport
>> fixes to older gdb versions (at least gdb-12 and gdb-11, but gdb-10 and gdb-9 are also affected). This will ensure those won't crash when
>> they connect to a qemu that advertises the pauth feature.
>>
>> It also means we won't need qemu-side changes. My understanding is that we're close to the 8.0.0 release, and the code is already in place.
>
> Having run into this problem in another couple of situations, one of
> which involved gdb 10, I think I'm increasingly favouring option
> 2 here. The affected gdbs seem to be quite widely deployed, and
> the bug results in crashes even for users who didn't really
> care about pauth. So I'd rather we didn't release a QEMU 8.0
> which crashes these affected deployed gdbs.
>

Are the affected gdb's packaged by distros? If so, a backport the distros can pick up
will solve this in a quick package update.

If we decide qemu should now emit a different xml for pauth, it will fix the crashes, but it also
means older gdb's (9/10/11/12) will not be able to backtrace properly through pauth-signed frames.

Maybe that's a reasonable drawback for qemu users?

If someone decides to implement a debugging stub that reports pauth (fast models, for example), it will
also crash gdb, so I still plan to do the backport anyway.

> So:
>   (a) if on the gdb side you can define (within the next week) a
> suitable new XML name you want QEMU to expose, we can commit a
> change to switch to that before we do the 8.0 release

pauth_v2 sounds about as good as any other for me.

>   (b) if that's too tight a timescale, we can commit a patch which
> just stops QEMU from exposing the pauth.xml, and we can come up
> with a better solution after 8.0 releases
>
> In fact, I think I'm going to submit a patch to do (b) for
> now and we can follow up with a patch for (a) if we want.
>
> thanks
> -- PMM

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-17 16:55                 ` Luis Machado
@ 2023-03-17 17:07                   ` Peter Maydell
  2023-03-17 17:12                     ` Luis Machado
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2023-03-17 17:07 UTC (permalink / raw)
  To: Luis Machado
  Cc: Alex Bennée, Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

On Fri, 17 Mar 2023 at 16:55, Luis Machado <luis.machado@arm.com> wrote:
> On 3/17/23 16:37, Peter Maydell wrote:
> > Having run into this problem in another couple of situations, one of
> > which involved gdb 10, I think I'm increasingly favouring option
> > 2 here. The affected gdbs seem to be quite widely deployed, and
> > the bug results in crashes even for users who didn't really
> > care about pauth. So I'd rather we didn't release a QEMU 8.0
> > which crashes these affected deployed gdbs.
> >
>
> Are the affected gdb's packaged by distros? If so, a backport the distros can pick up
> will solve this in a quick package update.

Yes, it's exactly because these gdbs are distro-packaged
that I don't want QEMU to make them crash. I think it's
going to take a long time for the fix to go into gdb branches
and gdb to make a point release and distros to pick up that
point release, and in the meantime that's a lot of crashing
gdb bug reports that we're going to have to field.

> If we decide qemu should now emit a different xml for pauth, it will fix the crashes, but it also
> means older gdb's (9/10/11/12) will not be able to backtrace properly through pauth-signed frames.
>
> Maybe that's a reasonable drawback for qemu users?

"No backtrace through pauth frames" is the situation we've
been in ever since we implemented pauth in 2019, so I think
that's fine. It's not regressing something that used to work.

> If someone decides to implement a debugging stub that reports pauth (fast models, for example), it will
> also crash gdb, so I still plan to do the backport anyway.

If you're backporting the fix, you could also backport the
(hopefully tiny) change that says "treat pauth_v2 the same
way we do pauth", and then users with an updated older
gdb will also get working backtraces.

thanks
-- PMM

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-17 17:07                   ` Peter Maydell
@ 2023-03-17 17:12                     ` Luis Machado
  2023-03-17 17:16                       ` Luis Machado
  0 siblings, 1 reply; 9+ messages in thread
From: Luis Machado @ 2023-03-17 17:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Alex Bennée, Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

On 3/17/23 17:07, Peter Maydell wrote:
> On Fri, 17 Mar 2023 at 16:55, Luis Machado <luis.machado@arm.com> wrote:
>> On 3/17/23 16:37, Peter Maydell wrote:
>>> Having run into this problem in another couple of situations, one of
>>> which involved gdb 10, I think I'm increasingly favouring option
>>> 2 here. The affected gdbs seem to be quite widely deployed, and
>>> the bug results in crashes even for users who didn't really
>>> care about pauth. So I'd rather we didn't release a QEMU 8.0
>>> which crashes these affected deployed gdbs.
>>>
>>
>> Are the affected gdb's packaged by distros? If so, a backport the distros can pick up
>> will solve this in a quick package update.
>
> Yes, it's exactly because these gdbs are distro-packaged
> that I don't want QEMU to make them crash. I think it's
> going to take a long time for the fix to go into gdb branches
> and gdb to make a point release and distros to pick up that
> point release, and in the meantime that's a lot of crashing
> gdb bug reports that we're going to have to field.

Just to clarify, there won't be any point releases for gdb's 9/10/11/12.  We might have a bug fix
release for gdb 13 though (which isn't affected).

>
>> If we decide qemu should now emit a different xml for pauth, it will fix the crashes, but it also
>> means older gdb's (9/10/11/12) will not be able to backtrace properly through pauth-signed frames.
>>
>> Maybe that's a reasonable drawback for qemu users?
>
> "No backtrace through pauth frames" is the situation we've
> been in ever since we implemented pauth in 2019, so I think
> that's fine. It's not regressing something that used to work.
>

Fair enough.

>> If someone decides to implement a debugging stub that reports pauth (fast models, for example), it will
>> also crash gdb, so I still plan to do the backport anyway.
>
> If you're backporting the fix, you could also backport the
> (hopefully tiny) change that says "treat pauth_v2 the same
> way we do pauth", and then users with an updated older
> gdb will also get working backtraces.

I can negotiate that as well, though it borders being a new feature.

>
> thanks
> -- PMM

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests
  2023-03-17 17:12                     ` Luis Machado
@ 2023-03-17 17:16                       ` Luis Machado
  0 siblings, 0 replies; 9+ messages in thread
From: Luis Machado @ 2023-03-17 17:16 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Alex Bennée, Fabiano Rosas, qemu-devel, David Hildenbrand,
	Wainer dos Santos Moschetta, Richard Henderson, qemu-arm,
	Peter Xu, Philippe Mathieu-Daudé,
	Cleber Rosa, Thomas Huth, Paolo Bonzini, Beraldo Leal, gdb,
	Thiago Jung Bauermann, Omair Javaid

On 3/17/23 17:12, Luis Machado wrote:
> On 3/17/23 17:07, Peter Maydell wrote:
>> On Fri, 17 Mar 2023 at 16:55, Luis Machado <luis.machado@arm.com> wrote:
>>> On 3/17/23 16:37, Peter Maydell wrote:
>>>> Having run into this problem in another couple of situations, one of
>>>> which involved gdb 10, I think I'm increasingly favouring option
>>>> 2 here. The affected gdbs seem to be quite widely deployed, and
>>>> the bug results in crashes even for users who didn't really
>>>> care about pauth. So I'd rather we didn't release a QEMU 8.0
>>>> which crashes these affected deployed gdbs.
>>>>
>>>
>>> Are the affected gdb's packaged by distros? If so, a backport the distros can pick up
>>> will solve this in a quick package update.
>>
>> Yes, it's exactly because these gdbs are distro-packaged
>> that I don't want QEMU to make them crash. I think it's
>> going to take a long time for the fix to go into gdb branches
>> and gdb to make a point release and distros to pick up that
>> point release, and in the meantime that's a lot of crashing
>> gdb bug reports that we're going to have to field.
>
> Just to clarify, there won't be any point releases for gdb's 9/10/11/12.  We might have a bug fix
> release for gdb 13 though (which isn't affected).
>

Just to complement, my plan is to make the backports available (via stable branch commits) so distro package
maintainers can pick those up easily. No new releases will be made for older gdb's, so the package maintainers
can pick the backport up as soon as they are pushed. There won't be waiting for a new release of gdb.

>>
>>> If we decide qemu should now emit a different xml for pauth, it will fix the crashes, but it also
>>> means older gdb's (9/10/11/12) will not be able to backtrace properly through pauth-signed frames.
>>>
>>> Maybe that's a reasonable drawback for qemu users?
>>
>> "No backtrace through pauth frames" is the situation we've
>> been in ever since we implemented pauth in 2019, so I think
>> that's fine. It's not regressing something that used to work.
>>
>
> Fair enough.
>
>>> If someone decides to implement a debugging stub that reports pauth (fast models, for example), it will
>>> also crash gdb, so I still plan to do the backport anyway.
>>
>> If you're backporting the fix, you could also backport the
>> (hopefully tiny) change that says "treat pauth_v2 the same
>> way we do pauth", and then users with an updated older
>> gdb will also get working backtraces.
>
> I can negotiate that as well, though it borders being a new feature.
>
>>
>> thanks
>> -- PMM
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

end of thread, other threads:[~2023-03-17 17:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20230310103123.2118519-1-alex.bennee@linaro.org>
     [not found] ` <20230310103123.2118519-10-alex.bennee@linaro.org>
     [not found]   ` <CAFEAcA89K6_-Uc0XmEa1q+_z_yuppq1kvh=uPfv9V80MBH=aQg@mail.gmail.com>
     [not found]     ` <87wn3ocwqz.fsf@suse.de>
2023-03-10 18:14       ` [PATCH 09/11] tests/tcg: disable pauth for aarch64 gdb tests Alex Bennée
2023-03-13 11:22         ` Peter Maydell
2023-03-13 11:44           ` Luis Machado
2023-03-15  9:50             ` Luis Machado
2023-03-17 16:37               ` Peter Maydell
2023-03-17 16:55                 ` Luis Machado
2023-03-17 17:07                   ` Peter Maydell
2023-03-17 17:12                     ` Luis Machado
2023-03-17 17:16                       ` Luis Machado

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).