Skip to content

[lldb] Implement LLDBMemoryReader::resolvePointer #3848

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
2 changes: 2 additions & 0 deletions lldb/include/lldb/Core/Section.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ class Section : public std::enable_shared_from_this<Section>,
ObjectFile *GetObjectFile() { return m_obj_file; }
const ObjectFile *GetObjectFile() const { return m_obj_file; }

bool CanContainSwiftReflectionData() const;

/// Read the section data from the object file that the section
/// resides in.
///
Expand Down
6 changes: 6 additions & 0 deletions lldb/include/lldb/Symbol/ObjectFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,12 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
virtual llvm::StringRef
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section);

#ifdef LLDB_ENABLE_SWIFT
virtual bool CanContainSwiftReflectionData(const Section &section) {
return false;
}
#endif // LLDB_ENABLE_SWIFT

/// Load binaries listed in a corefile
///
/// A corefile may have metadata listing binaries that can be loaded,
Expand Down
2 changes: 2 additions & 0 deletions lldb/include/lldb/Target/Target.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ class TargetProperties : public Properties {

bool GetSwiftReadMetadataFromFileCache() const;

bool GetSwiftUseReflectionSymbols() const;

bool GetEnableAutoImportClangModules() const;

bool GetUseAllCompilerFlags() const;
Expand Down
8 changes: 8 additions & 0 deletions lldb/source/Core/Section.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,14 @@ void Section::SetPermissions(uint32_t permissions) {
m_executable = (permissions & ePermissionsExecutable) != 0;
}

bool Section::CanContainSwiftReflectionData() const {
Copy link

Choose a reason for hiding this comment

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

Maybe GetCanContainSwiftReflectionData having this function be the same name feels confusing especially in this PR. e.g. we can see below GetSectionData calls m_obj_file->ReadSectionData.

Copy link
Author

Choose a reason for hiding this comment

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

Don't suppose you have alternative naming suggestions? I would say that Can is like Is or Has, which are not conventionally combined with Get. Using same name was by design, the idea is to connect them by name. If that's confusing, then I am open to other names, but I don't think GetCan… should be it.

#ifdef LLDB_ENABLE_SWIFT
return m_obj_file->CanContainSwiftReflectionData(*this);
#else
return false;
#endif // LLDB_ENABLE_SWIFT
}

lldb::offset_t Section::GetSectionData(void *dst, lldb::offset_t dst_len,
lldb::offset_t offset) {
if (m_obj_file)
Expand Down
46 changes: 46 additions & 0 deletions lldb/source/Plugins/LanguageRuntime/Swift/LLDBMemoryReader.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#include "LLDBMemoryReader.h"
#include "lldb/Core/Address.h"
#include "lldb/Core/Section.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Logging.h"
#include "swift/Demangling/Demangle.h"

#include "llvm/Support/MathExtras.h"

Expand Down Expand Up @@ -117,6 +119,50 @@ LLDBMemoryReader::getSymbolAddress(const std::string &name) {
return swift::remote::RemoteAddress(load_addr);
}

swift::remote::RemoteAbsolutePointer
LLDBMemoryReader::resolvePointer(swift::remote::RemoteAddress address,
uint64_t readValue) {
// If an address has a symbol, that symbol provides additional useful data to
// MetadataReader. Without the symbol, MetadataReader can derive the symbol
// by loading other parts of reflection metadata, but that work has a cost.
// For lldb, that data loading can be a significant performance hit. Providing
// a symbol greatly reduces memory read traffic to the process.
auto pointer = swift::remote::RemoteAbsolutePointer("", readValue);

auto &target = m_process.GetTarget();
if (!target.GetSwiftUseReflectionSymbols())
return pointer;

llvm::Optional<Address> maybeAddr =
resolveRemoteAddress(address.getAddressData());
// This is not an assert, but should never happen.
if (!maybeAddr)
return pointer;

Address addr;
if (maybeAddr->IsSectionOffset()) {
// `address` was tagged, and then successfully mapped (resolved).
addr = *maybeAddr;
} else {
// `address` is a real load address.
if (!target.ResolveLoadAddress(address.getAddressData(), addr))
return pointer;
}

if (!addr.GetSection()->CanContainSwiftReflectionData())
return pointer;

if (auto *symbol = addr.CalculateSymbolContextSymbol()) {
auto mangledName = symbol->GetMangled().GetMangledName().GetStringRef();
// MemoryReader requires this to be a Swift symbol. LLDB can also be
// aware of local symbols, so avoid returning those.
if (swift::Demangle::isSwiftSymbol(mangledName))
return {mangledName, 0};
}

return pointer;
}

bool LLDBMemoryReader::readBytes(swift::remote::RemoteAddress address,
uint8_t *dest, uint64_t size) {
if (m_local_buffer) {
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Plugins/LanguageRuntime/Swift/LLDBMemoryReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ class LLDBMemoryReader : public swift::remote::MemoryReader {
swift::remote::RemoteAddress
getSymbolAddress(const std::string &name) override;

swift::remote::RemoteAbsolutePointer
resolvePointer(swift::remote::RemoteAddress address,
uint64_t readValue) override;

bool readBytes(swift::remote::RemoteAddress address, uint8_t *dest,
uint64_t size) override;

Expand Down
8 changes: 8 additions & 0 deletions lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3429,3 +3429,11 @@ llvm::StringRef ObjectFileELF::GetReflectionSectionIdentifier(
llvm_unreachable("Swift support disabled");
#endif //LLDB_ENABLE_SWIFT
}

#ifdef LLDB_ENABLE_SWIFT
bool ObjectFileELF::CanContainSwiftReflectionData(const Section &section) {
swift::SwiftObjectFileFormatELF file_format;
return file_format.sectionContainsReflectionData(
section.GetName().GetStringRef());
}
#endif // LLDB_ENABLE_SWIFT
5 changes: 5 additions & 0 deletions lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,11 @@ class ObjectFileELF : public lldb_private::ObjectFile {

llvm::StringRef
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section) override;

#ifdef LLDB_ENABLE_SWIFT
bool
CanContainSwiftReflectionData(const lldb_private::Section &section) override;
#endif // LLDB_ENABLE_SWIFT
};

#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_ELF_OBJECTFILEELF_H
17 changes: 17 additions & 0 deletions lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7019,6 +7019,23 @@ llvm::StringRef ObjectFileMachO::GetReflectionSectionIdentifier(
#endif //LLDB_ENABLE_SWIFT
}

#ifdef LLDB_ENABLE_SWIFT
bool ObjectFileMachO::CanContainSwiftReflectionData(const Section &section) {
swift::SwiftObjectFileFormatMachO file_format;
if (file_format.sectionContainsReflectionData(
section.GetName().GetStringRef()))
return true;
// Some section names are not unique, such as `__const`, which could be in
// `__TEXT` or `__DATA`. For these, also check the segment,section name.
if (auto segment = section.GetParent()) {
std::string segmentSectionName =
llvm::formatv("{0},{1}", segment->GetName(), section.GetName()).str();
return file_format.sectionContainsReflectionData(segmentSectionName);
}
return false;
}
#endif // LLDB_ENABLE_SWIFT

ObjectFileMachO::MachOCorefileAllImageInfos
ObjectFileMachO::GetCorefileAllImageInfos() {
MachOCorefileAllImageInfos image_infos;
Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,11 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
llvm::StringRef
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section) override;

#ifdef LLDB_ENABLE_SWIFT
bool
CanContainSwiftReflectionData(const lldb_private::Section &section) override;
#endif // LLDB_ENABLE_SWIFT

/// A corefile may include metadata about all of the binaries that were
/// present in the process when the corefile was taken. This is only
/// implemented for Mach-O files for now; we'll generalize it when we
Expand Down
7 changes: 7 additions & 0 deletions lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1217,3 +1217,10 @@ llvm::StringRef ObjectFilePECOFF::GetReflectionSectionIdentifier(
#endif //LLDB_ENABLE_SWIFT
}

#ifdef LLDB_ENABLE_SWIFT
bool ObjectFilePECOFF::CanContainSwiftReflectionData(const Section &section) {
swift::SwiftObjectFileFormatCOFF file_format;
return file_format.sectionContainsReflectionData(
section.GetName().GetStringRef());
}
#endif // LLDB_ENABLE_SWIFT
5 changes: 5 additions & 0 deletions lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,11 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
llvm::StringRef
GetReflectionSectionIdentifier(swift::ReflectionSectionKind section) override;

#ifdef LLDB_ENABLE_SWIFT
bool
CanContainSwiftReflectionData(const lldb_private::Section &section) override;
#endif // LLDB_ENABLE_SWIFT

typedef std::vector<section_header_t> SectionHeaderColl;
typedef SectionHeaderColl::iterator SectionHeaderCollIter;
typedef SectionHeaderColl::const_iterator SectionHeaderCollConstIter;
Expand Down
12 changes: 12 additions & 0 deletions lldb/source/Target/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4168,6 +4168,18 @@ bool TargetProperties::GetSwiftReadMetadataFromFileCache() const {
return true;
}

bool TargetProperties::GetSwiftUseReflectionSymbols() const {
const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
nullptr, false, ePropertyExperimental);
OptionValueProperties *exp_values =
exp_property->GetValue()->GetAsProperties();
if (exp_values)
return exp_values->GetPropertyAtIndexAsBoolean(
nullptr, ePropertySwiftUseReflectionSymbols, true);
else
return true;
}

ArchSpec TargetProperties::GetDefaultArchitecture() const {
OptionValueArch *value = m_collection_sp->GetPropertyAtIndexAsOptionValueArch(
nullptr, ePropertyDefaultArch);
Expand Down
3 changes: 3 additions & 0 deletions lldb/source/Target/TargetProperties.td
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ let Definition = "target_experimental" in {
def SwiftReadMetadataFromFileCache: Property<"swift-read-metadata-from-file-cache", "Boolean">,
DefaultTrue,
Desc<"Read Swift reflection metadata from the file cache instead of the process when possible">;
def SwiftUseReflectionSymbols : Property<"swift-use-reflection-symbols", "Boolean">,
Global, DefaultTrue,
Desc<"if true, optimize the loading of Swift reflection metadata by making use of available symbols.">;
}

let Definition = "target" in {
Expand Down