<div dir="ltr">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.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Feb 15, 2017 at 12:58 PM Greg Clayton <<a href="mailto:gclayton@apple.com">gclayton@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Feb 15, 2017, at 11:22 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="m_6284121997344625594Apple-interchange-newline gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">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.<div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">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?</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg">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...</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Wed, Feb 15, 2017 at 11:19 AM Greg Clayton <<a href="mailto:gclayton@apple.com" class="gmail_msg" target="_blank">gclayton@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Feb 15, 2017, at 11:07 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="m_6284121997344625594m_5789889179168684522Apple-interchange-newline gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">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?</div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg">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.</div><div class="gmail_msg"></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_msg">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</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg">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...</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Greg</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton <<a href="mailto:gclayton@apple.com" class="gmail_msg" target="_blank">gclayton@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><br class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Feb 15, 2017, at 10:58 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="gmail_msg m_6284121997344625594m_5789889179168684522m_-3668642223291469573Apple-interchange-newline"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">On Wed, Feb 15, 2017 at 10:47 AM Reid Kleckner <<a href="mailto:rnk@google.com" class="gmail_msg" target="_blank">rnk@google.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg">On Wed, Feb 15, 2017 at 10:35 AM, Greg Clayton via lldb-dev <span class="gmail_msg"><<a href="mailto:lldb-dev@lists.llvm.org" class="gmail_msg" target="_blank">lldb-dev@lists.llvm.org</a>></span> wrote:<blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg">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.</div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div></div><div class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">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.</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">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.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">After that all lldb has to do is say MemoryBuffer::getOpenFile() and everything works.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Is there a good reason *not* to use it?  Evenif internally the implementation is complex, the external interface is not</div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"></div></div></div></div>
</blockquote></div></div>
</div></blockquote></div><br class="gmail_msg"></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg">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:</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">(lldb) file /tmp/invalid_codesig_app</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">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.</div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Greg</div></div></blockquote></div>
</div></blockquote></div><br class="gmail_msg"></div></blockquote></div>
</div></blockquote></div><br class="gmail_msg"></div></blockquote></div>