Skip to content

[RemoteInspection] Change RemoteAbsolutePointer (NFC) #82325

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+46 −70
Open

[RemoteInspection] Change RemoteAbsolutePointer (NFC) #82325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/swift/Remote/MemoryReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class MemoryReader {
virtual RemoteAbsolutePointer resolvePointer(RemoteAddress address,
uint64_t readValue) {
// Default implementation returns the read value as is.
return RemoteAbsolutePointer("", readValue);
return RemoteAbsolutePointer(RemoteAddress(readValue));
}

/// Performs the inverse operation of \ref resolvePointer.
Expand All @@ -166,7 +166,7 @@ class MemoryReader {
virtual RemoteAbsolutePointer getSymbol(RemoteAddress address) {
if (auto symbol = resolvePointerAsSymbol(address))
return *symbol;
return RemoteAbsolutePointer("", address.getAddressData());
return RemoteAbsolutePointer(address);
}

/// Lookup a dynamic symbol name (ie dynamic loader binding) for the given
Expand Down
44 changes: 11 additions & 33 deletions include/swift/Remote/MetadataReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,9 @@ class MetadataReader {
}

RemoteAbsolutePointer stripSignedPointer(const RemoteAbsolutePointer &P) {
if (P.isResolved()) {
return RemoteAbsolutePointer("",
P.getResolvedAddress().getAddressData() & PtrAuthMask);
}
return P;
return RemoteAbsolutePointer(
P.getSymbol(), P.getOffset(),
RemoteAddress(P.getResolvedAddress().getAddressData() & PtrAuthMask));
}

StoredPointer queryPtrAuthMask() {
Expand Down Expand Up @@ -519,29 +517,13 @@ class MetadataReader {
// The second entry is a relative address to the mangled protocol
// without symbolic references.

// lldb might return an unresolved remote absolute pointer from its
// resolvePointerAsSymbol implementation -- workaround this.
if (!resolved.isResolved()) {
auto remoteAddr = RemoteAddress(remoteAddress);
resolved =
RemoteAbsolutePointer("", remoteAddr.getAddressData());
}

auto addr =
resolved.getResolvedAddress().getAddressData() + sizeof(int32_t);
int32_t offset;
Reader->readInteger(RemoteAddress(addr), &offset);
auto addrOfTypeRef = addr + offset;
resolved = Reader->getSymbol(RemoteAddress(addrOfTypeRef));

// lldb might return an unresolved remote absolute pointer from its
// resolvePointerAsSymbol implementation -- workaround this.
if (!resolved.isResolved()) {
auto remoteAddr = RemoteAddress(addrOfTypeRef);
resolved =
RemoteAbsolutePointer("", remoteAddr.getAddressData());
}

// Dig out the protocol from the protocol list.
auto protocolList = readMangledName(resolved.getResolvedAddress(),
MangledNameKind::Type, dem);
Expand Down Expand Up @@ -1379,12 +1361,10 @@ class MetadataReader {
ParentContextDescriptorRef
readContextDescriptor(const RemoteAbsolutePointer &address) {
// Map an unresolved pointer to an unresolved context ref.
if (!address.isResolved()) {
if (!address.getSymbol().empty()) {
// We can only handle references to a symbol without an offset currently.
if (address.getOffset() != 0) {
return ParentContextDescriptorRef();
}
return ParentContextDescriptorRef(address.getSymbol());
if (address.getOffset() == 0)
return ParentContextDescriptorRef(address.getSymbol());
}

return ParentContextDescriptorRef(
Expand Down Expand Up @@ -2016,7 +1996,7 @@ class MetadataReader {

std::optional<StoredPointer> readResolvedPointerValue(StoredPointer address) {
if (auto pointer = readPointer(address)) {
if (!pointer->isResolved())
if (!pointer->getResolvedAddress())
return std::nullopt;
return (StoredPointer)pointer->getResolvedAddress().getAddressData();
}
Expand Down Expand Up @@ -2079,7 +2059,7 @@ class MetadataReader {
return std::nullopt;
}

return RemoteAbsolutePointer("", resultAddress);
return RemoteAbsolutePointer(RemoteAddress(resultAddress));
}

/// Given a pointer to an Objective-C class, try to read its class name.
Expand Down Expand Up @@ -2335,13 +2315,11 @@ class MetadataReader {
auto parentAddress = resolveRelativeIndirectableField(base, base->Parent);
if (!parentAddress)
return std::nullopt;
if (!parentAddress->isResolved()) {
if (!parentAddress->getSymbol().empty()) {
// Currently we can only handle references directly to a symbol without
// an offset.
if (parentAddress->getOffset() != 0) {
return std::nullopt;
}
return ParentContextDescriptorRef(parentAddress->getSymbol());
if (parentAddress->getOffset() == 0)
return ParentContextDescriptorRef(parentAddress->getSymbol());
}
auto addr = parentAddress->getResolvedAddress();
if (!addr)
Expand Down
41 changes: 18 additions & 23 deletions include/swift/Remote/RemoteAddress.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,35 +63,30 @@ class RemoteAddress {

/// A symbolic relocated absolute pointer value.
class RemoteAbsolutePointer {
/// The symbol name that the pointer refers to. Empty if the value is absolute.
/// The symbol name that the pointer refers to. Empty if only an absolute
/// address is available.
std::string Symbol;
/// The offset from the symbol, or the resolved remote address if \c Symbol is empty.
int64_t Offset;
/// The offset from the symbol.
int64_t Offset = 0;
/// The resolved remote address.
RemoteAddress Address = RemoteAddress{(uint64_t)0};

public:
RemoteAbsolutePointer()
: Symbol(), Offset(0)
{}

RemoteAbsolutePointer(std::nullptr_t)
: RemoteAbsolutePointer()
{}

RemoteAbsolutePointer(llvm::StringRef Symbol, int64_t Offset)
: Symbol(Symbol), Offset(Offset)
{}

bool isResolved() const { return Symbol.empty(); }
RemoteAbsolutePointer() = default;
RemoteAbsolutePointer(std::nullptr_t) : RemoteAbsolutePointer() {}

RemoteAbsolutePointer(llvm::StringRef Symbol, int64_t Offset,
RemoteAddress Address)
: Symbol(Symbol), Offset(Offset), Address(Address) {}
RemoteAbsolutePointer(RemoteAddress Address) : Address(Address) {}

llvm::StringRef getSymbol() const { return Symbol; }
int64_t getOffset() const { return Offset; }

RemoteAddress getResolvedAddress() const {
assert(isResolved());
return RemoteAddress(Offset);
}


RemoteAddress getResolvedAddress() const { return Address; }

explicit operator bool() const {
return Offset != 0 || !Symbol.empty();
return Address || !Symbol.empty();
}
};

Expand Down
2 changes: 1 addition & 1 deletion include/swift/RemoteInspection/ReflectionContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ class ReflectionContext
auto CDAddr = this->readCaptureDescriptorFromMetadata(*MetadataAddress);
if (!CDAddr)
return nullptr;
if (!CDAddr->isResolved())
if (!CDAddr->getResolvedAddress())
return nullptr;

// FIXME: Non-generic SIL boxes also use the HeapLocalVariable metadata
Expand Down
14 changes: 8 additions & 6 deletions include/swift/RemoteInspection/TypeRefBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1871,7 +1871,7 @@ class TypeRefBuilder {
if (auto symbol = OpaquePointerReader(
remote::RemoteAddress(adjustedProtocolDescriptorTarget),
PointerSize)) {
if (!symbol->getSymbol().empty()) {
if (!symbol->getSymbol().empty() && symbol->getOffset() == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why must the offset be 0 in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the other comments the combination of Symbol+Offset isn't actually implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just took this for granted without further investigation, but given that the Offset is never taken here, it seems plausible.

Demangle::Context Ctx;
auto demangledRoot =
Ctx.demangleSymbolAsNode(symbol->getSymbol().str());
Expand All @@ -1882,7 +1882,8 @@ class TypeRefBuilder {
nodeToString(demangledRoot->getChild(0)->getChild(0));
} else {
// This is an absolute address of a protocol descriptor
auto protocolDescriptorAddress = (uintptr_t)symbol->getOffset();
auto protocolDescriptorAddress =
(uintptr_t)symbol->getResolvedAddress().getAddressData();
protocolName = readFullyQualifiedProtocolNameFromProtocolDescriptor(
protocolDescriptorAddress);
}
Expand Down Expand Up @@ -2026,7 +2027,7 @@ class TypeRefBuilder {
if (auto symbol = OpaquePointerReader(
remote::RemoteAddress(adjustedParentTargetAddress),
PointerSize)) {
if (!symbol->getSymbol().empty()) {
if (!symbol->getSymbol().empty() && symbol->getOffset() == 0) {
Demangle::Context Ctx;
auto demangledRoot =
Ctx.demangleSymbolAsNode(symbol->getSymbol().str());
Expand Down Expand Up @@ -2264,7 +2265,7 @@ class TypeRefBuilder {
// external, check that first
if (auto symbol = OpaqueDynamicSymbolResolver(
remote::RemoteAddress(contextTypeDescriptorAddress))) {
if (!symbol->isResolved()) {
if (!symbol->getSymbol().empty() && symbol->getOffset() == 0) {
Demangle::Context Ctx;
auto demangledRoot =
Ctx.demangleSymbolAsNode(symbol->getSymbol().str());
Expand All @@ -2283,10 +2284,11 @@ class TypeRefBuilder {
mangledTypeName = typeMangling.result();

return std::make_pair(mangledTypeName, typeName);
} else if (symbol->getOffset()) {
} else if (symbol->getResolvedAddress()) {
// If symbol is empty and has an offset, this is the resolved remote
// address
contextTypeDescriptorAddress = symbol->getOffset();
contextTypeDescriptorAddress =
symbol->getResolvedAddress().getAddressData();
}
}

Expand Down
11 changes: 6 additions & 5 deletions lib/StaticMirror/ObjectFileContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ Image::resolvePointer(uint64_t Addr, uint64_t pointerValue) const {
// 32 bits.
if (isMachOWithPtrAuth()) {
return remote::RemoteAbsolutePointer(
"", HeaderAddress + (pointerValue & 0xffffffffull));
remote::RemoteAddress(HeaderAddress + (pointerValue & 0xffffffffull)));
} else {
return remote::RemoteAbsolutePointer("", pointerValue);
return remote::RemoteAbsolutePointer(remote::RemoteAddress(pointerValue));
}
}

Expand All @@ -333,7 +333,8 @@ remote::RemoteAbsolutePointer Image::getDynamicSymbol(uint64_t Addr) const {
if (found == DynamicRelocations.end())
return nullptr;
return remote::RemoteAbsolutePointer(found->second.Symbol,
found->second.Offset);
found->second.Offset,
remote::RemoteAddress((uint64_t)0));
}

std::pair<const Image *, uint64_t>
Expand Down Expand Up @@ -526,8 +527,8 @@ ObjectMemoryReader::resolvePointer(reflection::RemoteAddress Addr,
// Mix in the image index again to produce a remote address pointing into the
// same image.
return remote::RemoteAbsolutePointer(
"", encodeImageIndexAndAddress(
image, resolved.getResolvedAddress().getAddressData()));
remote::RemoteAddress(encodeImageIndexAndAddress(
image, resolved.getResolvedAddress().getAddressData())));
}

remote::RemoteAbsolutePointer
Expand Down