Make sure DataBufferLLVM contents are writable

[labath]

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. The way DataBufferLLVM implemented writable buffers
was through the Private=true flag, which causes the file contents to be
read into a malloc()ed buffer.

This wasn't a problem then, since we weren't aware of any cases where we
need to relocate the debug info, and we just always set Private=false
for the cases where we mmapped the full object file. This has changed in
D38142, where needed to relocate the FreeBSD kernel, and we've flipped
the private flag to true, without even realising the full impact of
that. (The impact is significant slowdown in debug info loading and a
huge number of dirty pages).

This (not particularly elegant) commit tries to restore the performance
and bring down memory usage by bringing back the private copy-on-write
file mappings. It does this by falling back to the lower-level llvm
primitive (mapped_file_region), which supports this type of mappings.
Unfortunately, this primitive does not support non-mmapped regions, so
for volatile regions I have to still use the MemoryBuffer class.

I have also removed the "Private" argument to DataBufferLLVM factory
functions -- what it really meant was "read-only", but we generally
assume all our databuffers are writable. If we ever want to have
read-only buffers, we can bring it back with an appropriate name.

[clayborg]

I had suggested in D38142 that we have ObjectFileELF check the file type of the file and only map with write abilities if the ELF file is an object file since that is the only time we need relocations. If we can pass a flag through to indicate how to map, would that provide any additional performance improvements?

[labath]

In D40079#926328, @clayborg wrote:

I had suggested in D38142 that we have ObjectFileELF check the file type of the file and only map with write abilities if the ELF file is an object file since that is the only time we need relocations. If we can pass a flag through to indicate how to map, would that provide any additional performance improvements?

Sorry, I hadn't seen that comment.

