JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg, jasonmolenda.
Herald added a project: All.
JDevlieghere requested review of this revision.
While discussing D122856 <https://reviews.llvm.org/D122856> with Pavel on IRC,
he pointed out that the current design allows that the file contents could be
mapped by one object file plugin and then used by another. Presumably the idea
here was to avoid mapping the same file twice. That becomes problematic for
ObjectFileELF which after D122856 <https://reviews.llvm.org/D122856> will map
the file as read-write.
https://reviews.llvm.org/D122944
Files:
lldb/source/Symbol/ObjectFile.cpp
Index: lldb/source/Symbol/ObjectFile.cpp
===================================================================
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -31,6 +31,30 @@
char ObjectFile::ID;
+// RAII Utility to reset the data buffer and data offset.
+class DataReset {
+public:
+ DataReset(DataBufferSP &data_sp, lldb::offset_t &data_offset)
+ : data_sp(data_sp), data_sp_ref(data_sp), data_offset(data_offset),
+ data_offset_ref(data_offset) {}
+
+ ~DataReset() {
+ if (!reset)
+ return;
+ data_sp_ref = data_sp;
+ data_offset_ref = data_offset;
+ }
+
+ void Release() { reset = false; }
+
+private:
+ bool reset = true;
+ DataBufferSP data_sp;
+ DataBufferSP data_sp_ref;
+ lldb::offset_t data_offset;
+ lldb::offset_t &data_offset_ref;
+};
+
static ObjectFileSP
CreateObjectFromContainer(const lldb::ModuleSP &module_sp, const FileSpec *file,
lldb::offset_t file_offset, lldb::offset_t file_size,
@@ -40,10 +64,33 @@
(callback = PluginManager::GetObjectContainerCreateCallbackAtIndex(
idx)) != nullptr;
++idx) {
+ DataReset data_reset(data_sp, data_offset);
std::unique_ptr<ObjectContainer> object_container_up(callback(
module_sp, data_sp, data_offset, file, file_offset, file_size));
- if (object_container_up)
+ if (object_container_up) {
+ data_reset.Release();
return object_container_up->GetObjectFile(file);
+ }
+ }
+ return {};
+}
+
+static ObjectFileSP
+CreateObject(const lldb::ModuleSP &module_sp, const FileSpec *file,
+ lldb::offset_t file_offset, lldb::offset_t file_size,
+ DataBufferSP &data_sp, lldb::offset_t &data_offset) {
+ ObjectFileCreateInstance callback;
+ for (uint32_t idx = 0;
+ (callback = PluginManager::GetObjectFileCreateCallbackAtIndex(idx)) !=
+ nullptr;
+ ++idx) {
+ DataReset data_reset(data_sp, data_offset);
+ ObjectFileSP object_file_sp(callback(module_sp, data_sp, data_offset, file,
+ file_offset, file_size));
+ if (object_file_sp.get()) {
+ data_reset.Release();
+ return object_file_sp;
+ }
}
return {};
}
@@ -71,9 +118,8 @@
// a static archive (.a file). Try and see if we have a cached archive
// first without reading any data first
if (file_exists && module_sp->GetObjectName()) {
- ObjectFileSP object_file_sp = CreateObjectFromContainer(
- module_sp, file, file_offset, file_size, data_sp, data_offset);
- if (object_file_sp)
+ if (ObjectFileSP object_file_sp = CreateObjectFromContainer(
+ module_sp, file, file_offset, file_size, data_sp, data_offset))
return object_file_sp;
}
// Ok, we didn't find any containers that have a named object, now lets
@@ -108,9 +154,8 @@
// ANY data in case there is data cached in the container plug-ins
// (like BSD archives caching the contained objects within an
// file).
- ObjectFileSP object_file_sp = CreateObjectFromContainer(
- module_sp, file, file_offset, file_size, data_sp, data_offset);
- if (object_file_sp)
+ if (ObjectFileSP object_file_sp = CreateObjectFromContainer(
+ module_sp, file, file_offset, file_size, data_sp, data_offset))
return object_file_sp;
// We failed to find any cached object files in the container plug-
// ins, so lets read the first 512 bytes and try again below...
@@ -121,25 +166,17 @@
}
if (data_sp && data_sp->GetByteSize() > 0) {
- // Check if this is a normal object file by iterating through all
- // object file plugin instances.
- ObjectFileCreateInstance callback;
- for (uint32_t idx = 0;
- (callback = PluginManager::GetObjectFileCreateCallbackAtIndex(idx)) !=
- nullptr;
- ++idx) {
- ObjectFileSP object_file_sp(callback(module_sp, data_sp, data_offset,
- file, file_offset, file_size));
- if (object_file_sp.get())
- return object_file_sp;
- }
+ // Check if this is a normal object file by iterating through all object
+ // file plugin instances.
+ if (ObjectFileSP object_file_sp = CreateObject(
+ module_sp, file, file_offset, file_size, data_sp, data_offset))
+ return object_file_sp;
// Check if this is a object container by iterating through all object
// container plugin instances and then trying to get an object file
// from the container.
- ObjectFileSP object_file_sp = CreateObjectFromContainer(
- module_sp, file, file_offset, file_size, data_sp, data_offset);
- if (object_file_sp)
+ if (ObjectFileSP object_file_sp = CreateObjectFromContainer(
+ module_sp, file, file_offset, file_size, data_sp, data_offset))
return object_file_sp;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
