Skip to content

Commit c8906b3

Browse files
committed
[NFC][RemoteInspection] Add an opaque AddressSpace field to RemoteAddress
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
1 parent ec8caf3 commit c8906b3

File tree

16 files changed

+960
-700
lines changed

16 files changed

+960
-700
lines changed

include/swift/Basic/RelativePointer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@
132132
#ifndef SWIFT_BASIC_RELATIVEPOINTER_H
133133
#define SWIFT_BASIC_RELATIVEPOINTER_H
134134

135+
#include <cassert>
135136
#include <cstdint>
137+
#include <type_traits>
136138

137139
namespace swift {
138140

include/swift/Remote/CMemoryReader.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ class CMemoryReader final : public MemoryReader {
6868
// Check to see if an address has bits outside the ptrauth mask. This suggests
6969
// that we're likely failing to strip a signed pointer when reading from it.
7070
bool hasSignatureBits(RemoteAddress address) {
71-
uint64_t addressData = address.getAddressData();
72-
return addressData != (addressData & getPtrauthMask());
71+
return address != (address & getPtrauthMask());
7372
}
7473

7574
public:
@@ -88,7 +87,7 @@ class CMemoryReader final : public MemoryReader {
8887
RemoteAddress getSymbolAddress(const std::string &name) override {
8988
auto addressData = Impl.getSymbolAddress(Impl.reader_context,
9089
name.c_str(), name.size());
91-
return RemoteAddress(addressData);
90+
return RemoteAddress(addressData, RemoteAddress::DefaultAddressSpace);
9291
}
9392

9493
uint64_t getStringLength(RemoteAddress address) {
@@ -132,9 +131,13 @@ class CMemoryReader final : public MemoryReader {
132131
};
133132
return ReadBytesResult(Ptr, freeLambda);
134133
}
134+
RemoteAddress stripSignedPointer(RemoteAddress P) override {
135+
assert(P.getAddressSpace() == 0);
136+
auto PtrAuthMask = getPtrauthMask();
137+
return P & PtrAuthMask;
138+
}
135139
};
136-
137-
}
138-
}
140+
} // namespace remote
141+
} // namespace swift
139142

140143
#endif

include/swift/Remote/InProcessMemoryReader.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,18 @@ class InProcessMemoryReader final : public MemoryReader {
104104
ReadBytesResult readBytes(RemoteAddress address, uint64_t size) override {
105105
return ReadBytesResult(address.getLocalPointer<void>(), [](const void *) {});
106106
}
107+
108+
RemoteAddress stripSignedPointer(RemoteAddress P) override {
109+
assert(P.getAddressSpace() == 0);
110+
uintptr_t PtrAuthMask = 0;
111+
if (!queryDataLayout(DataLayoutQueryType::DLQ_GetPtrAuthMask, nullptr,
112+
&PtrAuthMask))
113+
return RemoteAddress();
114+
return P & PtrAuthMask;
115+
}
107116
};
108-
109-
}
110-
}
117+
118+
} // namespace remote
119+
} // namespace swift
111120

112121
#endif

include/swift/Remote/MemoryReader.h

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,30 @@ class MemoryReader {
5757
///
5858
/// Returns false if the operation failed.
5959
virtual bool readString(RemoteAddress address, std::string &dest) = 0;
60-
60+
61+
/// Attempts to read a remote address from the given address in the remote
62+
/// process.
63+
///
64+
/// Returns false if the operator failed.
65+
virtual bool readRemoteAddress(RemoteAddress address, RemoteAddress &out) {
66+
return readInteger(address, &out.getAddressDataReference());
67+
}
6168
/// Attempts to read an integer from the given address in the remote
6269
/// process.
6370
///
6471
/// Returns false if the operation failed.
6572
template <typename IntegerType>
6673
bool readInteger(RemoteAddress address, IntegerType *dest) {
74+
static_assert(!std::is_same<RemoteAddress, IntegerType>(),
75+
"RemoteAddress cannot be read in directly, use "
76+
"readRemoteAddress instead.");
77+
6778
return readBytes(address, reinterpret_cast<uint8_t*>(dest),
6879
sizeof(IntegerType));
6980
}
7081

82+
virtual RemoteAddress stripSignedPointer(RemoteAddress P) = 0;
83+
7184
/// Attempts to read an integer of the specified size from the given
7285
/// address in the remote process. Following `storeEnumElement`
7386
/// in EnumImpl.h, this reads arbitrary-size integers by ignoring
@@ -147,7 +160,8 @@ class MemoryReader {
147160
virtual RemoteAbsolutePointer resolvePointer(RemoteAddress address,
148161
uint64_t readValue) {
149162
// Default implementation returns the read value as is.
150-
return RemoteAbsolutePointer(RemoteAddress(readValue));
163+
return RemoteAbsolutePointer(
164+
RemoteAddress(readValue, address.getAddressSpace()));
151165
}
152166

153167
/// Performs the inverse operation of \ref resolvePointer.
@@ -263,7 +277,7 @@ class MemoryReader {
263277
virtual ~MemoryReader() = default;
264278
};
265279

266-
} // end namespace reflection
280+
} // end namespace remote
267281
} // end namespace swift
268282

269283
#endif // SWIFT_REFLECTION_READER_H

0 commit comments

Comments
 (0)