But to answer to question, it would not provide *additional* performance improvements -- the memory will still be shared until you actually try to write to it (with the read-only shared mapping you'll get a SEGV, with a private read-write mapping, the page will be COWed).

However, I suppose something like that could be done *instead* of this patch. That way we would be using mmap for most object files, and only resorting to malloc+read for those that need relocating. It's not an ideal solution, but then again, neither is this.

[zturner]

On second thought I actually think the strategy used here is fine. But we have llvm::WritableBinaryStream which could server as a single base interface for a mapped_file_region based implementation as well as a malloc-based implementation.

For the malloc based implementation, we already have WritableBinaryByteStream, so you don't even need to implement that.

The nice thing about this is that it could later be improved to use the stream reader / stream writer idiom, which gets rid of a ton of manual pointer arithmetic and management.

[labath]

It took a while, but we've finally landed an llvm::WritableMemoryBuffer class.
This patch now becomes simple, as all I need to do is use that instead of the
MemoryBuffer class.

Well.. almost. The new class does not have the null-termination capability, so I
also update our buffer uses to not require null termination.

[clayborg]

See inlined comment. Quick fix and this will be good to go.

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

409–410 ↗

(On Diff #127500)

We should put the DataBufferLLVM::CreateSliceFromPath() call into a new static function in ObjectFile:

static DataBufferSP ObjectFile::MapFileData(FileSpec file, uint64_t Size, uint64_t Offset);

Then when we need to make fixes, we change just one place instead of all of them. We should add this new function and switch all ObjectFile subclasses over to using it.

[labath]

Add the suggested MapFileData function

[clayborg]

Looks good, thanks for the making the changes. This will make future tweaks to reading of file data in object files just a single change in ObjectFile.cpp.

lldb/trunk/include/lldb/Interpreter/OptionValueFileSpec.h

void SetCurrentValue(const FileSpec &value, bool set_value_was_set) {
     m_current_value = value;
     if (set_value_was_set)
       m_value_was_set = true;
     m_data_sp.reset();
   }
 
   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; }
 
 protected:
   FileSpec m_current_value;
   FileSpec m_default_value;
   lldb::DataBufferSP m_data_sp;
   llvm::sys::TimePoint<> m_data_mod_time;
   uint32_t m_completion_mask;
   bool m_resolve;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_OptionValueFileSpec_h_

lldb/trunk/include/lldb/Symbol/ObjectFile.h

protected:
   /// @return
   ///     Returns \b true if the architecture was changed, \b
   ///     false otherwise.
   //------------------------------------------------------------------
   bool SetModulesArchitecture(const ArchSpec &new_arch);
 
   ConstString GetNextSyntheticSymbolName();
 
+  static lldb::DataBufferSP MapFileData(const FileSpec &file, uint64_t Size,
+                                        uint64_t Offset);
+
 private:
   DISALLOW_COPY_AND_ASSIGN(ObjectFile);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_ObjectFile_h_

lldb/trunk/include/lldb/Target/Target.h

public:
   void SetStandardInputPath(const char *path) = delete;
   void SetStandardOutputPath(const char *path) = delete;
   void SetStandardErrorPath(const char *path) = delete;
 
   bool GetBreakpointsConsultPlatformAvoidList();
 
   lldb::LanguageType GetLanguage() const;
 
-  const char *GetExpressionPrefixContentsAsCString();
+  llvm::StringRef GetExpressionPrefixContents();
 
   bool GetUseHexImmediates() const;
 
   bool GetUseFastStepping() const;
 
   bool GetDisplayExpressionsInCrashlogs() const;
 
   LoadScriptFromSymFile GetLoadScriptFromSymbolFile() const;

lldb/trunk/include/lldb/Utility/DataBufferLLVM.h

 
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/lldb-types.h" // for offset_t
 
 #include <memory>
 #include <stdint.h> // for uint8_t, uint64_t
 
 namespace llvm {
-class MemoryBuffer;
+class WritableMemoryBuffer;
 class Twine;
 }
 
 namespace lldb_private {
 
 class DataBufferLLVM : public DataBuffer {
 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;
   lldb::offset_t GetByteSize() const override;
 
   char *GetChars() { return reinterpret_cast<char *>(GetBytes()); }
 
 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;
 };
 }
 
 #endif

lldb/trunk/source/Interpreter/OptionValueFileSpec.cpp

size_t OptionValueFileSpec::AutoComplete(
   word_complete = false;
   matches.Clear();
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, m_completion_mask, s, match_start_point, max_return_elements,
       nullptr, word_complete, matches);
   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;
 }

lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/SectionLoadList.h"
 #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"
 #include "lldb/Utility/Timer.h"
 
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Decompressor.h"

 
 ObjectFile *ObjectFileELF::CreateInstance(const lldb::ModuleSP &module_sp,
                                           DataBufferSP &data_sp,
                                           lldb::offset_t data_offset,
                                           const lldb_private::FileSpec *file,
                                           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;
   }
 
   assert(data_sp);
 
   if (data_sp->GetByteSize() <= (llvm::ELF::EI_NIDENT + data_offset))
     return nullptr;
 
   const uint8_t *magic = data_sp->GetBytes() + data_offset;
   if (!ELFHeader::MagicBytesMatch(magic))
     return nullptr;
 
   // 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;
     magic = data_sp->GetBytes();
   }
 
   unsigned address_size = ELFHeader::AddressSizeInBytes(magic);
   if (address_size == 4 || address_size == 8) {
if (header.Parse(data, &header_offset)) {
 
           // In case there is header extension in the section #0, the header
           // we parsed above could have sentinel values for e_phnum, e_shnum,
           // and e_shstrndx.  In this case we need to reparse the header
           // with a bigger data source to get the actual values.
           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;
               header.Parse(data, &header_offset);
             }
           }
 
           // Try to get the UUID from the section list. Usually that's at the
           // end, so map the file in if we don't have it already.
           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);
           }
 
           uint32_t gnu_debuglink_crc = 0;
           std::string gnu_debuglink_file;
           SectionHeaderColl section_headers;
           lldb_private::UUID &uuid = spec.GetUUID();
