🍒 [lldb] Cherry pick ptr metadata work from upstream #11493
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, 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 077e351)
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) (cherry picked from commit 8cc7914)
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) (cherry picked from commit c507ec1)
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)
(cherry picked from commit 31724b1)
felipepiovezan
changed the title
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.