Make sure DataBufferLLVM contents are writable
authorPavel Labath <labath@google.com>
Thu, 21 Dec 2017 10:54:30 +0000 (10:54 +0000)
committerPavel Labath <labath@google.com>
Thu, 21 Dec 2017 10:54:30 +0000 (10:54 +0000)
Summary:
We sometimes need to write to the object file we've mapped into memory,
generally to apply relocations to debug info sections. We've had that
ability before, but with the introduction of DataBufferLLVM, we have
lost it, as the underlying llvm class (MemoryBuffer) only supports
read-only mappings.

This switches DataBufferLLVM to use the new llvm::WritableMemoryBuffer
class as a back-end, as this one guarantees to return a writable buffer.

This removes the need for the "Private" flag to the DataBufferLLVM
creation functions, as it was really used to mean "writable". The LLVM
function also does not have the NullTerminate flag, so I've modified our
clients to not require this feature and removed that flag as well.

Reviewers: zturner, clayborg, jingham

Subscribers: emaste, aprantl, arichardson, krytarowski, lldb-commits

Differential Revision: https://reviews.llvm.org/D40079

llvm-svn: 321255

12 files changed:
lldb/include/lldb/Interpreter/OptionValueFileSpec.h
lldb/include/lldb/Symbol/ObjectFile.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Utility/DataBufferLLVM.h
lldb/source/Interpreter/OptionValueFileSpec.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Symbol/ObjectFile.cpp
lldb/source/Target/Target.cpp
lldb/source/Utility/DataBufferLLVM.cpp

index b53c03471e2b35edd14fd3eceef7c1ae4e3c62bb..293ecd0d805442955ddd1a25ab8ff0a52e39540e 100644 (file)
@@ -77,7 +77,7 @@ public:
 
   void SetDefaultValue(const FileSpec &value) { m_default_value = value; }
 
-  const lldb::DataBufferSP &GetFileContents(bool null_terminate);
+  const lldb::DataBufferSP &GetFileContents();
 
   void SetCompletionMask(uint32_t mask) { m_completion_mask = mask; }
 
index 3f9250af7e08a2eeebbc34e89631ca8b4068c387..dff802522da83fca60cca2840c1cee14f896d4b5 100644 (file)
@@ -879,6 +879,9 @@ protected:
 
   ConstString GetNextSyntheticSymbolName();
 
+  static lldb::DataBufferSP MapFileData(const FileSpec &file, uint64_t Size,
+                                        uint64_t Offset);
+
 private:
   DISALLOW_COPY_AND_ASSIGN(ObjectFile);
 };
index 242ec14165ebfa647d1c29f8b86ea4ea5c1fc562..a5d20006981034110d200fe329a5711d63f88d1f 100644 (file)
@@ -161,7 +161,7 @@ public:
 
   lldb::LanguageType GetLanguage() const;
 
-  const char *GetExpressionPrefixContentsAsCString();
+  llvm::StringRef GetExpressionPrefixContents();
 
   bool GetUseHexImmediates() const;
 
index 737e2d019044ec07b3d1032e98d379e3cda087e1..0eb229cffbe24f99809c5297033ed3b0c4eb5a60 100644 (file)
@@ -17,7 +17,7 @@
 #include <stdint.h> // for uint8_t, uint64_t
 
 namespace llvm {
-class MemoryBuffer;
+class WritableMemoryBuffer;
 class Twine;
 }
 
@@ -28,10 +28,10 @@ public:
   ~DataBufferLLVM();
 
   static std::shared_ptr<DataBufferLLVM>
-  CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size, uint64_t Offset, bool Private = false);
+  CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size, uint64_t Offset);
 
   static std::shared_ptr<DataBufferLLVM>
-  CreateFromPath(const llvm::Twine &Path, bool NullTerminate = false, bool Private = false);
+  CreateFromPath(const llvm::Twine &Path);
 
   uint8_t *GetBytes() override;
   const uint8_t *GetBytes() const override;
@@ -42,10 +42,9 @@ public:
 private:
   /// \brief Construct a DataBufferLLVM from \p Buffer.  \p Buffer must be a
   /// valid pointer.
-  explicit DataBufferLLVM(std::unique_ptr<llvm::MemoryBuffer> Buffer);
-  const uint8_t *GetBuffer() const;
+  explicit DataBufferLLVM(std::unique_ptr<llvm::WritableMemoryBuffer> Buffer);
 
-  std::unique_ptr<llvm::MemoryBuffer> Buffer;
+  std::unique_ptr<llvm::WritableMemoryBuffer> Buffer;
 };
 }
 
