[Lldb-commits] [lldb] r230283 - Avoid crashing by not mmap'ing files on network mounted file systems.

Greg Clayton gclayton at apple.com
Mon Feb 23 15:47:22 PST 2015


Author: gclayton
Date: Mon Feb 23 17:47:09 2015
New Revision: 230283

URL: http://llvm.org/viewvc/llvm-project?rev=230283&view=rev
Log:
Avoid crashing by not mmap'ing files on network mounted file systems.

This is implemented by making a new FileSystem function:

bool
FileSystem::IsLocal(const FileSpec &spec)

Then using this in a new function:

DataBufferSP
FileSpec::MemoryMapFileContentsIfLocal(off_t file_offset, size_t file_size) const;

This function only mmaps data if the file is a local file since that means we can reliably page in data. We were experiencing crashes where people would use debug info files on network mounted file systems and that mount would go away and cause the next access to a page that wasn't paged in to crash LLDB. 

We now avoid this by just copying the data into a heap buffer and keeping a permanent copy to avoid the crash. Updated all previous users of FileSpec::MemoryMapFileContentsIfLocal() in ObjectFile subclasses over to use the new FileSpec::MemoryMapFileContentsIfLocal() function.

<rdar://problem/19470249>


Modified:
    lldb/trunk/include/lldb/Host/FileSpec.h
    lldb/trunk/include/lldb/Host/FileSystem.h
    lldb/trunk/source/Host/common/FileSpec.cpp
    lldb/trunk/source/Host/posix/FileSystem.cpp
    lldb/trunk/source/Host/windows/FileSystem.cpp
    lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
    lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
    lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
    lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

Modified: lldb/trunk/include/lldb/Host/FileSpec.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/FileSpec.h (original)
+++ lldb/trunk/include/lldb/Host/FileSpec.h Mon Feb 23 17:47:09 2015
@@ -530,6 +530,45 @@ public:
     lldb::DataBufferSP
     MemoryMapFileContents (off_t offset = 0, size_t length = SIZE_MAX) const;
 
+
+    //------------------------------------------------------------------
+    /// Memory map part of, or the entire contents of, a file only if
+    /// the file is local (not on a network mount).
+    ///
+    /// Returns a shared pointer to a data buffer that contains all or
+    /// part of the contents of a file. The data will be memory mapped
+    /// if the file is local and will lazily page in data from the file
+    /// as memory is accessed. If the data is memory mapped, the data
+    /// that is mapped will start \a offset bytes into the file, and
+    /// \a length bytes will be mapped. If \a length is
+    /// greater than the number of bytes available in the file starting
+    /// at \a offset, the number of bytes will be appropriately
+    /// truncated. The final number of bytes that get mapped can be
+    /// verified using the DataBuffer::GetByteSize() function on the return
+    /// shared data pointer object contents.
+    ///
+    /// If the file is on a network mount the data will be read into a
+    /// heap buffer immediately so that accesses to the data won't later
+    /// cause a crash if we touch a page that isn't paged in and the
+    /// network mount has been disconnected or gone away.
+    ///
+    /// @param[in] offset
+    ///     The offset in bytes from the beginning of the file where
+    ///     memory mapping should begin.
+    ///
+    /// @param[in] length
+    ///     The size in bytes that should be mapped starting \a offset
+    ///     bytes into the file. If \a length is \c SIZE_MAX, map
+    ///     as many bytes as possible.
+    ///
+    /// @return
+    ///     A shared pointer to the memory mapped data. This shared
+    ///     pointer can contain a NULL DataBuffer pointer, so the contained
+    ///     pointer must be checked prior to using it.
+    //------------------------------------------------------------------
+    lldb::DataBufferSP
+    MemoryMapFileContentsIfLocal(off_t file_offset, size_t file_size) const;
+
     //------------------------------------------------------------------
     /// Read part of, or the entire contents of, a file into a heap based data buffer.
     ///

Modified: lldb/trunk/include/lldb/Host/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSystem.h?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/FileSystem.h (original)
+++ lldb/trunk/include/lldb/Host/FileSystem.h Mon Feb 23 17:47:09 2015
@@ -38,6 +38,9 @@ class FileSystem
 
     static bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high);
     static bool CalculateMD5AsString(const FileSpec &file_spec, std::string& digest_str);
+
+    /// Return \b true if \a spec is on a locally mounted file system, \b false otherwise.
+    static bool IsLocal(const FileSpec &spec);
 };
 }
 

Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Mon Feb 23 17:47:09 2015
@@ -852,6 +852,15 @@ FileSpec::MemoryMapFileContents(off_t fi
     return data_sp;
 }
 
