Skip to content

Advertise pointer authentication support to GDB#4073

Merged
khuey merged 2 commits into
rr-debugger:masterfrom
pcc:fix-pac-regs
May 13, 2026
Merged

Advertise pointer authentication support to GDB#4073
khuey merged 2 commits into
rr-debugger:masterfrom
pcc:fix-pac-regs

Conversation

@pcc
Copy link
Copy Markdown
Contributor

@pcc pcc commented May 13, 2026

GDB uses the values of the provided pointer authentication mask registers to mask return addresses from the stack trace. Because we weren't providing these masks, the return addresses in the stack trace were incorrect in programs built with -mbranch-protection=pac-ret, such as most of Fedora. Fix it by advertising pointer authentication to GDB, and providing the correct masks.

Fixes the following tests on PAC platforms:

fork_exec_info_thr
fork_exec_info_thr-no-syscallbuf
vdso_clock_gettime_stack
vdso_time_stack

@khuey
Copy link
Copy Markdown
Collaborator

khuey commented May 13, 2026

I think you forgot to git add pauth.xml.

Apparently that's already in the tree.

@pcc
Copy link
Copy Markdown
Contributor Author

pcc commented May 13, 2026

It's part of gdb

$ find | grep pauth.xml
./third-party/gdb/aarch64-pauth.xml

Comment thread src/ExtraRegisters.cc Outdated
Comment thread src/GdbServerConnection.cc Outdated
Comment thread src/Registers.cc Outdated
// PAuth masks. These aren't real registers, so we set the offset/size to 0
// and use custom handling in read_register().
RegisterInit(DREG_PAUTH_DMASK, RegisterValue("pauth_dmask", 0, 0)),
RegisterInit(DREG_PAUTH_CMASK, RegisterValue("pauth_cmask", 0, 0)),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this is needed. We don't need this for e.g. DREG_FPCR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not necessary, removed. I got confused and thought that ExtraRegisters was a subset of Registers when it's actually either/or.

We may consider removing the Registers code path and handling everything through what is now ExtraRegisters, which would be renamed to Registers. It would be a bit more code for each currently non-extra register but it would make it clearer how each register is handled.

GDB uses the values of the provided pointer authentication mask
registers to mask return addresses from the stack trace. Because we
weren't providing these masks, the return addresses in the stack trace
were incorrect in programs built with -mbranch-protection=pac-ret, such
as most of Fedora. Fix it by advertising pointer authentication to GDB,
and providing the correct masks.

Fixes the following tests on PAC platforms:

fork_exec_info_thr
fork_exec_info_thr-no-syscallbuf
vdso_clock_gettime_stack
vdso_time_stack
@pcc
Copy link
Copy Markdown
Contributor Author

pcc commented May 13, 2026

Looks like CI is failing (some of the tests SIGABRT) but I don't have any failures like that on my end. Would it be possible for someone with access to the CI to take a look?

@rocallahan
Copy link
Copy Markdown
Collaborator

rocallahan commented May 13, 2026

Would it be possible for someone with access to the CI to take a look?

I can do that, and I will try to do that soon. But it is relatively easy to replicate the CI environment. We use AWS c6g.8xlarge instances and run setup scripts scripts/github-actions-build.sh and scripts/github-actions-test.sh. Ubuntu 24 LTS.

@khuey
Copy link
Copy Markdown
Collaborator

khuey commented May 13, 2026

Program received signal SIGILL, Illegal instruction.
0x0000aaaaaae7e6e8 in rr::ExtraRegisters::read_register (this=0xaaaaab3b1a90, buf=0xffffffffc968 "",
    regno=rr::DREG_64_YMM8H, defined=0xffffffffc990) at /home/ubuntu/rr/src/ExtraRegisters.cc:284
284             __asm__ __volatile__("xpacd %0" : "+r"(ptr));

CI runs on Graviton2 and it looks like pointer authentication was added in Graviton3.

@khuey
Copy link
Copy Markdown
Collaborator

khuey commented May 13, 2026

Ignore my previous comment about HWCAP2. That's what I get for not double checking Gemini.

The cset I just pushed should fix the CI issues.

@khuey khuey merged commit b835bbd into rr-debugger:master May 13, 2026
7 of 9 checks passed
@khuey
Copy link
Copy Markdown
Collaborator

khuey commented May 13, 2026

Thanks for the PR!

@pcc
Copy link
Copy Markdown
Contributor Author

pcc commented May 13, 2026

CI runs on Graviton2 and it looks like pointer authentication was added in Graviton3.

So does it mean that we shouldn't be building with -march=armv8.3-a?

@khuey
Copy link
Copy Markdown
Collaborator

khuey commented May 13, 2026

@rocallahan We should probably bump the runners to Graviton3 instances.

@rocallahan
Copy link
Copy Markdown
Collaborator

@rocallahan We should probably bump the runners to Graviton3 instances.

Maybe we should have both?

@khuey
Copy link
Copy Markdown
Collaborator

khuey commented May 14, 2026

@rocallahan We should probably bump the runners to Graviton3 instances.

Maybe we should have both?

Well we're building with -march=armv8.3-a but running on a processor that only supports the v8.2-A ISA.

@rocallahan
Copy link
Copy Markdown
Collaborator

Ok I updated the runner function to spawn m7g.8xlarge instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants