Skip to content

[lldb] Add an extra optional did_read_live_memory to Target::ReadMemory and [lldb] Implement LLDBMemoryReader::readRemoteAddressImpl #11016

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions lldb/include/lldb/Target/Target.h
Original file line number Diff line number Diff line change
Expand Up @@ -1193,11 +1193,14 @@ class Target : public std::enable_shared_from_this<Target>,
// 2 - if there is a process, then read from memory
// 3 - if there is no process, then read from the file cache
//
// The optional did_read_live_memory parameter will be set true if live memory
// was read successfully.
//
// The method is virtual for mocking in the unit tests.
virtual size_t ReadMemory(const Address &addr, void *dst, size_t dst_len,
Status &error, bool force_live_memory = false,
lldb::addr_t *load_addr_ptr = nullptr);

lldb::addr_t *load_addr_ptr = nullptr,
bool *did_read_live_memory = nullptr);
size_t ReadCStringFromMemory(const Address &addr, std::string &out_str,
Status &error, bool force_live_memory = false);

Expand Down
67 changes: 50 additions & 17 deletions lldb/source/Plugins/LanguageRuntime/Swift/LLDBMemoryReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ LLDBMemoryReader::resolvePointer(swift::remote::RemoteAddress address,
// to a tagged address so further memory reads originating from it benefit
// from the file-cache optimization.
swift::remote::RemoteAbsolutePointer process_pointer{
swift::remote::RemoteAddress{readValue, address.getAddressSpace()}};
swift::remote::RemoteAddress{
readValue, swift::remote::RemoteAddress::DefaultAddressSpace}};