index b235d4ac6863dc6ba6bdfe834854e7104d5e6185..7d17ddec4f9c44d9535892180e874dde17e78e52 100644 (file)
@@ -113,14 +113,12 @@ size_t OptionValueFileSpec::AutoComplete(
   return matches.GetSize();
 }
 
-const lldb::DataBufferSP &
-OptionValueFileSpec::GetFileContents(bool null_terminate) {
+const lldb::DataBufferSP &OptionValueFileSpec::GetFileContents() {
   if (m_current_value) {
     const auto file_mod_time = FileSystem::GetModificationTime(m_current_value);
     if (m_data_sp && m_data_mod_time == file_mod_time)
       return m_data_sp;
-    m_data_sp = DataBufferLLVM::CreateFromPath(m_current_value.GetPath(),
-                                               null_terminate);
+    m_data_sp = DataBufferLLVM::CreateFromPath(m_current_value.GetPath());
     m_data_mod_time = file_mod_time;
   }
   return m_data_sp;
index 17d892450e4dee7081c7cbff082da8b72553c82e..36027dde04325fe3274373fcef9cff7ad4a1f50d 100644 (file)
@@ -24,7 +24,6 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/DataBufferLLVM.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
@@ -406,8 +405,7 @@ ObjectFile *ObjectFileELF::CreateInstance(const lldb::ModuleSP &module_sp,
                                           lldb::offset_t file_offset,
                                           lldb::offset_t length) {
   if (!data_sp) {
-    data_sp =
-        DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset, true);
+    data_sp = MapFileData(*file, length, file_offset);
     if (!data_sp)
       return nullptr;
     data_offset = 0;
@@ -424,8 +422,7 @@ ObjectFile *ObjectFileELF::CreateInstance(const lldb::ModuleSP &module_sp,
 
   // Update the data to contain the entire file if it doesn't already
   if (data_sp->GetByteSize() < length) {
-    data_sp =
-        DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset, true);
+    data_sp = MapFileData(*file, length, file_offset);
     if (!data_sp)
       return nullptr;
     data_offset = 0;
@@ -684,8 +681,7 @@ size_t ObjectFileELF::GetModuleSpecifications(
           size_t section_header_end = header.e_shoff + header.e_shentsize;
           if (header.HasHeaderExtension() &&
             section_header_end > data_sp->GetByteSize()) {
-            data_sp = DataBufferLLVM::CreateSliceFromPath(
-                file.GetPath(), section_header_end, file_offset);
+            data_sp = MapFileData(file, section_header_end, file_offset);
             if (data_sp) {
               data.SetData(data_sp);
               lldb::offset_t header_offset = data_offset;
@@ -698,8 +694,7 @@ size_t ObjectFileELF::GetModuleSpecifications(
           section_header_end =
               header.e_shoff + header.e_shnum * header.e_shentsize;
           if (section_header_end > data_sp->GetByteSize()) {
-            data_sp = DataBufferLLVM::CreateSliceFromPath(
-                file.GetPath(), section_header_end, file_offset);
+            data_sp = MapFileData(file, section_header_end, file_offset);
             if (data_sp)
               data.SetData(data_sp);
           }
@@ -741,8 +736,7 @@ size_t ObjectFileELF::GetModuleSpecifications(
                 size_t program_headers_end =
                     header.e_phoff + header.e_phnum * header.e_phentsize;
                 if (program_headers_end > data_sp->GetByteSize()) {
-                  data_sp = DataBufferLLVM::CreateSliceFromPath(
-                      file.GetPath(), program_headers_end, file_offset);
+                  data_sp = MapFileData(file, program_headers_end, file_offset);
                   if (data_sp)
                     data.SetData(data_sp);
                 }
@@ -757,8 +751,7 @@ size_t ObjectFileELF::GetModuleSpecifications(
                 }
 
                 if (segment_data_end > data_sp->GetByteSize()) {
-                  data_sp = DataBufferLLVM::CreateSliceFromPath(
-                      file.GetPath(), segment_data_end, file_offset);
+                  data_sp = MapFileData(file, segment_data_end, file_offset);
                   if (data_sp)
                     data.SetData(data_sp);
                 }
@@ -767,8 +760,7 @@ size_t ObjectFileELF::GetModuleSpecifications(
                     CalculateELFNotesSegmentsCRC32(program_headers, data);
               } else {
                 // Need to map entire file into memory to calculate the crc.
-                data_sp = DataBufferLLVM::CreateSliceFromPath(file.GetPath(), -1,
-                                                         file_offset);
+                data_sp = MapFileData(file, -1, file_offset);
                 if (data_sp) {
                   data.SetData(data_sp);
                   gnu_debuglink_crc = calc_gnu_debuglink_crc32(
index df334f88ee3b4c4219b1e916c3dfba4bde1f7662..e6941c9f6ed6b692ac9ee7827053e47e30c60313 100644 (file)
@@ -38,7 +38,7 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/DataBufferLLVM.h"
+#include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Status.h"
@@ -862,8 +862,7 @@ ObjectFile *ObjectFileMachO::CreateInstance(const lldb::ModuleSP &module_sp,
                                             lldb::offset_t file_offset,
                                             lldb::offset_t length) {
   if (!data_sp) {
-    data_sp =
-        DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset);
+    data_sp = MapFileData(*file, length, file_offset);
     if (!data_sp)
       return nullptr;
     data_offset = 0;
@@ -874,8 +873,7 @@ ObjectFile *ObjectFileMachO::CreateInstance(const lldb::ModuleSP &module_sp,
 
   // Update the data to contain the entire file if it doesn't already
   if (data_sp->GetByteSize() < length) {
-    data_sp =
-        DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset);
+    data_sp = MapFileData(*file, length, file_offset);
     if (!data_sp)
       return nullptr;
     data_offset = 0;
@@ -914,8 +912,7 @@ size_t ObjectFileMachO::GetModuleSpecifications(
       size_t header_and_load_cmds =
           header.sizeofcmds + MachHeaderSizeFromMagic(header.magic);
       if (header_and_load_cmds >= data_sp->GetByteSize()) {
-        data_sp = DataBufferLLVM::CreateSliceFromPath(
-            file.GetPath(), header_and_load_cmds, file_offset);
+        data_sp = MapFileData(file, header_and_load_cmds, file_offset);
         data.SetData(data_sp);
         data_offset = MachHeaderSizeFromMagic(header.magic);
       }
@@ -1127,8 +1124,7 @@ bool ObjectFileMachO::ParseHeader() {
                   ReadMemory(process_sp, m_memory_addr, header_and_lc_size);
             } else {
               // Read in all only the load command data from the file on disk
-              data_sp = DataBufferLLVM::CreateSliceFromPath(
-                  m_file.GetPath(), header_and_lc_size, m_file_offset);
+              data_sp = MapFileData(m_file, header_and_lc_size, m_file_offset);
               if (data_sp->GetByteSize() != header_and_lc_size)
                 return false;
             }
@@ -2100,9 +2096,8 @@ UUID ObjectFileMachO::GetSharedCacheUUID(FileSpec dyld_shared_cache,
                                          const ByteOrder byte_order,
                                          const uint32_t addr_byte_size) {
   UUID dsc_uuid;
-  DataBufferSP DscData = DataBufferLLVM::CreateSliceFromPath(
-      dyld_shared_cache.GetPath(),
-      sizeof(struct lldb_copy_dyld_cache_header_v1), 0);
+  DataBufferSP DscData = MapFileData(
+      dyld_shared_cache, sizeof(struct lldb_copy_dyld_cache_header_v1), 0);
   if (!DscData)
     return dsc_uuid;
   DataExtractor dsc_header_data(DscData, byte_order, addr_byte_size);
@@ -2708,9 +2703,8 @@ size_t ObjectFileMachO::ParseSymtab() {
 
       // Process the dyld shared cache header to find the unmapped symbols
 
-      DataBufferSP dsc_data_sp = DataBufferLLVM::CreateSliceFromPath(
-          dsc_filespec.GetPath(), sizeof(struct lldb_copy_dyld_cache_header_v1),
-          0);
+      DataBufferSP dsc_data_sp = MapFileData(
+          dsc_filespec, sizeof(struct lldb_copy_dyld_cache_header_v1), 0);
       if (!dsc_uuid.IsValid()) {
         dsc_uuid = GetSharedCacheUUID(dsc_filespec, byte_order, addr_byte_size);
       }
@@ -2742,11 +2736,9 @@ size_t ObjectFileMachO::ParseSymtab() {
         if (uuid_match &&
             mappingOffset >= sizeof(struct lldb_copy_dyld_cache_header_v1)) {
 
-          DataBufferSP dsc_mapping_info_data_sp =
-              DataBufferLLVM::CreateSliceFromPath(
-                  dsc_filespec.GetPath(),
-                  sizeof(struct lldb_copy_dyld_cache_mapping_info),
-                  mappingOffset);
+          DataBufferSP dsc_mapping_info_data_sp = MapFileData(
+              dsc_filespec, sizeof(struct lldb_copy_dyld_cache_mapping_info),
+              mappingOffset);
 
           DataExtractor dsc_mapping_info_data(dsc_mapping_info_data_sp,
                                               byte_order, addr_byte_size);
@@ -2770,9 +2762,7 @@ size_t ObjectFileMachO::ParseSymtab() {
           if (localSymbolsOffset && localSymbolsSize) {
             // Map the local symbols
             DataBufferSP dsc_local_symbols_data_sp =
-                DataBufferLLVM::CreateSliceFromPath(dsc_filespec.GetPath(),
-                                               localSymbolsSize,
-                                               localSymbolsOffset);
+                MapFileData(dsc_filespec, localSymbolsSize, localSymbolsOffset);
 
             if (dsc_local_symbols_data_sp) {
               DataExtractor dsc_local_symbols_data(dsc_local_symbols_data_sp,
index 72b1b15f08f86ed5a23a339c583c2cb680e2236e..77bfa7fe0a640e036d4e4fc812fb211183edff12 100644 (file)
@@ -22,7 +22,6 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/DataBufferLLVM.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
@@ -66,8 +65,7 @@ ObjectFile *ObjectFilePECOFF::CreateInstance(const lldb::ModuleSP &module_sp,
                                              lldb::offset_t file_offset,
                                              lldb::offset_t length) {
   if (!data_sp) {
-    data_sp =
-        DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset);
+    data_sp = MapFileData(file, length, file_offset);
     if (!data_sp)
       return nullptr;
     data_offset = 0;
@@ -78,8 +76,7 @@ ObjectFile *ObjectFilePECOFF::CreateInstance(const lldb::ModuleSP &module_sp,
 
   // Update the data to contain the entire file if it doesn't already
   if (data_sp->GetByteSize() < length) {
-    data_sp =
-        DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset);
+    data_sp = MapFileData(file, length, file_offset);
     if (!data_sp)
       return nullptr;
   }
@@ -436,8 +433,7 @@ DataExtractor ObjectFilePECOFF::ReadImageData(uint32_t offset, size_t size) {
   if (m_file) {
     // A bit of a hack, but we intend to write to this buffer, so we can't 
     // mmap it.
-    auto buffer_sp =
-        DataBufferLLVM::CreateSliceFromPath(m_file.GetPath(), size, offset, true);
+    auto buffer_sp = MapFileData(m_file, size, offset);
     return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
   }
   ProcessSP process_sp(m_process_wp.lock());
index b39aa103f1dbd143e5c553a1093533929dadaff6..b04d72f755f68f1dd88e14e2f48d845449f1ea24 100644 (file)
@@ -1173,7 +1173,7 @@ const char *PlatformDarwin::GetDeveloperDirectory() {
       xcode_dir_path.append("/usr/share/xcode-select/xcode_dir_path");
       temp_file_spec.SetFile(xcode_dir_path, false);
       auto dir_buffer =
-          DataBufferLLVM::CreateFromPath(temp_file_spec.GetPath(), true);
+          DataBufferLLVM::CreateFromPath(temp_file_spec.GetPath());
       if (dir_buffer && dir_buffer->GetByteSize() > 0) {
         llvm::StringRef path_ref(dir_buffer->GetChars());
         // Trim tailing newlines and make sure there is enough room for a null
index 7d73cb19d508322c15a55830f18ae7fd8e54ea4a..2129a4463cdd3e14cc7b4776bfc2160937c45e2e 100644 (file)
@@ -688,3 +688,8 @@ Status ObjectFile::LoadInMemory(Target &target, bool set_pc) {
 void ObjectFile::RelocateSection(lldb_private::Section *section)
 {
 }
+
+DataBufferSP ObjectFile::MapFileData(const FileSpec &file, uint64_t Size,
+                                     uint64_t Offset) {
+  return DataBufferLLVM::CreateSliceFromPath(file.GetPath(), Size, Offset);
+}
index 903f50887fec02c3906c5810c10583f58dc70119..fdc10cf4827557928fd8e5cbf5660eb7703e9efd 100644 (file)
@@ -2313,7 +2313,7 @@ ExpressionResults Target::EvaluateExpression(
     result_valobj_sp = persistent_var_sp->GetValueObject();
     execution_results = eExpressionCompleted;
   } else {
-    const char *prefix = GetExpressionPrefixContentsAsCString();
+    llvm::StringRef prefix = GetExpressionPrefixContents();
     Status error;
     execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix,
                                                  result_valobj_sp, error,
@@ -4046,18 +4046,19 @@ LanguageType TargetProperties::GetLanguage() const {
   return LanguageType();
 }
 
-const char *TargetProperties::GetExpressionPrefixContentsAsCString() {
+llvm::StringRef TargetProperties::GetExpressionPrefixContents() {
   const uint32_t idx = ePropertyExprPrefix;
   OptionValueFileSpec *file =
       m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
                                                                idx);
   if (file) {
-    const bool null_terminate = true;
-    DataBufferSP data_sp(file->GetFileContents(null_terminate));
+    DataBufferSP data_sp(file->GetFileContents());
     if (data_sp)
-      return (const char *)data_sp->GetBytes();
+      return llvm::StringRef(
+          reinterpret_cast<const char *>(data_sp->GetBytes()),
+          data_sp->GetByteSize());
   }
-  return nullptr;
+  return "";
 }
 
 bool TargetProperties::GetBreakpointsConsultPlatformAvoidList() {
index bebcafbf9150d589214580533264d4f9dbb169eb..713c3c2814eafaad82ccf21536f5993158e29587 100644 (file)
@@ -18,7 +18,8 @@
 
 using namespace lldb_private;
 
-DataBufferLLVM::DataBufferLLVM(std::unique_ptr<llvm::MemoryBuffer> MemBuffer)
+DataBufferLLVM::DataBufferLLVM(
+    std::unique_ptr<llvm::WritableMemoryBuffer> MemBuffer)
     : Buffer(std::move(MemBuffer)) {
   assert(Buffer != nullptr &&
          "Cannot construct a DataBufferLLVM with a null buffer");
@@ -28,13 +29,13 @@ DataBufferLLVM::~DataBufferLLVM() {}
 
 std::shared_ptr<DataBufferLLVM>
 DataBufferLLVM::CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size,
-                               uint64_t Offset, bool Private) {
+                                    uint64_t Offset) {
   // If the file resides non-locally, pass the volatile flag so that we don't
   // mmap it.
-  if (!Private)
-    Private = !llvm::sys::fs::is_local(Path);
+  bool IsVolatile = !llvm::sys::fs::is_local(Path);
 
-  auto Buffer = llvm::MemoryBuffer::getFileSlice(Path, Size, Offset, Private);
+  auto Buffer =
+      llvm::WritableMemoryBuffer::getFileSlice(Path, Size, Offset, IsVolatile);
   if (!Buffer)
     return nullptr;
   return std::shared_ptr<DataBufferLLVM>(
@@ -42,13 +43,12 @@ DataBufferLLVM::CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size,
 }
 
 std::shared_ptr<DataBufferLLVM>
-DataBufferLLVM::CreateFromPath(const llvm::Twine &Path, bool NullTerminate, bool Private) {
+DataBufferLLVM::CreateFromPath(const llvm::Twine &Path) {
   // If the file resides non-locally, pass the volatile flag so that we don't
   // mmap it.
-  if (!Private)
-    Private = !llvm::sys::fs::is_local(Path);
+  bool IsVolatile = !llvm::sys::fs::is_local(Path);
 
-  auto Buffer = llvm::MemoryBuffer::getFile(Path, -1, NullTerminate, Private);
+  auto Buffer = llvm::WritableMemoryBuffer::getFile(Path, -1, IsVolatile);
   if (!Buffer)
     return nullptr;
   return std::shared_ptr<DataBufferLLVM>(
@@ -56,15 +56,13 @@ DataBufferLLVM::CreateFromPath(const llvm::Twine &Path, bool NullTerminate, bool
 }
 
 uint8_t *DataBufferLLVM::GetBytes() {
-  return const_cast<uint8_t *>(GetBuffer());
+  return reinterpret_cast<uint8_t *>(Buffer->getBufferStart());
 }
 
-const uint8_t *DataBufferLLVM::GetBytes() const { return GetBuffer(); }
+const uint8_t *DataBufferLLVM::GetBytes() const {
+  return reinterpret_cast<const uint8_t *>(Buffer->getBufferStart());
+}
 
 lldb::offset_t DataBufferLLVM::GetByteSize() const {
   return Buffer->getBufferSize();
 }
-
-const uint8_t *DataBufferLLVM::GetBuffer() const {
-  return reinterpret_cast<const uint8_t *>(Buffer->getBufferStart());
-}