| Message ID | 20230310103123.2118519-10-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tweaks and fixes for 8.0-rc1 (tests, plugins, docs) | expand |
On 3/10/23 02:31, Alex Bennée 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> > --- > tests/tcg/aarch64/Makefile.target | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > > diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target > index 9e91a20b0d..8ffde3b0ed 100644 > --- a/tests/tcg/aarch64/Makefile.target > +++ b/tests/tcg/aarch64/Makefile.target > @@ -84,6 +84,8 @@ TESTS += sha512-vector > ifeq ($(HOST_GDB_SUPPORTS_ARCH),y) > GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py > > +run-gdbstub-%: QEMU_OPTS=-cpu max,pauth=off > + > run-gdbstub-sysregs: sysregs > $(call run-test, $@, $(GDB_SCRIPT) \ > --gdb $(HAVE_GDB_BIN) \
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... -- PMM
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.
On Fri, 10 Mar 2023 at 18:00, Fabiano Rosas <farosas@suse.de> wrote: > > 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. But then presumably a pauth-aware GDB won't actually know the values it needs to be able to convert between with-PAC and without-PAC addresses for backtracing? Luis, how is this intended to work? Is there some way the stub can check with the gdb that's connected whether the gdb is able to cope with the pauth XML, so it can avoid sending it to a gdb that is going to crash if it sees it ? thanks -- PMM
On 3/10/23 09:47, Peter Maydell wrote: > 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... It is, annoyingly, a bug in gdb 12 alone. Before gdb 12, the pauth extension isn't recognized and so it gets treated as non-special registers. From gdb 13, whatever lead to the internal_error() is fixed and the extension works swimmingly. r~
(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.
Peter Maydell <peter.maydell@linaro.org> writes: > On Fri, 10 Mar 2023 at 18:00, Fabiano Rosas <farosas@suse.de> wrote: >> >> 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. > > But then presumably a pauth-aware GDB won't actually know > the values it needs to be able to convert between with-PAC > and without-PAC addresses for backtracing? > Good question. Although that feels to me more like a GDB feature. If we don't break it even worse by doing that, the QEMU side which is more about reading the registers should be fine. Note that we already have other .xml files using a .qemu namespace in the codebase. As I understand it, gdb simply treats these as extra registers not tied to any specific feature.
On 3/10/23 18:07, Peter Maydell wrote: > On Fri, 10 Mar 2023 at 18:00, Fabiano Rosas <farosas@suse.de> wrote: >> >> 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. It might be a better option to emit a pauth feature in the qemu namespace to dodge the crashing bug from older gdb's (a latent pauth-related bug in gdb that is triggered by having gdb identify that a target supports pauth and at the same time having a target description containing system registers gdb doesn't care about). > > But then presumably a pauth-aware GDB won't actually know > the values it needs to be able to convert between with-PAC > and without-PAC addresses for backtracing? > > Luis, how is this intended to work? Is there some way the > stub can check with the gdb that's connected whether the > gdb is able to cope with the pauth XML, so it can avoid > sending it to a gdb that is going to crash if it sees it ? There isn't a probing mechanism unfortunately, and gdb isn't supposed to crash in this case. With the changes from commit 6d0020873deb2f2c4e0965dc2ebf227bc1db3140, gdb now unmasks signed addresses using the additional pauth registers. If gdb doesn't detect the pauth feature, it will still mask out the top bits using a default mask of 0xff80000000000000. While that should be enough for user addresses, it won't help with "kernel" addresses (when the VA select bit is 1). To dodge the crashing bug of older gdb's, I can adjust gdb to also look for the pauth registers in the qemu namespace and document that accordingly. > > 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.
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
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.
On 3/13/23 04:44, Luis Machado wrote: >> 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. What if we leave the original two registers, pauth_[cd]mask, in org.gnu.gdb.aarch64.pauth and move the new *_high registers into a different feature? That would maximize the set of gdb version for which the original user-only support is functional. r~
On Mon, 13 Mar 2023 at 19:21, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 3/13/23 04:44, Luis Machado wrote: > >> 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. > > What if we leave the original two registers, pauth_[cd]mask, in org.gnu.gdb.aarch64.pauth > and move the new *_high registers into a different feature? That would maximize the set > of gdb version for which the original user-only support is functional. If that avoids the gdb crash, sure. But I had the impression from Luis' description of it that that would not help (i.e. that it was the not-used-by-gdb registers in other XML sections like sysregs that resulted in it getting confused about the register number for its internal pauth-related register). thanks -- PMM
On 3/13/23 19:21, Richard Henderson wrote: > On 3/13/23 04:44, Luis Machado wrote: >>> 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. > > What if we leave the original two registers, pauth_[cd]mask, in org.gnu.gdb.aarch64.pauth and move the new *_high registers into a different feature? That would maximize the set of gdb version for which the original user-only support is functional. From what I recall from the gdb bug, I don't think that will help. gdb will detect pauth support, will add the ra_sign_state pseudo-register internally with the incorrect numbering and will also see the additional system registers from QEMU, resulting in a crash. > > > r~ > 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.
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.
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
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.
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
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.
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.
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index 9e91a20b0d..8ffde3b0ed 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -84,6 +84,8 @@ TESTS += sha512-vector ifeq ($(HOST_GDB_SUPPORTS_ARCH),y) GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py +run-gdbstub-%: QEMU_OPTS=-cpu max,pauth=off + run-gdbstub-sysregs: sysregs $(call run-test, $@, $(GDB_SCRIPT) \ --gdb $(HAVE_GDB_BIN) \
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> --- tests/tcg/aarch64/Makefile.target | 2 ++ 1 file changed, 2 insertions(+)