if (!readMetadataFromFileCacheEnabled()) {
assert(address.getAddressSpace() ==
Expand Down Expand Up @@ -314,6 +315,13 @@ LLDBMemoryReader::resolvePointer(swift::remote::RemoteAddress address,

bool LLDBMemoryReader::readBytes(swift::remote::RemoteAddress address,
uint8_t *dest, uint64_t size) {
auto [success, _] = readBytesImpl(address, dest, size);
return success;
}

std::pair<bool, bool>
LLDBMemoryReader::readBytesImpl(swift::remote::RemoteAddress address,
uint8_t *dest, uint64_t size) {
Log *log = GetLog(LLDBLog::Types);
if (m_local_buffer) {
bool overflow = false;
Expand All @@ -322,15 +330,15 @@ bool LLDBMemoryReader::readBytes(swift::remote::RemoteAddress address,
if (overflow) {
LLDB_LOGV(log, "[MemoryReader] address {0:x} + size {1} overflows", addr,
size);
return false;
return {false, false};
}
if (addr >= *m_local_buffer &&
end <= *m_local_buffer + m_local_buffer_size) {
// If this crashes, the assumptions stated in
// GetDynamicTypeAndAddress_Protocol() most likely no longer
// hold.
memcpy(dest, (void *)addr, size);
return true;
return {true, false};
}
}

Expand All @@ -345,7 +353,7 @@ bool LLDBMemoryReader::readBytes(swift::remote::RemoteAddress address,
if (!maybeAddr) {
LLDB_LOGV(log, "[MemoryReader] could not resolve address {0:x}",
address.getRawAddress());
return false;
return {false, false};
}
auto addr = *maybeAddr;
if (addr.IsSectionOffset()) {
Expand All @@ -354,30 +362,34 @@ bool LLDBMemoryReader::readBytes(swift::remote::RemoteAddress address,
if (object_file->GetType() == ObjectFile::Type::eTypeDebugInfo) {
LLDB_LOGV(log, "[MemoryReader] Reading memory from symbol rich binary");

return object_file->ReadSectionData(section.get(), addr.GetOffset(), dest,
size);
bool success = object_file->ReadSectionData(section.get(),
addr.GetOffset(), dest, size);
return {success, false};
}
}

if (size > m_max_read_amount) {
LLDB_LOGV(log, "[MemoryReader] memory read exceeds maximum allowed size");
return false;
return {false, false};
}
Target &target(m_process.GetTarget());
Status error;
// We only want to allow the file-cache optimization if we resolved the
// address to section + offset.
const bool force_live_memory =
!readMetadataFromFileCacheEnabled() || !addr.IsSectionOffset();
if (size > target.ReadMemory(addr, dest, size, error, force_live_memory)) {
bool did_read_live_memory = false;
if (size > target.ReadMemory(addr, dest, size, error, force_live_memory,
/*load_addr_ptr=*/nullptr,
&did_read_live_memory)) {
LLDB_LOGV(log,
"[MemoryReader] memory read returned fewer bytes than asked for");
return false;
return {false, did_read_live_memory};
}
if (error.Fail()) {
LLDB_LOGV(log, "[MemoryReader] memory read returned error: {0}",
error.AsCString());
return false;
return {false, did_read_live_memory};
}

auto format_data = [](auto dest, auto size) {
Expand All @@ -391,7 +403,7 @@ bool LLDBMemoryReader::readBytes(swift::remote::RemoteAddress address,
LLDB_LOGV(log, "[MemoryReader] memory read returned data: {0}",
format_data(dest, size));

return true;
return {true, did_read_live_memory};
}

bool LLDBMemoryReader::readString(swift::remote::RemoteAddress address,
Expand Down Expand Up @@ -596,6 +608,32 @@ LLDBMemoryReader::getFileAddressAndModuleForTaggedAddress(
return {{file_address, module}};
}

bool LLDBMemoryReader::readRemoteAddressImpl(
swift::remote::RemoteAddress address, swift::remote::RemoteAddress &out,
std::size_t size) {
assert((size == 4 || size == 8) &&
"Only 32 or 64 bit architectures are supported!");
auto *dest = (uint8_t *)std::malloc(size);
auto defer = llvm::make_scope_exit([&] { free(dest); });

auto [success, did_read_live_memory] = readBytesImpl(address, dest, size);
if (!success)
return false;

uint8_t addressSpace = did_read_live_memory
? swift::remote::RemoteAddress::DefaultAddressSpace
: LLDBAddressSpace;
if (size == 4)
out = swift::remote::RemoteAddress(*reinterpret_cast<uint32_t *>(dest),
addressSpace);
else if (size == 8)
out = swift::remote::RemoteAddress(*reinterpret_cast<uint64_t *>(dest),
addressSpace);
else
return false;
return true;
}

std::optional<swift::reflection::RemoteAddress>
LLDBMemoryReader::resolveRemoteAddress(
swift::reflection::RemoteAddress address) const {
Expand Down Expand Up @@ -730,11 +768,6 @@ LLDBMemoryReader::resolveRemoteAddressFromSymbolObjectFile(
}

bool LLDBMemoryReader::readMetadataFromFileCacheEnabled() const {
auto &triple = m_process.GetTarget().GetArchitecture().GetTriple();

// 32 doesn't have a flag bit we can reliably use, so reading from filecache
// is disabled on it.
return m_process.GetTarget().GetSwiftReadMetadataFromFileCache() &&
triple.isArch64Bit();
return m_process.GetTarget().GetSwiftReadMetadataFromFileCache();
}
} // namespace lldb_private
11 changes: 11 additions & 0 deletions lldb/source/Plugins/LanguageRuntime/Swift/LLDBMemoryReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ class LLDBMemoryReader : public swift::remote::MemoryReader {
/// Returns whether the filecache optimization is enabled or not.
bool readMetadataFromFileCacheEnabled() const;

protected:
bool readRemoteAddressImpl(swift::remote::RemoteAddress address,
swift::remote::RemoteAddress &out,
std::size_t integerSize) override;

private:
friend MemoryReaderLocalBufferHolder;

Expand All @@ -119,6 +124,12 @@ class LLDBMemoryReader : public swift::remote::MemoryReader {
std::optional<Address>
remoteAddressToLLDBAddress(swift::remote::RemoteAddress address) const;

/// Implementation detail of readBytes. Returns a pair where the first element
/// indicates whether the memory was read successfully, the second element
/// indicates whether live memory was read.
std::pair<bool, bool> readBytesImpl(swift::remote::RemoteAddress address,
uint8_t *dest, uint64_t size);

/// Reads memory from the symbol rich binary from the address into dest.
/// \return true if it was able to successfully read memory.
std::optional<Address> resolveRemoteAddressFromSymbolObjectFile(
Expand Down
5 changes: 4 additions & 1 deletion lldb/source/Target/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1921,7 +1921,8 @@ size_t Target::ReadMemoryFromFileCache(const Address &addr, void *dst,

size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
Status &error, bool force_live_memory,
lldb::addr_t *load_addr_ptr) {
lldb::addr_t *load_addr_ptr,
bool *did_read_live_memory) {
error.Clear();

Address fixed_addr = addr;
Expand Down Expand Up @@ -2020,6 +2021,8 @@ size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
if (bytes_read) {
if (load_addr_ptr)
*load_addr_ptr = load_addr;
if (did_read_live_memory)
*did_read_live_memory = true;
return bytes_read;
}
}
Expand Down
3 changes: 2 additions & 1 deletion lldb/unittests/Expression/DWARFExpressionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ class MockTarget : public Target {

size_t ReadMemory(const Address &addr, void *dst, size_t dst_len,
Status &error, bool force_live_memory = false,
lldb::addr_t *load_addr_ptr = nullptr) /*override*/ {
lldb::addr_t *load_addr_ptr = nullptr,
bool *did_read_live_memory = nullptr) /*override*/ {
auto expected_memory = this->ReadMemory(addr.GetOffset(), dst_len);
if (!expected_memory) {
llvm::consumeError(expected_memory.takeError());
Expand Down