[llvm] r176886 - [Support][MemoryBuffer] Use sys::fs::mapped_file_region instead of sys::Path::MapInFilePages.

[Aaron Ballman]

This patch broke a lot of the tests on Windows.  I've attached a patch
that fixes most of the tests (we need to take ownership of the file
descriptor because the caller closes it in FileManager, but
mapped_file_region closes it as well), but there are still a handful
of failing tests that you may want to look into as well.

http://pastebin.com/4D5unsYj

You can ignore the ARCMT/protected-scope.m test case as that's a
different bug (already logged).

~Aaron

On Tue, Mar 12, 2013 at 3:28 PM, Michael J. Spencer
<bigcheesegs at gmail.com> wrote:
> Author: mspencer
> Date: Tue Mar 12 14:28:19 2013
> New Revision: 176886
>
> URL: http://llvm.org/viewvc/llvm-project?rev=176886&view=rev
> Log:
> [Support][MemoryBuffer] Use sys::fs::mapped_file_region instead of sys::Path::MapInFilePages.
>
> This gives us memory mapped file I/O on Windows.
>
> Modified:
>     llvm/trunk/lib/Support/MemoryBuffer.cpp
>
> Modified: llvm/trunk/lib/Support/MemoryBuffer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/MemoryBuffer.cpp?rev=176886&r1=176885&r2=176886&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/MemoryBuffer.cpp (original)
> +++ llvm/trunk/lib/Support/MemoryBuffer.cpp Tue Mar 12 14:28:19 2013
> @@ -72,13 +72,15 @@ static void CopyStringRef(char *Memory,
>    Memory[Data.size()] = 0; // Null terminate string.
>  }
>
> -/// GetNamedBuffer - Allocates a new MemoryBuffer with Name copied after it.
> -template <typename T>
> -static T *GetNamedBuffer(StringRef Buffer, StringRef Name,
> -                         bool RequiresNullTerminator) {
> -  char *Mem = static_cast<char*>(operator new(sizeof(T) + Name.size() + 1));
> -  CopyStringRef(Mem + sizeof(T), Name);
> -  return new (Mem) T(Buffer, RequiresNullTerminator);
> +struct NamedBufferAlloc {
> +  StringRef Name;
> +  NamedBufferAlloc(StringRef Name) : Name(Name) {}
> +};
> +
> +void *operator new(size_t N, const NamedBufferAlloc &Alloc) {
> +  char *Mem = static_cast<char *>(operator new(N + Alloc.Name.size() + 1));
> +  CopyStringRef(Mem + N, Alloc.Name);
> +  return Mem;
>  }
>
>  namespace {
> @@ -105,8 +107,8 @@ public:
>  MemoryBuffer *MemoryBuffer::getMemBuffer(StringRef InputData,
>                                           StringRef BufferName,
>                                           bool RequiresNullTerminator) {
> -  return GetNamedBuffer<MemoryBufferMem>(InputData, BufferName,
> -                                         RequiresNullTerminator);
> +  return new (NamedBufferAlloc(BufferName))
> +      MemoryBufferMem(InputData, RequiresNullTerminator);
>  }
>
>  /// getMemBufferCopy - Open the specified memory range as a MemoryBuffer,
> @@ -183,24 +185,38 @@ error_code MemoryBuffer::getFileOrSTDIN(
>  //===----------------------------------------------------------------------===//
>
>  namespace {
> -/// MemoryBufferMMapFile - This represents a file that was mapped in with the
> -/// sys::Path::MapInFilePages method.  When destroyed, it calls the
> -/// sys::Path::UnMapFilePages method.
> -class MemoryBufferMMapFile : public MemoryBufferMem {
> -public:
> -  MemoryBufferMMapFile(StringRef Buffer, bool RequiresNullTerminator)
> -    : MemoryBufferMem(Buffer, RequiresNullTerminator) { }
> +/// \brief Memorry maps a file descriptor using sys::fs::mapped_file_region.
> +///
> +/// This handles converting the offset into a legal offset on the platform.
> +class MemoryBufferMMapFile : public MemoryBuffer {
> +  sys::fs::mapped_file_region MFR;
>
> -  ~MemoryBufferMMapFile() {
> -    static int PageSize = sys::process::get_self()->page_size();
> +  static uint64_t getLegalMapOffset(uint64_t Offset) {
> +    return Offset & ~(sys::fs::mapped_file_region::alignment() - 1);
> +  }
>
> -    uintptr_t Start = reinterpret_cast<uintptr_t>(getBufferStart());
> -    size_t Size = getBufferSize();
> -    uintptr_t RealStart = Start & ~(PageSize - 1);
> -    size_t RealSize = Size + (Start - RealStart);
> +  static uint64_t getLegalMapSize(uint64_t Len, uint64_t Offset) {
> +    return Len + (Offset - getLegalMapOffset(Offset));
> +  }
>
> -    sys::Path::UnMapFilePages(reinterpret_cast<const char*>(RealStart),
> -                              RealSize);
> +  const char *getStart(uint64_t Len, uint64_t Offset) {
> +    return MFR.const_data() + (Offset - getLegalMapOffset(Offset));
> +  }
> +
> +public:
> +  MemoryBufferMMapFile(bool RequiresNullTerminator, int FD, uint64_t Len,
> +                       uint64_t Offset, error_code EC)
> +      : MFR(FD, sys::fs::mapped_file_region::readonly,
> +            getLegalMapSize(Len, Offset), getLegalMapOffset(Offset), EC) {
> +    if (!EC) {
> +      const char *Start = getStart(Len, Offset);
> +      init(Start, Start + Len, RequiresNullTerminator);
> +    }
> +  }
> +
> +  virtual const char *getBufferIdentifier() const LLVM_OVERRIDE {
> +    // The name is stored after the class itself.
> +    return reinterpret_cast<const char *>(this + 1);
>    }
>
>    virtual BufferKind getBufferKind() const LLVM_OVERRIDE {
> @@ -265,7 +281,6 @@ error_code MemoryBuffer::getFile(const c
>
>    error_code ret = getOpenFile(FD, Filename, result, FileSize, FileSize,
>                                 0, RequiresNullTerminator);
> -  close(FD);
>    return ret;
>  }
>
> @@ -344,17 +359,11 @@ error_code MemoryBuffer::getOpenFile(int
>
>    if (shouldUseMmap(FD, FileSize, MapSize, Offset, RequiresNullTerminator,
>                      PageSize)) {
> -    off_t RealMapOffset = Offset & ~(PageSize - 1);
> -    off_t Delta = Offset - RealMapOffset;
> -    size_t RealMapSize = MapSize + Delta;
> -
> -    if (const char *Pages = sys::Path::MapInFilePages(FD,
> -                                                      RealMapSize,
> -                                                      RealMapOffset)) {
> -      result.reset(GetNamedBuffer<MemoryBufferMMapFile>(
> -          StringRef(Pages + Delta, MapSize), Filename, RequiresNullTerminator));
> +    error_code EC;
> +    result.reset(new (NamedBufferAlloc(Filename)) MemoryBufferMMapFile(
> +        RequiresNullTerminator, FD, MapSize, Offset, EC));
> +    if (!EC)
>        return error_code::success();
> -    }
>    }
>
>    MemoryBuffer *Buf = MemoryBuffer::getNewUninitMemBuffer(MapSize, Filename);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PathV2.inc.patch
Type: application/octet-stream
Size: 466 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130313/2aab5aa2/attachment.obj>