augusto2112 created this revision.
augusto2112 added reviewers: jasonmolenda, aprantl.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
On mach-O, if an object file is eligible to be in the shared cache, it
cannot have data read from disk, since offsets in the process may be
fixed by the shared cache builder. This patch takes this into account by
checking if the mach-O file contains a LC_SEGMENT_SPLIT_INFO load
command, which indicates shared cache eligibility.
This patch disables the file cache optimization by default for other
object files, since there's a risk we were doing wrong reads elsewhere
as well, but makes it easy for them to implement their own logic to check
if a file cache read is safe by overriding IsSafeToReadFromFileCache.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D106584
Files:
lldb/include/lldb/Core/Section.h
lldb/include/lldb/Symbol/ObjectFile.h
lldb/source/Core/Section.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/source/Target/Target.cpp
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1761,21 +1761,34 @@
// Read from file cache if read-only section.
if (!force_live_memory && resolved_addr.IsSectionOffset()) {
SectionSP section_sp(resolved_addr.GetSection());
- if (section_sp) {
- auto permissions = Flags(section_sp->GetPermissions());
- bool is_readonly = !permissions.Test(ePermissionsWritable) &&
- permissions.Test(ePermissionsReadable);
- if (is_readonly) {
- file_cache_bytes_read =
- ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
- if (file_cache_bytes_read == dst_len)
- return file_cache_bytes_read;
- else if (file_cache_bytes_read > 0) {
- file_cache_read_buffer =
- std::make_unique<uint8_t[]>(file_cache_bytes_read);
- std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+ ObjectFile *obj_file = section_sp->GetObjectFile();
+ if (obj_file && obj_file->IsSafeToReadFromFileCache(resolved_addr)) {
+ file_cache_bytes_read =
+ ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+
+#ifndef NDEBUG // Assert that file cache matches the process memory
+ if (ProcessIsValid() && file_cache_bytes_read == dst_len) {
+ if (load_addr == LLDB_INVALID_ADDRESS)
+ load_addr = resolved_addr.GetLoadAddress(this);
+ if (load_addr != LLDB_INVALID_ADDRESS) {
+ uint8_t *live_buf = (uint8_t *)malloc(dst_len);
+ bytes_read =
+ m_process_sp->ReadMemory(load_addr, live_buf, dst_len, error);
+ if (bytes_read == dst_len) {
+ assert(memcmp(live_buf, dst, dst_len) == 0);
+ }
+ free(live_buf);
}
}
+#endif
+
+ if (file_cache_bytes_read == dst_len)
+ return file_cache_bytes_read;
+ if (file_cache_bytes_read > 0) {
+ file_cache_read_buffer =
+ std::make_unique<uint8_t[]>(file_cache_bytes_read);
+ std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+ }
}
}
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -148,6 +148,11 @@
uint32_t GetPluginVersion() override;
+ bool ContainsLoadCommand(uint32_t lc);
+
+ bool IsSafeToReadFromFileCache(const lldb_private::Address &addr) override;
+
+
protected:
static lldb_private::UUID
GetUUID(const llvm::MachO::mach_header &header,
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -7006,3 +7006,38 @@
}
return added_images;
}
+
+bool ObjectFileMachO::ContainsLoadCommand(uint32_t lc) {
+ lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+ llvm::MachO::load_command load_cmd;
+ for (uint32_t i = 0; i < m_header.ncmds; ++i) {
+ const lldb::offset_t load_cmd_offset = offset;
+ if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
+ break;
+
+ if (load_cmd.cmd == lc)
+ return true;
+ offset = load_cmd_offset + load_cmd.cmdsize;
+ }
+ return false;
+}
+
+bool ObjectFileMachO::IsSafeToReadFromFileCache(
+ const lldb_private::Address &addr) {
+ if (!addr.IsValid())
+ return false;
+ SectionSP section_sp(addr.GetSection());
+ if (!section_sp)
+ return false;
+
+ // A writable section is not safe to read from the file cache, as the contents
+ // between it and the process may be different.
+ bool is_readonly_section = section_sp->IsReadOnly();
+
+ // If the object file contains the segment split info load command, it's
+ // eligible to be part of the shared cache, and as such, it's not safe to read
+ // from, since offsets may be changed by the shared cache builder.
+ bool contains_split_info = ContainsLoadCommand(LC_SEGMENT_SPLIT_INFO);
+
+ return is_readonly_section && !contains_split_info;
+}
Index: lldb/source/Core/Section.cpp
===================================================================
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -599,3 +599,9 @@
}
return count;
}
+
+bool Section::IsReadOnly() {
+ auto permissions = Flags(GetPermissions());
+ return !permissions.Test(ePermissionsWritable) &&
+ permissions.Test(ePermissionsReadable);
+}
\ No newline at end of file
Index: lldb/include/lldb/Symbol/ObjectFile.h
===================================================================
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -692,6 +692,13 @@
return false;
}
+ /// Returns if reading at an address from the on disk file instead of the
+ /// inferior is safe. For example, checking if the section the address lies in
+ /// is read-only may be enough to deem the file cache read safe. The default
+ /// implementation errs on the side of caution, but subclasses may provide a
+ /// more suitable implementation.
+ virtual bool IsSafeToReadFromFileCache(const Address &addr) { return false; }
+
protected:
// Member variables.
FileSpec m_file;
Index: lldb/include/lldb/Core/Section.h
===================================================================
--- lldb/include/lldb/Core/Section.h
+++ lldb/include/lldb/Core/Section.h
@@ -236,6 +236,8 @@
void SetIsRelocated(bool b) { m_relocated = b; }
+ bool IsReadOnly();
+
protected:
ObjectFile *m_obj_file; // The object file that data for this section should
// be read from
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
