Skip to content

[NFC][RemoteInspection] Add an opaque AddressSpace field to RemoteAddress #82638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 10, 2025

Conversation

augusto2112
Copy link
Contributor

@augusto2112 augusto2112 commented Jun 30, 2025

Add an extra opaque field to AddressSpace, which can be used by clients
of RemoteInspection to distinguish between different address spaces.

LLDB employs an optimization where it reads memory from files instead of
the running process whenever it can to speed up memory reads (these can
be slow when debugging something over a network). To do this, it needs
to keep track whether an address originated from a process or a file. It
currently distinguishes addresses by setting an unused high bit on the
address, but because of pointer authentication this is not a reliable
solution. In order to keep this optimization working, this patch adds an
extra opaque AddressSpace field to RemoteAddress, which LLDB can use on
its own implementation of MemoryReader to distinguish between addresses.

This patch is NFC for the other RemoteInspection clients, as it adds
extra information to RemoteAddress, which is entirely optional and if
unused should not change the behavior of the library.

Although this patch is quite big the changes are largely mechanical,
replacing threading StoredPointer with RemoteAddress.

rdar://148361743

@augusto2112
Copy link
Contributor Author

@augusto2112 augusto2112 force-pushed the change-remote-addr branch from 2133870 to b031186 Compare July 3, 2025 23:11
@augusto2112 augusto2112 requested a review from adrian-prantl July 3, 2025 23:12
@augusto2112 augusto2112 marked this pull request as ready for review July 3, 2025 23:12
@augusto2112 augusto2112 requested review from slavapestov, a team and DougGregor as code owners July 3, 2025 23:12
@augusto2112 augusto2112 requested review from tbkka and mikeash July 3, 2025 23:13
@augusto2112 augusto2112 force-pushed the change-remote-addr branch from b031186 to c8906b3 Compare July 3, 2025 23:42
@augusto2112 augusto2112 changed the title [DRAFT][RemoteInspection] Replace StoredPointer with RemoteAddress [NFC][RemoteInspection] Add an opaque AddressSpace field to RemoteAddress Jul 3, 2025
@augusto2112 augusto2112 requested a review from al45tair July 3, 2025 23:43
@augusto2112
Copy link
Contributor Author

@augusto2112 augusto2112 force-pushed the change-remote-addr branch from c8906b3 to 434fe88 Compare July 7, 2025 17:24
@augusto2112
Copy link
Contributor Author

@augusto2112 augusto2112 force-pushed the change-remote-addr branch from 434fe88 to 0368168 Compare July 7, 2025 19:55
@augusto2112 augusto2112 force-pushed the change-remote-addr branch from 0368168 to faa6b7d Compare July 8, 2025 14:58
@augusto2112
Copy link
Contributor Author

@augusto2112 augusto2112 force-pushed the change-remote-addr branch from faa6b7d to 83096c8 Compare July 9, 2025 00:14
@augusto2112
Copy link
Contributor Author

@augusto2112 augusto2112 force-pushed the change-remote-addr branch from 83096c8 to f402316 Compare July 9, 2025 19:59
@augusto2112
Copy link
Contributor Author

@augusto2112 augusto2112 force-pushed the change-remote-addr branch from f402316 to e6d807a Compare July 9, 2025 20:10
…ress

Add an extra opaque field to AddressSpace, which can be used by clients
of RemoteInspection to distinguish between different address spaces.

LLDB employs an optimization where it reads memory from files instead of
the running process whenever it can to speed up memory reads (these can
be slow when debugging something over a network). To do this, it needs
to keep track whether an address originated from a process or a file. It
currently distinguishes addresses by setting an unused high bit on the
address, but because of pointer authentication this is not a reliable
solution. In order to keep this optimization working, this patch adds an
extra opaque AddressSpace field to RemoteAddress, which LLDB can use on
its own implementation of MemoryReader to distinguish between addresses.

This patch is NFC for the other RemoteInspection clients, as it adds
extra information to RemoteAddress, which is entirely optional and if
unused should not change the behavior of the library.

Although this patch is quite big the changes are largely mechanical,
replacing threading StoredPointer with RemoteAddress.

rdar://148361743
@augusto2112 augusto2112 force-pushed the change-remote-addr branch from e6d807a to 58df553 Compare July 9, 2025 21:52
@augusto2112
Copy link
Contributor Author

1 similar comment
@augusto2112
Copy link
Contributor Author

@augusto2112 augusto2112 merged commit 21ded5c into swiftlang:main Jul 10, 2025
5 checks passed
@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Jul 10, 2025

@augusto2112 Could this PR have broken the build here? https://ci.swift.org/job/swift-PR-Linux-smoke-test/21185/console

16:25:11  /home/build-user/llvm-project/lldb/source/Plugins/LanguageRuntime/Swift/SwiftMetadataCache.cpp:175:36: error: no member named 'getAddressData' in 'swift::remote::RemoteRef<swift::reflection::TargetFieldDescriptor<swift::InProcess>>'
16:25:11      auto offset = field_descriptor.getAddressData() -
16:25:11                    ~~~~~~~~~~~~~~~~ ^
16:25:11  /home/build-user/llvm-project/lldb/source/Plugins/LanguageRuntime/Swift/SwiftMetadataCache.cpp:176:52: error: no member named 'getAddressData' in 'swift::remote::RemoteRef<void>'
16:25:11                    field_descriptors.startAddress().getAddressData();
16:25:11                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
16:25:11  2 errors generated.
16:25:11  [502/1131][ 44%][86.096s] B

@augusto2112
Copy link
Contributor Author

@augusto2112 Could this PR have broken the build here? https://ci.swift.org/job/swift-PR-Linux-smoke-test/21185/console

16:25:11  /home/build-user/llvm-project/lldb/source/Plugins/LanguageRuntime/Swift/SwiftMetadataCache.cpp:175:36: error: no member named 'getAddressData' in 'swift::remote::RemoteRef<swift::reflection::TargetFieldDescriptor<swift::InProcess>>'
16:25:11      auto offset = field_descriptor.getAddressData() -
16:25:11                    ~~~~~~~~~~~~~~~~ ^
16:25:11  /home/build-user/llvm-project/lldb/source/Plugins/LanguageRuntime/Swift/SwiftMetadataCache.cpp:176:52: error: no member named 'getAddressData' in 'swift::remote::RemoteRef<void>'
16:25:11                    field_descriptors.startAddress().getAddressData();
16:25:11                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
16:25:11  2 errors generated.
16:25:11  [502/1131][ 44%][86.096s] B

There's an accompanying LLDB PR (swiftlang/llvm-project#10928). The CI job probably picked up this commit but not the LLDB commit. It just started a new job which I assume should be fine now.

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.

4 participants