if (header.Parse(data, &header_offset)) {
               // For core files - which usually don't happen to have a
               // gnu_debuglink, and are pretty bulky - calculating whole
               // contents crc32 would be too much of luxury.  Thus we will need
               // to fallback to something simpler.
               if (header.e_type == llvm::ELF::ET_CORE) {
                 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);
                 }
                 ProgramHeaderColl program_headers;
                 GetProgramHeaderInfo(program_headers, data, header);
 
                 size_t segment_data_end = 0;
                 for (ProgramHeaderCollConstIter I = program_headers.begin();
                      I != program_headers.end(); ++I) {
                   segment_data_end = std::max<unsigned long long>(
                       I->p_offset + I->p_filesz, segment_data_end);
                 }
 
                 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);
                 }
 
                 core_notes_crc =
                     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(
                       data.GetDataStart(), data.GetByteSize());
                 }
               }
             }
             if (gnu_debuglink_crc) {

lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #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"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/UUID.h"
 
 #include "lldb/Utility/SafeMachO.h"

 
 ObjectFile *ObjectFileMachO::CreateInstance(const lldb::ModuleSP &module_sp,
                                             DataBufferSP &data_sp,
                                             lldb::offset_t data_offset,
                                             const FileSpec *file,
                                             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;
   }
 
   if (!ObjectFileMachO::MagicBytesMatch(data_sp, data_offset, length))
     return nullptr;
 
   // 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;
   }
   auto objfile_ap = llvm::make_unique<ObjectFileMachO>(
       module_sp, data_sp, data_offset, file, file_offset, length);
   if (!objfile_ap || !objfile_ap->ParseHeader())
     return nullptr;