+DataBufferSP
+FileSpec::MemoryMapFileContentsIfLocal(off_t file_offset, size_t file_size) const
+{
+    if (FileSystem::IsLocal(*this))
+        return MemoryMapFileContents(file_offset, file_size);
+    else
+        return ReadFileContents(file_offset, file_size, NULL);
+}
+
 
 //------------------------------------------------------------------
 // Return the size in bytes that this object takes in memory. This

Modified: lldb/trunk/source/Host/posix/FileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/FileSystem.cpp?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/source/Host/posix/FileSystem.cpp (original)
+++ lldb/trunk/source/Host/posix/FileSystem.cpp Mon Feb 23 17:47:09 2015
@@ -10,6 +10,8 @@
 #include "lldb/Host/FileSystem.h"
 
 // C includes
+#include <sys/mount.h>
+#include <sys/param.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 
@@ -172,3 +174,13 @@ FileSystem::Readlink(const char *path, c
         error.SetErrorString("'buf' buffer is too small to contain link contents");
     return error;
 }
+
+bool
+FileSystem::IsLocal(const FileSpec &spec)
+{
+    struct statfs statfs_info;
+    std::string path (spec.GetPath());
+    if (statfs(path.c_str(), &statfs_info) == 0)
+        return (statfs_info.f_flags & MNT_LOCAL) != 0;
+    return false;
+}

Modified: lldb/trunk/source/Host/windows/FileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/FileSystem.cpp?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/source/Host/windows/FileSystem.cpp (original)
+++ lldb/trunk/source/Host/windows/FileSystem.cpp Mon Feb 23 17:47:09 2015
@@ -138,3 +138,15 @@ FileSystem::Readlink(const char *path, c
     ::CloseHandle(h);
     return error;
 }
+
+bool
+FileSystem::IsLocal(const FileSpec &spec)
+{
+    if (spec)
+    {
+        // TODO: return true if the file is on a locally mounted file system
+        return true;
+    }
+
+    return false;
+}

Modified: lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp Mon Feb 23 17:47:09 2015
@@ -364,7 +364,7 @@ ObjectContainerBSDArchive::CreateInstanc
 
                 // Map the entire .a file to be sure that we don't lose any data if the file
                 // gets updated by a new build while this .a file is being used for debugging
