[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. Herald added subscribers: krytarowski, arichardson, aprantl, emaste. 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 DataBufferLL

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. I had suggested in https://reviews.llvm.org/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

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D40079#926328, @clayborg wrote: > I had suggested in https://reviews.llvm.org/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

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. 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 imp

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-12-19 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 127500. labath added a comment. 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

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-12-19 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. See inlined comment. Quick fix and this will be good to go. Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:409-410 if (!data_sp) { -data_sp

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-12-20 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 127687. labath added a comment. Add the suggested MapFileData function https://reviews.llvm.org/D40079 Files: include/lldb/Interpreter/OptionValueFileSpec.h include/lldb/Symbol/ObjectFile.h include/lldb/Target/Target.h include/lldb/Utility/DataBuffer

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-12-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. 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. https://reviews.llvm.org/D40079

[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-12-21 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL321255: Make sure DataBufferLLVM contents are writable (authored by labath, committed by ). Repository: rL LLVM https://reviews.llvm.org/D40079 Files: lldb/trunk/include/lldb/Interpreter/OptionValue

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Zachary Turner via lldb-commits
Can we just extend llvm's mapped_file_region to support a boolean Writable flag? On Wed, Nov 15, 2017 at 8:03 AM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath created this revision. > Herald added subscribers: krytarowski, arichardson, aprantl, emaste. > > We sometimes

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via lldb-commits
On 15 November 2017 at 17:42, Zachary Turner wrote: > Can we just extend llvm's mapped_file_region to support a boolean Writable > flag? > mapped_file_region already can be writable. The feature it is missing is the ability to *not* use mmap. And that's not a good idea, as the whole purpose of tha

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Zachary Turner via lldb-commits
On Wed, Nov 15, 2017 at 9:51 AM Pavel Labath wrote: > On 15 November 2017 at 17:42, Zachary Turner wrote: > > Can we just extend llvm's mapped_file_region to support a boolean > Writable > > flag? > > > mapped_file_region already can be writable. The feature it is missing > is the ability to *no

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Greg Clayton via lldb-commits
> On Nov 15, 2017, at 10:11 AM, Zachary Turner wrote: > > > > On Wed, Nov 15, 2017 at 9:51 AM Pavel Labath > wrote: > On 15 November 2017 at 17:42, Zachary Turner > wrote: > > Can we just extend llvm's mapped_file_region to support a boole

Re: [Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable

2017-11-15 Thread Pavel Labath via lldb-commits
On 15 November 2017 at 18:11, Zachary Turner wrote: > > > On Wed, Nov 15, 2017 at 9:51 AM Pavel Labath wrote: >> >> On 15 November 2017 at 17:42, Zachary Turner wrote: >> > Can we just extend llvm's mapped_file_region to support a boolean >> > Writable >> > flag? >> > >> mapped_file_region alrea