size_t ObjectFileMachO::GetModuleSpecifications(
   if (ObjectFileMachO::MagicBytesMatch(data_sp, 0, data_sp->GetByteSize())) {
     DataExtractor data;
     data.SetData(data_sp);
     llvm::MachO::mach_header header;
     if (ParseHeader(data, &data_offset, header)) {
       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);
       }
       if (data_sp) {
         ModuleSpec spec;
         spec.GetFileSpec() = file;
         spec.SetObjectOffset(file_offset);
         spec.SetObjectSize(length);
if (can_parse) {
           if (m_data.GetByteSize() < header_and_lc_size) {
             DataBufferSP data_sp;
             ProcessSP process_sp(m_process_wp.lock());
             if (process_sp) {
               data_sp =
                   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;
             }
             if (data_sp)
               m_data.SetData(data_sp);
           }
         }
         return true;
static bool ParseTrieEntries(DataExtractor &data, lldb::offset_t offset,
   return true;
 }
 
 // Read the UUID out of a dyld_shared_cache file on-disk.
 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);
 
   char version_str[7];
   lldb::offset_t offset = 0;
   memcpy(version_str, dsc_header_data.GetData(&offset, 6), 6);
   version_str[6] = '\0';
if (m_header.flags & 0x80000000u) {
          The dyld_cache_local_symbols_entry also lists the start of this
          dylib/framework's nlist records
          and the count of how many nlist records there are for this
          dylib/framework.
       */
 
       // 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);
       }
       if (dsc_data_sp) {
         DataExtractor dsc_header_data(dsc_data_sp, byte_order, addr_byte_size);
 
         bool uuid_match = true;
         if (dsc_uuid.IsValid() && process) {
if (m_header.flags & 0x80000000u) {
 
         uint32_t mappingOffset = dsc_header_data.GetU32(&offset);
 
         // If the mappingOffset points to a location inside the header, we've
         // opened an old dyld shared cache, and should not proceed further.
         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),
 
           DataExtractor dsc_mapping_info_data(dsc_mapping_info_data_sp,
                                               byte_order, addr_byte_size);
           offset = 0;
 
           // The File addresses (from the in-memory Mach-O load commands) for
           // the shared libraries
           // in the shared library cache need to be adjusted by an offset to
           // match up with the
           // dylibOffset identifying field in the
           // dyld_cache_local_symbol_entry's.  This offset is
           // recorded in mapping_offset_value.
           const uint64_t mapping_offset_value =
               dsc_mapping_info_data.GetU64(&offset);
 
           offset = offsetof(struct lldb_copy_dyld_cache_header_v1,
                             localSymbolsOffset);
           uint64_t localSymbolsOffset = dsc_header_data.GetU64(&offset);
           uint64_t localSymbolsSize = dsc_header_data.GetU64(&offset);
 
           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,
                                                    byte_order, addr_byte_size);
 
               offset = 0;
 
               typedef std::map<ConstString, uint16_t> UndefinedNameToDescMap;

lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #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"
 #include "lldb/Utility/UUID.h"
 #include "llvm/BinaryFormat/COFF.h"
 
 #include "llvm/Support/MemoryBuffer.h"
 

 
 ObjectFile *ObjectFilePECOFF::CreateInstance(const lldb::ModuleSP &module_sp,
                                              DataBufferSP &data_sp,
                                              lldb::offset_t data_offset,
                                              const lldb_private::FileSpec *file,
                                              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;
   }
 
   if (!ObjectFilePECOFF::MagicBytesMatch(data_sp))
     return nullptr;
 
   // 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;
   }
 
   auto objfile_ap = llvm::make_unique<ObjectFilePECOFF>(
       module_sp, data_sp, data_offset, file, file_offset, length);
   if (!objfile_ap || !objfile_ap->ParseHeader())
     return nullptr;
bool ObjectFilePECOFF::ParseCOFFOptionalHeader(lldb::offset_t *offset_ptr) {
   *offset_ptr = end_offset;
   return success;
 }
 
 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());
   DataExtractor data;
   if (process_sp) {
     auto data_ap = llvm::make_unique<DataBufferHeap>(size, 0);
     Status readmem_error;
     size_t bytes_read =

lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

if (m_developer_directory.empty()) {
     if (!developer_dir_path_valid) {
       std::string xcode_dir_path;
       const char *xcode_select_prefix_dir = getenv("XCODE_SELECT_PREFIX_DIR");
       if (xcode_select_prefix_dir)
         xcode_dir_path.append(xcode_select_prefix_dir);
       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
         // terminator.
         path_ref =
             path_ref.rtrim("\r\n").take_front(sizeof(developer_dir_path) - 1);
         ::memcpy(developer_dir_path, path_ref.data(), path_ref.size());
         developer_dir_path[path_ref.size()] = '\0';

lldb/trunk/source/Symbol/ObjectFile.cpp

if (set_pc) {
     reg_context->SetPC(file_entry.GetLoadAddress(&target));
   }
   return error;
 }
 
 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);
+}

lldb/trunk/source/Target/Target.cpp

if (expr[0] == '$')
     persistent_var_sp = GetScratchTypeSystemForLanguage(nullptr, eLanguageTypeC)
                             ->GetPersistentExpressionState()
                             ->GetVariable(expr);
 
   if (persistent_var_sp) {
     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,
                                                  0, // Line Number
                                                  fixed_expression);
   }
 
   m_suppress_stop_hooks = old_suppress_value;
LanguageType TargetProperties::GetLanguage() const {
   OptionValueLanguage *value =
       m_collection_sp->GetPropertyAtIndexAsOptionValueLanguage(
           nullptr, ePropertyLanguage);
   if (value)
     return value->GetCurrentValue();
   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() {
   const uint32_t idx = ePropertyBreakpointUseAvoidList;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
       nullptr, idx, g_properties[idx].default_uint_value != 0);
 }
 

lldb/trunk/source/Utility/DataBufferLLVM.cpp

 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 
 #include <assert.h>    // for assert
 #include <type_traits> // for move
 
 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");
 }
 
 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>(
       new DataBufferLLVM(std::move(*Buffer)));
 }
 
 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>(
       new DataBufferLLVM(std::move(*Buffer)));
 }
 
 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());
-}