-                DataBufferSP archive_data_sp (file->MemoryMapFileContents(file_offset, length));
+                DataBufferSP archive_data_sp (file->MemoryMapFileContentsIfLocal(file_offset, length));
                 lldb::offset_t archive_data_offset = 0;
 
                 Archive::shared_ptr archive_sp (Archive::FindCachedArchive (*file,
@@ -570,7 +570,7 @@ ObjectContainerBSDArchive::GetModuleSpec
         if (!archive_sp)
         {
             set_archive_arch = true;
-            DataBufferSP data_sp (file.MemoryMapFileContents(file_offset, file_size));
+            DataBufferSP data_sp (file.MemoryMapFileContentsIfLocal(file_offset, file_size));
             data.SetData (data_sp, 0, data_sp->GetByteSize());
             archive_sp = Archive::ParseAndCacheArchiveForFile(file, ArchSpec(), file_mod_time, file_offset, data);
         }

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Mon Feb 23 17:47:09 2015
@@ -365,7 +365,7 @@ ObjectFileELF::CreateInstance (const lld
 {
     if (!data_sp)
     {
-        data_sp = file->MemoryMapFileContents(file_offset, length);
+        data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
         data_offset = 0;
     }
 
@@ -376,7 +376,7 @@ ObjectFileELF::CreateInstance (const lld
         {
             // Update the data to contain the entire file if it doesn't already
             if (data_sp->GetByteSize() < length) {
-                data_sp = file->MemoryMapFileContents(file_offset, length);
+                data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
                 data_offset = 0;
                 magic = data_sp->GetBytes();
             }
@@ -636,7 +636,7 @@ ObjectFileELF::GetModuleSpecifications (
                     size_t section_header_end = header.e_shoff + header.e_shnum * header.e_shentsize;
                     if (section_header_end > data_sp->GetByteSize())
                     {
-                        data_sp = file.MemoryMapFileContents (file_offset, section_header_end);
+                        data_sp = file.MemoryMapFileContentsIfLocal (file_offset, section_header_end);
                         data.SetData(data_sp);
                     }
 
@@ -678,7 +678,7 @@ ObjectFileELF::GetModuleSpecifications (
                                 size_t program_headers_end = header.e_phoff + header.e_phnum * header.e_phentsize;
                                 if (program_headers_end > data_sp->GetByteSize())
                                 {
-                                    data_sp = file.MemoryMapFileContents(file_offset, program_headers_end);
+                                    data_sp = file.MemoryMapFileContentsIfLocal(file_offset, program_headers_end);
                                     data.SetData(data_sp);
                                 }
                                 ProgramHeaderColl program_headers;
@@ -693,7 +693,7 @@ ObjectFileELF::GetModuleSpecifications (
 
                                 if (segment_data_end > data_sp->GetByteSize())
                                 {
-                                    data_sp = file.MemoryMapFileContents(file_offset, segment_data_end);
+                                    data_sp = file.MemoryMapFileContentsIfLocal(file_offset, segment_data_end);
                                     data.SetData(data_sp);
                                 }
 
@@ -702,7 +702,7 @@ ObjectFileELF::GetModuleSpecifications (
                             else
                             {
                                 // Need to map entire file into memory to calculate the crc.
-                                data_sp = file.MemoryMapFileContents (file_offset, SIZE_MAX);
+                                data_sp = file.MemoryMapFileContentsIfLocal (file_offset, SIZE_MAX);
                                 data.SetData(data_sp);
                                 gnu_debuglink_crc = calc_gnu_debuglink_crc32 (data.GetDataStart(), data.GetByteSize());
                             }

Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Mon Feb 23 17:47:09 2015
@@ -931,7 +931,7 @@ ObjectFileMachO::CreateInstance (const l
 {
     if (!data_sp)
     {
-        data_sp = file->MemoryMapFileContents(file_offset, length);
+        data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
         data_offset = 0;
     }
 
@@ -940,7 +940,7 @@ ObjectFileMachO::CreateInstance (const l
         // Update the data to contain the entire file if it doesn't already
         if (data_sp->GetByteSize() < length)
         {
-            data_sp = file->MemoryMapFileContents(file_offset, length);
+            data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
             data_offset = 0;
         }
         std::unique_ptr<ObjectFile> objfile_ap(new ObjectFileMachO (module_sp, data_sp, data_offset, file, file_offset, length));
@@ -2650,7 +2650,7 @@ ObjectFileMachO::ParseSymtab ()
             // Save some VM space, do not map the entire cache in one shot.
 
             DataBufferSP dsc_data_sp;
-            dsc_data_sp = dsc_filespec.MemoryMapFileContents(0, sizeof(struct lldb_copy_dyld_cache_header_v1));
+            dsc_data_sp = dsc_filespec.MemoryMapFileContentsIfLocal(0, sizeof(struct lldb_copy_dyld_cache_header_v1));
 
             if (dsc_data_sp)
             {
@@ -2704,7 +2704,7 @@ ObjectFileMachO::ParseSymtab ()
                 if (uuid_match && mappingOffset >= sizeof(struct lldb_copy_dyld_cache_header_v0))
                 {
 
-                    DataBufferSP dsc_mapping_info_data_sp = dsc_filespec.MemoryMapFileContents(mappingOffset, sizeof (struct lldb_copy_dyld_cache_mapping_info));
+                    DataBufferSP dsc_mapping_info_data_sp = dsc_filespec.MemoryMapFileContentsIfLocal(mappingOffset, 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;
 
@@ -2721,7 +2721,7 @@ ObjectFileMachO::ParseSymtab ()
                     if (localSymbolsOffset && localSymbolsSize)
                     {
                         // Map the local symbols
-                        if (DataBufferSP dsc_local_symbols_data_sp = dsc_filespec.MemoryMapFileContents(localSymbolsOffset, localSymbolsSize))
+                        if (DataBufferSP dsc_local_symbols_data_sp = dsc_filespec.MemoryMapFileContentsIfLocal(localSymbolsOffset, localSymbolsSize))
                         {
                             DataExtractor dsc_local_symbols_data(dsc_local_symbols_data_sp, byte_order, addr_byte_size);
 

Modified: lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp?rev=230283&r1=230282&r2=230283&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Mon Feb 23 17:47:09 2015
@@ -76,7 +76,7 @@ ObjectFilePECOFF::CreateInstance (const
 {
     if (!data_sp)
     {
-        data_sp = file->MemoryMapFileContents(file_offset, length);
+        data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
         data_offset = 0;
     }
 
@@ -84,7 +84,7 @@ ObjectFilePECOFF::CreateInstance (const
     {
         // Update the data to contain the entire file if it doesn't already
         if (data_sp->GetByteSize() < length)
-            data_sp = file->MemoryMapFileContents(file_offset, length);
+            data_sp = file->MemoryMapFileContentsIfLocal(file_offset, length);
         std::unique_ptr<ObjectFile> objfile_ap(new ObjectFilePECOFF (module_sp, data_sp, data_offset, file, file_offset, length));
         if (objfile_ap.get() && objfile_ap->ParseHeader())
             return objfile_ap.release();





More information about the lldb-commits mailing list