[llvm] r176995 - [Support] Fix lifetime of file descriptors when using MemoryBuffer.
Michael J. Spencer
bigcheesegs at gmail.com
Wed Mar 13 17:20:10 PDT 2013
Author: mspencer
Date: Wed Mar 13 19:20:10 2013
New Revision: 176995
URL: http://llvm.org/viewvc/llvm-project?rev=176995&view=rev
Log:
[Support] Fix lifetime of file descriptors when using MemoryBuffer.
Clients of MemoryBuffer::getOpenFile expect it not to take ownership of the file
descriptor passed in. So don't.
Modified:
llvm/trunk/include/llvm/Support/FileSystem.h
llvm/trunk/lib/Support/FileOutputBuffer.cpp
llvm/trunk/lib/Support/MemoryBuffer.cpp
llvm/trunk/lib/Support/Unix/PathV2.inc
llvm/trunk/lib/Support/Windows/PathV2.inc
Modified: llvm/trunk/include/llvm/Support/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=176995&r1=176994&r2=176995&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Wed Mar 13 19:20:10 2013
@@ -602,7 +602,7 @@ private:
void *FileMappingHandle;
#endif
- error_code init(int FD, uint64_t Offset);
+ error_code init(int FD, bool CloseFD, uint64_t Offset);
public:
typedef char char_type;
@@ -633,8 +633,10 @@ public:
error_code &ec);
/// \param fd An open file descriptor to map. mapped_file_region takes
- /// ownership. It must have been opended in the correct mode.
+ /// ownership if closefd is true. It must have been opended in the correct
+ /// mode.
mapped_file_region(int fd,
+ bool closefd,
mapmode mode,
uint64_t length,
uint64_t offset,
Modified: llvm/trunk/lib/Support/FileOutputBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FileOutputBuffer.cpp?rev=176995&r1=176994&r2=176995&view=diff
==============================================================================
--- llvm/trunk/lib/Support/FileOutputBuffer.cpp (original)
+++ llvm/trunk/lib/Support/FileOutputBuffer.cpp Wed Mar 13 19:20:10 2013
@@ -70,8 +70,8 @@ error_code FileOutputBuffer::create(Stri
if (EC)
return EC;
- OwningPtr<mapped_file_region> MappedFile(
- new mapped_file_region(FD, mapped_file_region::readwrite, Size, 0, EC));
+ OwningPtr<mapped_file_region> MappedFile(new mapped_file_region(
+ FD, true, mapped_file_region::readwrite, Size, 0, EC));
if (EC)
return EC;
Modified: llvm/trunk/lib/Support/MemoryBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/MemoryBuffer.cpp?rev=176995&r1=176994&r2=176995&view=diff
==============================================================================
--- llvm/trunk/lib/Support/MemoryBuffer.cpp (original)
+++ llvm/trunk/lib/Support/MemoryBuffer.cpp Wed Mar 13 19:20:10 2013
@@ -206,7 +206,7 @@ class MemoryBufferMMapFile : public Memo
public:
MemoryBufferMMapFile(bool RequiresNullTerminator, int FD, uint64_t Len,
uint64_t Offset, error_code EC)
- : MFR(FD, sys::fs::mapped_file_region::readonly,
+ : MFR(FD, false, sys::fs::mapped_file_region::readonly,
getLegalMapSize(Len, Offset), getLegalMapOffset(Offset), EC) {
if (!EC) {
const char *Start = getStart(Len, Offset);
@@ -281,6 +281,7 @@ error_code MemoryBuffer::getFile(const c
error_code ret = getOpenFile(FD, Filename, result, FileSize, FileSize,
0, RequiresNullTerminator);
+ close(FD);
return ret;
}
Modified: llvm/trunk/lib/Support/Unix/PathV2.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/PathV2.inc?rev=176995&r1=176994&r2=176995&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/PathV2.inc (original)
+++ llvm/trunk/lib/Support/Unix/PathV2.inc Wed Mar 13 19:20:10 2013
@@ -475,12 +475,14 @@ rety_open_create:
return error_code::success();
}
-error_code mapped_file_region::init(int fd, uint64_t offset) {
- AutoFD FD(fd);
+error_code mapped_file_region::init(int FD, bool CloseFD, uint64_t Offset) {
+ AutoFD ScopedFD(FD);
+ if (!CloseFD)
+ ScopedFD.take();
// Figure out how large the file is.
struct stat FileInfo;
- if (fstat(fd, &FileInfo) == -1)
+ if (fstat(FD, &FileInfo) == -1)
return error_code(errno, system_category());
uint64_t FileSize = FileInfo.st_size;
@@ -488,7 +490,7 @@ error_code mapped_file_region::init(int
Size = FileSize;
else if (FileSize < Size) {
// We need to grow the file.
- if (ftruncate(fd, Size) == -1)
+ if (ftruncate(FD, Size) == -1)
return error_code(errno, system_category());
}
@@ -497,7 +499,7 @@ error_code mapped_file_region::init(int
#ifdef MAP_FILE
flags |= MAP_FILE;
#endif
- Mapping = ::mmap(0, Size, prot, flags, fd, offset);
+ Mapping = ::mmap(0, Size, prot, flags, FD, Offset);
if (Mapping == MAP_FAILED)
return error_code(errno, system_category());
return error_code::success();
@@ -526,12 +528,13 @@ mapped_file_region::mapped_file_region(c
return;
}
- ec = init(ofd, offset);
+ ec = init(ofd, true, offset);
if (ec)
Mapping = 0;
}
mapped_file_region::mapped_file_region(int fd,
+ bool closefd,
mapmode mode,
uint64_t length,
uint64_t offset,
@@ -545,7 +548,7 @@ mapped_file_region::mapped_file_region(i
return;
}
- ec = init(fd, offset);
+ ec = init(fd, closefd, offset);
if (ec)
Mapping = 0;
}
Modified: llvm/trunk/lib/Support/Windows/PathV2.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/PathV2.inc?rev=176995&r1=176994&r2=176995&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/PathV2.inc (original)
+++ llvm/trunk/lib/Support/Windows/PathV2.inc Wed Mar 13 19:20:10 2013
@@ -711,12 +711,13 @@ error_code get_magic(const Twine &path,
return error_code::success();
}
-error_code mapped_file_region::init(int FD, uint64_t Offset) {
+error_code mapped_file_region::init(int FD, bool CloseFD, uint64_t Offset) {
FileDescriptor = FD;
// Make sure that the requested size fits within SIZE_T.
if (Size > std::numeric_limits<SIZE_T>::max()) {
if (FileDescriptor)
- _close(FileDescriptor);
+ if (CloseFD)
+ _close(FileDescriptor);
else
::CloseHandle(FileHandle);
return make_error_code(errc::invalid_argument);
@@ -739,7 +740,8 @@ error_code mapped_file_region::init(int
if (FileMappingHandle == NULL) {
error_code ec = windows_error(GetLastError());
if (FileDescriptor)
- _close(FileDescriptor);
+ if (CloseFD)
+ _close(FileDescriptor);
else
::CloseHandle(FileHandle);
return ec;
@@ -761,7 +763,8 @@ error_code mapped_file_region::init(int
error_code ec = windows_error(GetLastError());
::CloseHandle(FileMappingHandle);
if (FileDescriptor)
- _close(FileDescriptor);
+ if (CloseFD)
+ _close(FileDescriptor);
else
::CloseHandle(FileHandle);
return ec;
@@ -775,13 +778,23 @@ error_code mapped_file_region::init(int
::UnmapViewOfFile(Mapping);
::CloseHandle(FileMappingHandle);
if (FileDescriptor)
- _close(FileDescriptor);
+ if (CloseFD)
+ _close(FileDescriptor);
else
::CloseHandle(FileHandle);
return ec;
}
Size = mbi.RegionSize;
}
+
+ // Close all the handles except for the view. It will keep the other handles
+ // alive.
+ ::CloseHandle(FileMappingHandle);
+ if (FileDescriptor)
+ if (CloseFD)
+ _close(FileDescriptor); // Also closes FileHandle.
+ else
+ ::CloseHandle(FileHandle);
return error_code::success();
}
@@ -821,7 +834,7 @@ mapped_file_region::mapped_file_region(c
}
FileDescriptor = 0;
- ec = init(FileDescriptor, offset);
+ ec = init(FileDescriptor, true, offset);
if (ec) {
Mapping = FileMappingHandle = 0;
FileHandle = INVALID_HANDLE_VALUE;
@@ -830,6 +843,7 @@ mapped_file_region::mapped_file_region(c
}
mapped_file_region::mapped_file_region(int fd,
+ bool closefd,
mapmode mode,
uint64_t length,
uint64_t offset,
@@ -842,13 +856,14 @@ mapped_file_region::mapped_file_region(i
, FileMappingHandle() {
FileHandle = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
if (FileHandle == INVALID_HANDLE_VALUE) {
- _close(FileDescriptor);
+ if (closefd)
+ _close(FileDescriptor);
FileDescriptor = 0;
ec = make_error_code(errc::bad_file_descriptor);
return;
}
- ec = init(FileDescriptor, offset);
+ ec = init(FileDescriptor, closefd, offset);
if (ec) {
Mapping = FileMappingHandle = 0;
FileHandle = INVALID_HANDLE_VALUE;
@@ -859,12 +874,6 @@ mapped_file_region::mapped_file_region(i
mapped_file_region::~mapped_file_region() {
if (Mapping)
::UnmapViewOfFile(Mapping);
- if (FileMappingHandle)
- ::CloseHandle(FileMappingHandle);
- if (FileDescriptor)
- _close(FileDescriptor);
- else if (FileHandle != INVALID_HANDLE_VALUE)
- ::CloseHandle(FileHandle);
}
#if LLVM_HAS_RVALUE_REFERENCES
More information about the llvm-commits
mailing list