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?
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 On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton <gclay...@apple.com> wrote: > > On Feb 15, 2017, at 10:58 AM, Zachary Turner <ztur...@google.com> wrote: > > > On Wed, Feb 15, 2017 at 10:47 AM Reid Kleckner <r...@google.com> wrote: > > On Wed, Feb 15, 2017 at 10:35 AM, Greg Clayton via lldb-dev < > lldb-dev@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 >
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
