[lldb] Cherry-pick patches for pointer authentication support in 6.2.1 #11456
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In architectures where pointers may contain metadata, such as arm64e, it is important to ignore those bits when comparing two different StackIDs, as the metadata may not be the same even if the pointers are. This patch is a step towards allowing consumers of pointers to decide whether they want to keep or remove metadata, as opposed to discarding metadata at the moment pointers are created. See llvm#150537. This was tested running the LLDB test suite on arm64e. (cherry picked from commit d662d77)
In architectures where pointers may contain metadata, such as arm64e, the metadata may need to be cleaned prior to sending this pointer to be used in expression evaluation generated code. This patch is a step towards allowing consumers of pointers to decide whether they want to keep or remove metadata, as opposed to discarding metadata at the moment pointers are created. See llvm#150537. This was tested running the LLDB test suite on arm64e. (The first attempt at this patch caused a failure in TestScriptedProcessEmptyMemoryRegion.py. This test exercises a case where IRMemoryMap uses host memory in its allocations; pointers to such allocations should not be fixed, which is what the original patch failed to account for). (cherry picked from commit f88eadd)
(cherry picked from commit bbdcc47)
…nwindAllInstructions In arm64, this is number is 0.102, which is barely above 0.1 (cherry picked from commit bfaa20a)
This test was designed to distinguish the case that doesn't work on this architecture. (cherry picked from commit 04fdf8d)
This was disabling the test for the wrong architecture. (cherry picked from commit 5a93cad)
This test jumps to address 0 and expects to get a bad frame on top of the stack. However, on arm64e this will fail while still on main because pointer authentication will fail. (cherry picked from commit 2adf931)
Some comments were "suffixed" to member variable declarations; these are moved to before the variable. Some constructors and operators were just defaulted and not necessary. Some comments dividing the class into logical sections, like "// constructors and destructors", were not applied everywhere. These were removed. They are used in some parts of LLDB, but are the exception. An include was not needed. The operator != can be defined in terms of ==. (cherry picked from commit 54b3dc1)
One of those arguments should be called `pointer` to correlate it to the name of the function and to distinguish it from the address where it will be written. (cherry picked from commit d367c7d)
…inters By always stripping pointers in the test driver, the test will work regardless of architecture. (cherry picked from commit 761ac92)
This is mostly a cosmetic choice (and also to fix test expectations). Users don't need the metadata when running `task` commands that expect a pointer. (cherry picked from commit 92fb650)
In this commit: 9c8e716 [lldb] Make StackID call Fix{Code,Data} pointers (llvm#152796) We made StackID keep track of the CFA without any pointer metadata in it. This is necessary when comparing two StackIDs to determine which one is "younger". However, the CFA inside StackIDs is also used in other contexts through the method StackID::GetCallFrameAddress. One notable case is DWARFExpression: the computation of `DW_OP_call_frame_address` is done using StackID. This feeds into many other places, e.g. expression evaluation may require the address of a variable that is computed from the CFA; to access the variable without faulting, we may need to preserve the pointer metadata. As such, StackID must be able to provide both versions of the CFA. In the spirit of allowing consumers of pointers to decide what to do with pointer metadata, this patch changes StackID to store both versions of the cfa pointer. Two getter methods are provided, and all call sites except DWARFExpression preserve their existing behavior (stripped pointer). Other alternatives were considered: * Just store the raw pointer. This would require changing the comparisong operator `<` to also receive a Process, as the comparison requires stripped pointers. It wasn't clear if all call-sites had a non-null process, whereas we know we have a process when creating a StackID. * Store a weak pointer to the process inside the class, and then strip metadata as needed. This would require a `weak_ptr::lock` in many operations of LLDB, and it felt wasteful. It also prevents stripping of the pointer if the process has gone away. This patch also changes RegisterContextUnwind::ReadFrameAddress, which is the method computing the CFA fed into StackID, to also preserve the signature pointers. (cherry picked from commit 5d088ba)
We needed to strip more pointers in the test. (cherry picked from commit 62d9ce2)
…lts (llvm#159460)"" The original had an issue on "AArch-less" bots. Fixed it with some ifdefs around the presence of the AArch ABI plugin. Note for the cherry-pick: the test was removed as the related test file in this branch is too old. This reverts commit 1a4685d. Cherry-picked from 40eb976.
This requires fixes from upstream llvm to work.
The first call, in InitializeNonZerothFrame, is inside a logging branch. For that, it's better to log the real value instead of the fixed one. The second call, inside RegisterContextUnwind::ReadFrameAddress, is computing an address which is then passed to ReadRegisterValueFromMemory, which boils down to a Process::ReadMemory, which fixes the address if it wants to. The current variable names are misleading, making readers believe it is the cfa value itself that is being passed to Fix*Address; that's not the case. This commit renames the variable to make this abundantly clear. (cherry picked from commit 113357f)
…m#159606) Based on testing on processors that use pointer metadata, and with all the work done to delay calls to FixDataAddress, this is no longer necessary. Note that, with debugserver in particular, this is an NFC change: the code path here is for frame zero, and debugserver will strip metadata when reading fp from frame zero anyway. (cherry picked from commit bce48c8)
This is yet another variant of the Fix{Code,Data}Address methods, but
tailored for pointers that both:
1. Are going to be used in-process,
2. Require authentication metadata.
Currently, the callsite inside IRMemoryMap::WritePointerToMemory is an
example of 1; the pointer written to memory will be used by JITed code
during expression evaluation.
An example of (2) can be found in the MTE extension on arm processors.
An MTE-tagged pointer must preserve its normal bits in order for load
instructions to complete without faulting. However, PAC bits must be
stripped, as codegen for some expressions may generate regular load
instructions for accesses to those (instead of the special PAC
instructions).
(cherry picked from commit 37ad33e39ac960178e4cf02e5598db35a279ae21)
|
@swift-ci test |
adrian-prantl
approved these changes
Sep 19, 2025
|
|
||
| class TestDAP_stackTraceMissingFunctionName(lldbdap_testcase.DAPTestCaseBase): | ||
| @skipIfWindows | ||
| # Jumping to address 0 will fail PAC signing before crashign on a bad frame. |
There was a problem hiding this comment.
Suggested change
| # Jumping to address 0 will fail PAC signing before crashign on a bad frame. | |
| # Jumping to address 0 will fail PAC signing before crashing on a bad frame. |
There was a problem hiding this comment.
I'll fix this in 21.x & next if that's ok, to avoid having to do a rerun of CI. (unless we find more things to change)
JDevlieghere
approved these changes
Sep 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
rdar://156639763