[lldb-dev] About lldbHost

Wed Feb 15 15:48:44 PST 2017

Looked into this a little bit, and it's not a problem AFAICT.  LLDB only
memory maps in one place, which is from FileSpec::MemoryMapFileContents,
and it passes writeable=false.  LLVM uses MAP_PRIVATE when
writeable==false, so from LLDB's point of view there is no functional
change.  The writeable codepath seems entirely unused in LLDB.

On Wed, Feb 15, 2017 at 12:58 PM Greg Clayton <gclayton at apple.com> wrote:

> On Feb 15, 2017, at 11:22 AM, Zachary Turner <zturner at google.com> wrote:
>
> Yea, the flag seems like one you would want to use almost always, so I'm
> not opposed to having it.  I'll see about making the changes in LLVM, even
> if we end up not using it in LLDB, they seem useful in LLVM independently.
>
> BTW, one difference in LLVM's mmap code is that in LLDB we always use
> MAP_PRIVATE, but in LLVM if you are opening for readwrite it uses
> MAP_SHARED.  Are you against using MAP_SHARED when mmaping with readwrite
> privileges?
>
>
> You will need to do some testing. If you do MAP_SHARED and the file goes
> away, you might crash as it won't keep the file around as long as it needs
> it. I could be wrong on this. But I do remember explicitly picking
> MAP_PRIVATE for some reason in the past...
>
> On Wed, Feb 15, 2017 at 11:19 AM Greg Clayton <gclayton at apple.com> wrote:
>
> On Feb 15, 2017, at 11:07 AM, Zachary Turner <zturner at google.com> wrote:
>
> If you only ever call MemoryMapContentsIfLocal, then is that first flag
> even doing anything?  And if you are passing that flag, then can you just
> mmap it always even if non-local?
>
>
> If that is the only call people are using, then we don't really need the
> flag. Not sure how else as local file could go away such that the backing
> store wouldn't be available. mmap() on unix will keep the file around as
> long as its needed AFAIK, so even if someone deletes it, it would be kept
> around. So if those are our only clients we should be ok. The code signing
> bit would need to be added for Mac though.
>
> I searched the code and only in the windows minidump plugin are we
> unconditionally mmaping, and that could be changed to a
> conditional-on-local just like everywhere else.  If we do that, then nobody
> is ever mmaping any non-local file AFAICT
>
>
> That currently is true, but it would be shame to lose the ability to be
> resilient in cases where you do want to mmap. If a file is too large, we
> really should probably have an upper end cutoff on file size that would
> mmap even if remote. If we have a 4 GB file that we want to access,
> probably not a great thing to just pop into a heap based memory buffer...
>
> Greg
>
>
> On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton <gclayton at apple.com> wrote:
>
>
> On Feb 15, 2017, at 10:58 AM, Zachary Turner <zturner at google.com> wrote:
>
>
> On Wed, Feb 15, 2017 at 10:47 AM Reid Kleckner <rnk at google.com> wrote:
>
> On Wed, Feb 15, 2017 at 10:35 AM, Greg Clayton via lldb-dev <
> lldb-dev at lists.llvm.org> wrote:
>
> I am fine with switching mmap over to llvm if it works. One important
> thing to LLDB is we have a "mmap if not on remote file system" that must
> continue to work. If you mmap something from a network drive and then it
> gets disconnected, you can crash LLDB. So we have a function we used that
> implements mmap if local, read all contents if remote, that must work to
> avoid crashes.
>
>
> LLVM's MemoryBuffer API already serves too many different use cases.
> Initially it was designed to be a utility for efficiently reading source
> file inputs. It has a bunch of functionality around ensuring that the
> buffer is null terminated, and a boolean to avoid using mmap when the user
> might modify the file concurrently. It's too complicated. I wouldn't
> recommend using it without a good reason beyond just reusing a platform
> abstraction. mmap and MapViewOfFile are not that complicated. LLDB is
> probably better off doing its own thing unless it needs to pass mapped file
> contents to other parts of LLVM, like maybe clang's VFS.
>
>
> I just took a look and it seems like almost a drop in replacement.  Only
> thing that would need changing is updating shouldUseMmap() to return false
> if a file is on a network share.  But this seems like a good change
> independently of lldb.
>
> After that all lldb has to do is say MemoryBuffer::getOpenFile() and
> everything works.
>
> Is there a good reason *not* to use it?  Evenif internally the
> implementation is complex, the external interface is not
>
>
> The other thing is on Mac we add new flags to mmap that allow us not to
> crash if the backing store (network mount) goes away. There is also a flag
> that says "if code signature is borked, please still allow me to read the
> file without crashing. That functionality will need to be ported into the
> LLVM code somehow so we don't start crashing again. Previously we would
> crash if someone would do:
>
> (lldb) file /tmp/invalid_codesig_app
>
> And also if the network mounted share would go away on something that was
> mmap'ed and someone would touch any byte within it. Our version of mmap
> works around this and we need that to be in any version we adopt.
>
> Greg
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20170215/facf45d2/attachment.html>